MemoryInfra: don't count tracing memory in heap profiler |
||||||
Issue descriptionBackground context:go/memory-infra: memory profiling in chrome://tracing Today the heap profiler shows a lot of memory allocated from tracing. Whilst technically true, that is really confusing. There is a long term strategy to that, which will be tracing v2 (doc will be announced within 48h) but that is a major project and at least a quarter-wide OKR. In the meantime we should figure out a reasonalbe way to exclude the biggest contributors. Concretely, I think we need to create a MallocDumpProvider::ScopedHeapProfilerIgnore class. In my mind this is a simple RAII object that when ctor-ed increases a thread-local counter and when dtor-ed decreses the counter. hooked mallocs should be ignored if they happen when the tls counter > 0. Thinking more, we should probably put that counter into AllocationContextTracker (Which is already per-thread), as the suppression logic should probably be not malloc-specifig but partition-alloc wide. I think it's a pretty simple change to do. The biggest part of the work (but not really big) would be adding those ScopedHeapProfilerIgnore() suppressions in the codebase. I suppose at the end there are a dozen places that need it. Everybody cc-ed, does the plan LGTY? I'd assign this to kraynov as it's a good starter bug, but he is in MTV for orientation and not sure how much bandwidth he has. Let's organize this.
,
Apr 19 2016
Sounds like a good plan!
,
Apr 19 2016
ssid/skiba/mariakhomenko is this something browser-memory-mtv@ would be able to help with? I can definitely create the ScopedHeapProfilerIgnore stuff, but I wouldn't mind some help adding the exclusions in the codebase.
,
Apr 19 2016
This plan sounds good. From discussion with tasak@ about this, he already tried to implement this and faced a few difficult issues. tasak@ do you have something to share regarding this issue?
,
Apr 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/04d1750c6721a424ae45e7e28f71dba69f1789db commit 04d1750c6721a424ae45e7e28f71dba69f1789db Author: ssid <ssid@chromium.org> Date: Fri Apr 22 17:56:45 2016 [tracing] Ignore tracing allocations in heap profiler This CL removes the memory usage of tracing from the heap profiler. Adds support for an ignore event which clears the stack frame and type for the allocations happening in that scope and reset them to "overhead". We do not make it null because it will show under "<other>" or "[unknown]". Adds the ignore events to major functions that allocate memory for tracing. The total is usually 500Kib more than the total in "tracing" dump. BUG= 604689 Review URL: https://codereview.chromium.org/1900223003 Cr-Commit-Position: refs/heads/master@{#389159} [modify] https://crrev.com/04d1750c6721a424ae45e7e28f71dba69f1789db/base/BUILD.gn [add] https://crrev.com/04d1750c6721a424ae45e7e28f71dba69f1789db/base/trace_event/heap_profiler.h [modify] https://crrev.com/04d1750c6721a424ae45e7e28f71dba69f1789db/base/trace_event/heap_profiler_allocation_context_tracker.cc [modify] https://crrev.com/04d1750c6721a424ae45e7e28f71dba69f1789db/base/trace_event/heap_profiler_allocation_context_tracker.h [modify] https://crrev.com/04d1750c6721a424ae45e7e28f71dba69f1789db/base/trace_event/heap_profiler_allocation_context_tracker_unittest.cc [modify] https://crrev.com/04d1750c6721a424ae45e7e28f71dba69f1789db/base/trace_event/memory_dump_manager.cc [modify] https://crrev.com/04d1750c6721a424ae45e7e28f71dba69f1789db/base/trace_event/trace_buffer.cc [modify] https://crrev.com/04d1750c6721a424ae45e7e28f71dba69f1789db/base/trace_event/trace_event.gypi [modify] https://crrev.com/04d1750c6721a424ae45e7e28f71dba69f1789db/base/trace_event/trace_log.cc
,
Apr 23 2016
,
May 6 2016
I don't think we found more cases where the ignore events are required. I am guessing (from investigations) that all the useful locations allocating memory for tracing already have been added with ignore events. If anything goes missing in future lets file another bug.
,
May 9 2016
Issue 580114 has been merged into this issue.
,
Jan 4 2017
,
Jan 4 2017
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by primiano@chromium.org
, Apr 19 2016