Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(95)

Issue 2415183004: Revert of [tracing] Add filtering mode in TraceLog (Closed)

Created:
4 years, 2 months ago by Ilya Kulshin
Modified:
4 years, 2 months ago
CC:
chromium-reviews, tracing+reviews_chromium.org, wfh+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of [tracing] Add filtering mode in TraceLog (patchset #7 id:380001 of https://codereview.chromium.org/2323483005/ ) Reason for revert: Causes webkit failures https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Linux_Precise__dbg_/603/layout-test-results/results.html in https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Linux%20Precise%20%28dbg%29/builds/603 Original issue's description: > [tracing] Add filtering mode in TraceLog > > This CL does: > 1. Add new FILTERING_MODE in TraceLog. TraceLog can be enabled and > disabled with FILTERING_MODE independently from RECORDING MODE. The > TraceLog::mode_ is a bitmap. > 2. In FILTERING_MODE trace buffer is not created and trace events are > passed to filters but not recorded. > 3. The filters can be added only by FILTERING_MODE and trace config. > Once enabled, the filters cannot be changed until the filters are > disabled. > 4. HeapProfilerFilter is enabled using FILTERING_MODE when > "--enable-heap-profiling" flag is passed, so that pseudo stack is > recorded for all allocations, irrespective of tracing. > > BUG=625170 > > Committed: https://crrev.com/20d2eeed2c735fbf018b9a9dffa810d51a25940d > Cr-Commit-Position: refs/heads/master@{#425156} TBR=oysteine@chromium.org,primiano@chromium.org,ssid@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=625170

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+105 lines, -326 lines) Patch
M base/trace_event/heap_profiler_allocation_context_tracker.cc View 1 chunk +1 line, -3 lines 0 comments Download
M base/trace_event/heap_profiler_allocation_context_tracker_unittest.cc View 2 chunks +3 lines, -15 lines 0 comments Download
M base/trace_event/memory_dump_manager.cc View 1 chunk +1 line, -24 lines 0 comments Download
M base/trace_event/trace_config.h View 3 chunks +1 line, -6 lines 0 comments Download
M base/trace_event/trace_config.cc View 6 chunks +26 lines, -21 lines 0 comments Download
M base/trace_event/trace_event_unittest.cc View 6 chunks +5 lines, -123 lines 0 comments Download
M base/trace_event/trace_log.h View 5 chunks +14 lines, -34 lines 0 comments Download
M base/trace_event/trace_log.cc View 9 chunks +54 lines, -100 lines 0 comments Download

Messages

Total messages: 5 (2 generated)
Ilya Kulshin
Created Revert of [tracing] Add filtering mode in TraceLog
4 years, 2 months ago (2016-10-13 23:23:20 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2415183004/1
4 years, 2 months ago (2016-10-13 23:23:46 UTC) #3
commit-bot: I haz the power
4 years, 2 months ago (2016-10-13 23:24:25 UTC) #5
Failed to apply patch for base/trace_event/trace_log.h:
While running git apply --index -3 -p1;
  error: patch failed: base/trace_event/trace_log.h:538
  Falling back to three-way merge...
  Applied patch to 'base/trace_event/trace_log.h' with conflicts.
  U base/trace_event/trace_log.h

Patch:       base/trace_event/trace_log.h
Index: base/trace_event/trace_log.h
diff --git a/base/trace_event/trace_log.h b/base/trace_event/trace_log.h
index
d7c4dcb2f11fb4e4ba95dae3d507319ae53453d9..d0759cd9835c1ce9e0f9e2eeea727dab94fd035f
100644
--- a/base/trace_event/trace_log.h
+++ b/base/trace_event/trace_log.h
@@ -44,14 +44,9 @@
 
 class BASE_EXPORT TraceLog : public MemoryDumpProvider {
  public:
-  // Argument passed to TraceLog::SetEnabled.
-  enum Mode : uint8_t {
-    // Enables normal tracing (recording trace events in the trace buffer).
-    RECORDING_MODE = 1 << 0,
-
-    // Trace events are enabled just for filtering but not for recording. Only
-    // event filters config of |trace_config| argument is used.
-    FILTERING_MODE = 1 << 1
+  enum Mode {
+    DISABLED = 0,
+    RECORDING_MODE
   };
 
   // The pointer returned from GetCategoryGroupEnabledInternal() points to a
@@ -82,30 +77,16 @@
   // if the current thread supports that (has a message loop).
   void InitializeThreadLocalEventBufferIfSupported();
 
-  // See TraceConfig comments for details on how to control which categories
-  // will be traced. SetDisabled must be called distinctly for each mode that
is
-  // enabled. If tracing has already been enabled for recording, category
filter
-  // (enabled and disabled categories) will be merged into the current category
-  // filter. Enabling RECORDING_MODE does not enable filters. Trace event
-  // filters will be used only if FILTERING_MODE is set on |modes_to_enable|.
-  // Conversely to RECORDING_MODE, FILTERING_MODE doesn't support upgrading,
-  // i.e. filters can only be enabled if not previously enabled.
-  void SetEnabled(const TraceConfig& trace_config, uint8_t modes_to_enable);
-
-  // TODO(ssid): Remove the default SetEnabled and IsEnabled. They should take
-  // Mode as argument.
-
-  // Disables tracing for all categories for the specified |modes_to_disable|
-  // only. Only RECORDING_MODE is taken as default |modes_to_disable|.
+  // Enables normal tracing (recording trace events in the trace buffer).
+  // See TraceConfig comments for details on how to control what categories
+  // will be traced. If tracing has already been enabled, |category_filter|
will
+  // be merged into the current category filter.
+  void SetEnabled(const TraceConfig& trace_config, Mode mode);
+
+  // Disables normal tracing for all categories.
   void SetDisabled();
-  void SetDisabled(uint8_t modes_to_disable);
-
-  // Returns true if TraceLog is enabled on recording mode.
-  // Note: Returns false even if FILTERING_MODE is enabled.
-  bool IsEnabled() { return enabled_modes_ & RECORDING_MODE; }
-
-  // Returns a bitmap of enabled modes from TraceLog::Mode.
-  uint8_t enabled_modes() { return enabled_modes_; }
+
+  bool IsEnabled() { return mode_ != DISABLED; }
 
   // The number of times we have begun recording traces. If tracing is off,
   // returns -1. If tracing is on, then it returns the number of times we have
@@ -452,7 +433,7 @@
   TraceEvent* AddEventToThreadSharedChunkWhileLocked(TraceEventHandle* handle,
                                                      bool
check_buffer_is_full);
   void CheckIfBufferIsFullWhileLocked();
-  void SetDisabledWhileLocked(uint8_t modes);
+  void SetDisabledWhileLocked();
 
   TraceEvent* GetEventByHandleInternal(TraceEventHandle handle,
                                        OptionalAutoLock* lock);
@@ -500,7 +481,7 @@
   // This lock protects accesses to thread_names_, thread_event_start_times_
   // and thread_colors_.
   Lock thread_info_lock_;
-  uint8_t enabled_modes_;  // See TraceLog::Mode.
+  Mode mode_;
   int num_traces_recorded_;
   std::unique_ptr<TraceBuffer> logged_events_;
   std::vector<std::unique_ptr<TraceEvent>> metadata_events_;
@@ -538,7 +519,6 @@
 
   TraceConfig trace_config_;
   TraceConfig event_callback_trace_config_;
-  TraceConfig::EventFilters enabled_event_filters_;
 
   ThreadLocalPointer<ThreadLocalEventBuffer> thread_local_event_buffer_;
   ThreadLocalBoolean thread_blocks_message_loop_;

Powered by Google App Engine
This is Rietveld 408576698