New issue
Advanced search Search tips

Issue 644385 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Feature

Blocked on:
issue 669593



Sign in to add a comment

Allow measuring the memory usage and churn in a scope.

Project Member Reported by siggi@chromium.org, Sep 6 2016

Issue description

There exists a task I suspect of allocating 20-30% of my browser startup memory footprint. I'd like to be able to profile this in the field, and phone home with UMA.
Additionally, it would be nice to be able to expose per-task memory allocation statistics in the task profiler, e.g. chrome://profiler. The universal heap shim allows doing this without performance impact to clients where it's not enabled, so I'm going forward with an implementation to do this in base::debug.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/46e1b077586d5b91f9abb7a6a8f9eb6558a584bd

commit 46e1b077586d5b91f9abb7a6a8f9eb6558a584bd
Author: siggi <siggi@chromium.org>
Date: Fri Sep 09 16:43:31 2016

Implement a ScopedThreadHeapUsage class to allow profiling per-thread heap usage.

This uses the generic allocator shim to hook into heap allocations.
When disabled and unused, there is no runtime penalty for this. When heap tracing is enabled, there's a small accounting overhead for every allocation.

Instantiating a ScopedThreadHeapUsage instance carries O(1) cost, whether or not heap tracing is enabled.

BUG= 644385 

Review-Url: https://codereview.chromium.org/2163783003
Cr-Commit-Position: refs/heads/master@{#417601}

[modify] https://crrev.com/46e1b077586d5b91f9abb7a6a8f9eb6558a584bd/base/BUILD.gn
[modify] https://crrev.com/46e1b077586d5b91f9abb7a6a8f9eb6558a584bd/base/allocator/allocator_shim.h
[modify] https://crrev.com/46e1b077586d5b91f9abb7a6a8f9eb6558a584bd/base/allocator/allocator_shim_default_dispatch_to_glibc.cc
[modify] https://crrev.com/46e1b077586d5b91f9abb7a6a8f9eb6558a584bd/base/allocator/allocator_shim_default_dispatch_to_linker_wrapped_symbols.cc
[modify] https://crrev.com/46e1b077586d5b91f9abb7a6a8f9eb6558a584bd/base/allocator/allocator_shim_default_dispatch_to_tcmalloc.cc
[modify] https://crrev.com/46e1b077586d5b91f9abb7a6a8f9eb6558a584bd/base/allocator/allocator_shim_default_dispatch_to_winheap.cc
[modify] https://crrev.com/46e1b077586d5b91f9abb7a6a8f9eb6558a584bd/base/allocator/winheap_stubs_win.cc
[modify] https://crrev.com/46e1b077586d5b91f9abb7a6a8f9eb6558a584bd/base/allocator/winheap_stubs_win.h
[add] https://crrev.com/46e1b077586d5b91f9abb7a6a8f9eb6558a584bd/base/debug/scoped_thread_heap_usage.cc
[add] https://crrev.com/46e1b077586d5b91f9abb7a6a8f9eb6558a584bd/base/debug/scoped_thread_heap_usage.h
[add] https://crrev.com/46e1b077586d5b91f9abb7a6a8f9eb6558a584bd/base/debug/scoped_thread_heap_usage_unittest.cc
[modify] https://crrev.com/46e1b077586d5b91f9abb7a6a8f9eb6558a584bd/base/trace_event/malloc_dump_provider.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 18 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/07856a7b8add0eba66d71fc62b5e32be9bc03d69

commit 07856a7b8add0eba66d71fc62b5e32be9bc03d69
Author: siggi <siggi@chromium.org>
Date: Tue Oct 18 17:18:29 2016

Rename ScopedThreadHeapUsage to ThreadHeapUsageTracker and change interface.

This is broken out from https://codereview.chromium.org/2386123003/, and the interface changes are influenced by actual usage.

The interface is changed to use explicit Start/Stop, instead of implicitly starting and stopping on creation/destruction. This is necessary to support exclusive scopes, which is how base/tracked_objects.h does task execution time measurement. Unfortunately a Scoped* abstraction can't support exclusive scopes, as here must always be a gap from reading the current state to when the Scoped* instance goes out of scope. Any heap activity in this gap will be lost from measurement, and will neither tally to the current nor the outer scope.

Initialization is also changed, such that TLS is initialized as heap tracking is enabled. It is now allowed to create ThreadHeapTracker instances without any initialization, though it's not allowed to Start() them.

BUG= 644385 

Review-Url: https://codereview.chromium.org/2427503003
Cr-Commit-Position: refs/heads/master@{#425997}

[modify] https://crrev.com/07856a7b8add0eba66d71fc62b5e32be9bc03d69/base/BUILD.gn
[delete] https://crrev.com/ba4b204b711dff1d75ed36725aa7c8d4c46e7146/base/debug/scoped_thread_heap_usage.h
[delete] https://crrev.com/ba4b204b711dff1d75ed36725aa7c8d4c46e7146/base/debug/scoped_thread_heap_usage_unittest.cc
[rename] https://crrev.com/07856a7b8add0eba66d71fc62b5e32be9bc03d69/base/debug/thread_heap_usage_tracker.cc
[add] https://crrev.com/07856a7b8add0eba66d71fc62b5e32be9bc03d69/base/debug/thread_heap_usage_tracker.h
[add] https://crrev.com/07856a7b8add0eba66d71fc62b5e32be9bc03d69/base/debug/thread_heap_usage_tracker_unittest.cc

Comment 3 by siggi@chromium.org, Nov 29 2016

Blockedon: 669593
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 2 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/de38d0cc1f554562dc9062c97079585cc6522fe7

commit de38d0cc1f554562dc9062c97079585cc6522fe7
Author: siggi <siggi@chromium.org>
Date: Fri Dec 02 20:04:21 2016

Add heap allocator usage to task profiler.

This enables heap tracking in the task profiler (chrome://profiler) under a build flag, which is enabled for developer builds, but disabled for official builds for now.

As-is, this surfaces the following heap metrics per task by default:
- Avg number of allocations per run.
- Avg number of frees per run.
- Avg net bytes allocated (or freed if negative).
- Max allocated bytes at any time in any run.
  This is a high-watermark for memory usage in the task.

Additionally it's possible to enable the following metrics by ticking a checkbox in chrome://profiler:
- Allocation count.
- Free count.
  A running count of how many allocations and frees a task
  has performed in all runs.
- Allocated bytes.
- Freed bytes.
  A running tally of how many bytes a task has allocated and
  freed in all runs.
- Overhead bytes.
  An estimate of how many allocated bytes go to heap overhead.
  This might be helpful to guide implementation to use larger
  allocations or chunked data structures like std::vector

BUG= 644385 

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2386123003
Cr-Commit-Position: refs/heads/master@{#436003}

[modify] https://crrev.com/de38d0cc1f554562dc9062c97079585cc6522fe7/base/BUILD.gn
[modify] https://crrev.com/de38d0cc1f554562dc9062c97079585cc6522fe7/base/debug/thread_heap_usage_tracker.cc
[modify] https://crrev.com/de38d0cc1f554562dc9062c97079585cc6522fe7/base/tracked_objects.cc
[modify] https://crrev.com/de38d0cc1f554562dc9062c97079585cc6522fe7/base/tracked_objects.h
[modify] https://crrev.com/de38d0cc1f554562dc9062c97079585cc6522fe7/base/tracked_objects_unittest.cc
[modify] https://crrev.com/de38d0cc1f554562dc9062c97079585cc6522fe7/chrome/browser/resources/profiler/profiler.html
[modify] https://crrev.com/de38d0cc1f554562dc9062c97079585cc6522fe7/chrome/browser/resources/profiler/profiler.js
[modify] https://crrev.com/de38d0cc1f554562dc9062c97079585cc6522fe7/chrome/browser/task_profiler/task_profiler_data_serializer.cc
[modify] https://crrev.com/de38d0cc1f554562dc9062c97079585cc6522fe7/chrome/browser/task_profiler/task_profiler_data_serializer_unittest.cc
[modify] https://crrev.com/de38d0cc1f554562dc9062c97079585cc6522fe7/chrome/browser/ui/webui/profiler_ui.cc
[modify] https://crrev.com/de38d0cc1f554562dc9062c97079585cc6522fe7/content/common/child_process_messages.h

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/56b333c5957668b0b572f321b9487a1fba2edb5d

commit 56b333c5957668b0b572f321b9487a1fba2edb5d
Author: siggi <siggi@chromium.org>
Date: Wed Dec 07 17:04:33 2016

Add a documentation link to the chrome://profiler page.

BUG= 644385 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2555493005
Cr-Commit-Position: refs/heads/master@{#436985}

[modify] https://crrev.com/56b333c5957668b0b572f321b9487a1fba2edb5d/chrome/browser/resources/profiler/profiler.html

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/46cb2b79062956cce4cb142a3a07f358159d3555

commit 46cb2b79062956cce4cb142a3a07f358159d3555
Author: siggi <siggi@chromium.org>
Date: Wed Dec 07 22:21:53 2016

Enable memory profiling in SyzyASAN builds.

This should expose any outstanding bugs without upsetting perf metrics,
as SyzyASAN builds are excluded from perf scrutiny.

BUG= 644385 

Review-Url: https://codereview.chromium.org/2559833002
Cr-Commit-Position: refs/heads/master@{#437087}

[modify] https://crrev.com/46cb2b79062956cce4cb142a3a07f358159d3555/base/BUILD.gn

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e9ffc9c6104f4cc7ff5e62a94869f111c645106a

commit e9ffc9c6104f4cc7ff5e62a94869f111c645106a
Author: primiano <primiano@chromium.org>
Date: Mon Feb 06 19:52:44 2017

profiler: decouple ThreadHeapUsageTracker from allocator shim internals

Background:
-----------
Until crrev.com/2658723007, the allocator shim was initialized at
compile time and hence always ready to be used. After that CL,
the shim requires a run-time initialization. The code in
ThreadHeapUsageTracker currently assumes that the shim is initialized
by directly calling into the shim chain even before a malloc/free call
is intercepted.
There are two possible solutions to this:
1. Initialize the shim before ThreadHeapUsageTracker is initialized.
2. Prevent ThreadHeapUsageTracker from directly entering the shim.
(more discussion in https://codereview.chromium.org/2658723007/#msg53).

1. is a one liner but not particularly clean. ThreadHeapUsageTracker is
lazily initialized when a Thread (the main thread) is created (in the
TrackedObject construction). This would mean, in practice, putting
the shim initialization in tracked_objects.cc which is quite obscure.

This CL:
--------
The approach used here is 2. The existing code had already the
right sentinel logic to detect and prevent re-entrancy. This CL
is just extending that to the dtor and just using new/delete to
create the TLS object.
This allows to initialize the shim in a less obscure place (e.g.
content_main_runner).

BUG= 665567 , 644385 
TEST=Build with use_experimental_allocator_shim=true on Mac, run base_unittests

Review-Url: https://codereview.chromium.org/2675433002
Cr-Commit-Position: refs/heads/master@{#448360}

[modify] https://crrev.com/e9ffc9c6104f4cc7ff5e62a94869f111c645106a/base/debug/thread_heap_usage_tracker.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/97c2faae478f944add1951dc0cdcb5556e1d60b4

commit 97c2faae478f944add1951dc0cdcb5556e1d60b4
Author: siggi <siggi@chromium.org>
Date: Sun Feb 19 22:00:11 2017

Enable the memory task profiler in all builds where the heap shim is available.

BUG= 644385 

Review-Url: https://codereview.chromium.org/2699833006
Cr-Commit-Position: refs/heads/master@{#451527}

[modify] https://crrev.com/97c2faae478f944add1951dc0cdcb5556e1d60b4/base/BUILD.gn

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3ee99d6d523ef55317ad8e33e0bc21550dfdf053

commit 3ee99d6d523ef55317ad8e33e0bc21550dfdf053
Author: siggi <siggi@chromium.org>
Date: Tue Feb 21 17:07:17 2017

chrome://profiler: Only show memory metrics in when heap tracking is enabled.

BUG= 644385 

Review-Url: https://codereview.chromium.org/2705243002
Cr-Commit-Position: refs/heads/master@{#451783}

[modify] https://crrev.com/3ee99d6d523ef55317ad8e33e0bc21550dfdf053/chrome/browser/ui/webui/profiler_ui.cc

Project Member

Comment 10 by bugdroid1@chromium.org, May 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f467a630a42f536db8a502d820b83266c4342f6b

commit f467a630a42f536db8a502d820b83266c4342f6b
Author: siggi <siggi@chromium.org>
Date: Wed May 03 21:53:14 2017

Tracked objects: Bump cumulative byte count storage to 64 bits to avoid saturation.

Since Chrome churns heap memory at a prodigious rate, it only takes an hour or two for the 32 bit byte counts to saturate. After saturation occurs, the derived memory metrics described in https://sites.google.com/a/chromium.org/dev/developers/threaded-task-tracking run off the rails and become useless.

BUG= 644385 

Review-Url: https://codereview.chromium.org/2859493002
Cr-Commit-Position: refs/heads/master@{#469145}

[modify] https://crrev.com/f467a630a42f536db8a502d820b83266c4342f6b/base/tracked_objects.cc
[modify] https://crrev.com/f467a630a42f536db8a502d820b83266c4342f6b/base/tracked_objects.h
[modify] https://crrev.com/f467a630a42f536db8a502d820b83266c4342f6b/base/tracked_objects_unittest.cc

Comment 11 by siggi@chromium.org, May 12 2017

Cc: fdoray@chromium.org
The byte count sums are not matching up, even with the saturation and slicing alleviated. Francois hypothesizes that the IO thread is allocating a bunch of memory outside this instrumentation, which then gets passed to tasks etc. Window message handlers are likely also outside instrumentation.
Project Member

Comment 12 by bugdroid1@chromium.org, May 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/03ce267472a3f16830796d5bd04d79371d61afaf

commit 03ce267472a3f16830796d5bd04d79371d61afaf
Author: siggi <siggi@chromium.org>
Date: Fri May 12 17:46:40 2017

Marshal 64 bit byte counts as double through to JavaScript.

As-is, byte counts larger than 31 bits are getting truncated and/or
mis-interpreted as negative numbers, which does no good on the summing.

Also explicitly test the transitions from <31 bit to >31 bit to >32 bits
which I originally suspected of causing the negative aggregates.

BUG= 644385 
TBR=jochen@chromium.org

Review-Url: https://codereview.chromium.org/2881493004
Cr-Commit-Position: refs/heads/master@{#471356}

[modify] https://crrev.com/03ce267472a3f16830796d5bd04d79371d61afaf/base/tracked_objects_unittest.cc
[modify] https://crrev.com/03ce267472a3f16830796d5bd04d79371d61afaf/chrome/browser/task_profiler/task_profiler_data_serializer.cc
[modify] https://crrev.com/03ce267472a3f16830796d5bd04d79371d61afaf/chrome/browser/task_profiler/task_profiler_data_serializer_unittest.cc

Project Member

Comment 13 by bugdroid1@chromium.org, May 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/21a0c8f770ea811a3830d5f8c6447306a686a63b

commit 21a0c8f770ea811a3830d5f8c6447306a686a63b
Author: siggi <siggi@chromium.org>
Date: Tue May 16 17:15:15 2017

Make window message processing tally in the task profiler.

As-is the processing and memory allocations taking place during window
message processing is invisible to the task profiler.

BUG= 644385 

Review-Url: https://codereview.chromium.org/2881943002
Cr-Commit-Position: refs/heads/master@{#472142}

[modify] https://crrev.com/21a0c8f770ea811a3830d5f8c6447306a686a63b/base/win/wrapped_window_proc.h

Comment 14 by siggi@chromium.org, Jan 11 2018

Status: WontFix (was: Started)
This wasn't nearly as useful as I thought it would be, and the Task Profiler has now been retired.

Sign in to add a comment