New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 917476 link

Starred by 2 users
Status: Started
Owner:
alph@chromium.org
Cc:
etienneb@chromium.org
ssid@chromium.org
erikc...@chromium.org
█ picksi@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Task
Hotlist-MemoryInfra



Sign in to add a comment

Remove non-sampling mode of Chrome heap profiler

Project Member Reported by alph@chromium.org, Dec 21 
Non-sampling (full instrumentation) mode has been introduced a while ago. Since then we have added sampling that is strictly better than the full instrumentation.

Namely it has less overhead, while providing the same level of actionable information. Moreover the precision vs. overhead trade-off can be controlled with the sampling interval parameter.

It is still technically possible to collect full profile by setting the sampling interval to an extremely low value, e.g. 1 byte.
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 7 

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

commit 89fbc0a325f9c4208702a115b723c56efeca6965
Author: Alexei Filippov <alph@chromium.org>
Date: Mon Jan 07 22:58:19 2019

[heap profiler] Make sampling v2 default and remove v1 code.

BUG=917476

Change-Id: I129a9d3af71d239c6be8c117c1c3fbbfe5d86072
Reviewed-on: https://chromium-review.googlesource.com/c/1391356
Commit-Queue: Alexei Filippov <alph@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620515}
[modify] https://crrev.com/89fbc0a325f9c4208702a115b723c56efeca6965/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/89fbc0a325f9c4208702a115b723c56efeca6965/components/services/heap_profiling/public/cpp/allocator_shim.cc
[modify] https://crrev.com/89fbc0a325f9c4208702a115b723c56efeca6965/components/services/heap_profiling/public/cpp/allocator_shim.h
[modify] https://crrev.com/89fbc0a325f9c4208702a115b723c56efeca6965/components/services/heap_profiling/public/cpp/client.cc
[modify] https://crrev.com/89fbc0a325f9c4208702a115b723c56efeca6965/components/services/heap_profiling/public/cpp/sampling_profiler_wrapper.cc
[modify] https://crrev.com/89fbc0a325f9c4208702a115b723c56efeca6965/components/services/heap_profiling/public/cpp/sampling_profiler_wrapper.h
[modify] https://crrev.com/89fbc0a325f9c4208702a115b723c56efeca6965/components/services/heap_profiling/public/cpp/settings.cc
[modify] https://crrev.com/89fbc0a325f9c4208702a115b723c56efeca6965/components/services/heap_profiling/public/cpp/settings.h
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 8 

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

commit 67bf4f720d2826c54fcdba595c20d1b0c1adf60f
Author: Thiemo Nagel <tnagel@chromium.org>
Date: Tue Jan 08 10:47:00 2019

Revert "[heap profiler] Make sampling v2 default and remove v1 code."

This reverts commit 89fbc0a325f9c4208702a115b723c56efeca6965.

Reason for revert: MemlogBrowserTest has started crashing on Mac10.12

This is a speculative revert. (Your CL seems to be the only one on the blamelist that mentions profiling.) I apologize in advance if my suspicion turns out to be unsubstantiated.

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.12%20Tests/18408

[ RUN      ] Memlog/MemlogBrowserTest.EndToEnd/18
[79940:4099:0107/172123.369006:WARNING:notification_platform_bridge_mac.mm(521)] AlertNotificationService: XPC connection invalidated.
[79940:775:0107/172123.978820:INFO:memlog_browsertest.cc(87)] Memlog mode: 3
[79940:775:0107/172123.978872:INFO:memlog_browsertest.cc(88)] Memlog stack mode: 0
[79940:775:0107/172123.978889:INFO:memlog_browsertest.cc(89)] Started via command line flag: 0
[79940:775:0107/172123.978911:INFO:memlog_browsertest.cc(91)] Should sample: 1
[79940:775:0107/172123.978924:INFO:memlog_browsertest.cc(92)] Sample everything: 1
[79940:775:0107/172124.276612:WARNING:test_driver.cc(323)] Allocation candidate (size:1241399 count:157)
Received signal 11 SEGV_MAPERR 000000000069

Original change's description:
> [heap profiler] Make sampling v2 default and remove v1 code.
> 
> BUG=917476
> 
> Change-Id: I129a9d3af71d239c6be8c117c1c3fbbfe5d86072
> Reviewed-on: https://chromium-review.googlesource.com/c/1391356
> Commit-Queue: Alexei Filippov <alph@chromium.org>
> Reviewed-by: Erik Chen <erikchen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#620515}

TBR=alph@chromium.org,erikchen@chromium.org

Change-Id: I157cb9ac2899e10a445e0ab9004924d282b87d34
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 917476
Reviewed-on: https://chromium-review.googlesource.com/c/1400662
Reviewed-by: Thiemo Nagel <tnagel@chromium.org>
Commit-Queue: Thiemo Nagel <tnagel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620671}
[modify] https://crrev.com/67bf4f720d2826c54fcdba595c20d1b0c1adf60f/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/67bf4f720d2826c54fcdba595c20d1b0c1adf60f/components/services/heap_profiling/public/cpp/allocator_shim.cc
[modify] https://crrev.com/67bf4f720d2826c54fcdba595c20d1b0c1adf60f/components/services/heap_profiling/public/cpp/allocator_shim.h
[modify] https://crrev.com/67bf4f720d2826c54fcdba595c20d1b0c1adf60f/components/services/heap_profiling/public/cpp/client.cc
[modify] https://crrev.com/67bf4f720d2826c54fcdba595c20d1b0c1adf60f/components/services/heap_profiling/public/cpp/sampling_profiler_wrapper.cc
[modify] https://crrev.com/67bf4f720d2826c54fcdba595c20d1b0c1adf60f/components/services/heap_profiling/public/cpp/sampling_profiler_wrapper.h
[modify] https://crrev.com/67bf4f720d2826c54fcdba595c20d1b0c1adf60f/components/services/heap_profiling/public/cpp/settings.cc
[modify] https://crrev.com/67bf4f720d2826c54fcdba595c20d1b0c1adf60f/components/services/heap_profiling/public/cpp/settings.h
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 9 

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

commit 85e0a26e197fc890afe9aaa387a46d04bef70f4f
Author: Alexei Filippov <alph@chromium.org>
Date: Wed Jan 09 02:37:16 2019

[heap profiler] Do not try to record allocations with NULL address.

Sometimes RecordAlloc is called with NULL address. This is probably
caused by failed allocations. Do not try to record a sample in this
case. The hash set does not allow zero keys anyway.

BUG=917476

Change-Id: Ie734c4343d8def7036ac670beb571b4d369490ae
Reviewed-on: https://chromium-review.googlesource.com/c/1400819
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621009}
[modify] https://crrev.com/85e0a26e197fc890afe9aaa387a46d04bef70f4f/base/sampling_heap_profiler/poisson_allocation_sampler.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 9 

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

commit 4666c5b9bc43aca528babd63deb5a936c9c189ff
Author: Alexei Filippov <alph@chromium.org>
Date: Wed Jan 09 19:31:09 2019

[heap profiler] Check if TLS has been destroyed before using it.

BUG=917476

Change-Id: Iba91affeb712ff5de7ceab62c19ec1f2b7e3d94a
Reviewed-on: https://chromium-review.googlesource.com/c/1401617
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621268}
[modify] https://crrev.com/4666c5b9bc43aca528babd63deb5a936c9c189ff/components/services/heap_profiling/public/cpp/allocator_shim.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 9 

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

commit 70a90c6a5bdd118f23664ad1047a3c3a65ee9f25
Author: Alexei Filippov <alph@chromium.org>
Date: Wed Jan 09 22:10:47 2019

Reland "[heap profiler] Make sampling v2 default and remove v1 code."

This is a reland of 89fbc0a325f9c4208702a115b723c56efeca6965

Original change's description:
> [heap profiler] Make sampling v2 default and remove v1 code.
>
> BUG=917476
>
> Change-Id: I129a9d3af71d239c6be8c117c1c3fbbfe5d86072
> Reviewed-on: https://chromium-review.googlesource.com/c/1391356
> Commit-Queue: Alexei Filippov <alph@chromium.org>
> Reviewed-by: Erik Chen <erikchen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#620515}

TBR=erikchen@chromium.org

Bug: 917476
Change-Id: I573006be775a75ffb2763fd1d9ffa12c6e540253
Reviewed-on: https://chromium-review.googlesource.com/c/1403683
Commit-Queue: Alexei Filippov <alph@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621316}
[modify] https://crrev.com/70a90c6a5bdd118f23664ad1047a3c3a65ee9f25/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/70a90c6a5bdd118f23664ad1047a3c3a65ee9f25/components/services/heap_profiling/public/cpp/allocator_shim.cc
[modify] https://crrev.com/70a90c6a5bdd118f23664ad1047a3c3a65ee9f25/components/services/heap_profiling/public/cpp/allocator_shim.h
[modify] https://crrev.com/70a90c6a5bdd118f23664ad1047a3c3a65ee9f25/components/services/heap_profiling/public/cpp/client.cc
[modify] https://crrev.com/70a90c6a5bdd118f23664ad1047a3c3a65ee9f25/components/services/heap_profiling/public/cpp/sampling_profiler_wrapper.cc
[modify] https://crrev.com/70a90c6a5bdd118f23664ad1047a3c3a65ee9f25/components/services/heap_profiling/public/cpp/sampling_profiler_wrapper.h
[modify] https://crrev.com/70a90c6a5bdd118f23664ad1047a3c3a65ee9f25/components/services/heap_profiling/public/cpp/settings.cc
[modify] https://crrev.com/70a90c6a5bdd118f23664ad1047a3c3a65ee9f25/components/services/heap_profiling/public/cpp/settings.h
►

Sign in to add a comment