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

Issue 700245 link

Starred by 3 users
Status: Untriaged
Owner: ----
Cc:
primiano@chromium.org
█ picksi@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug

Blocked on:
issue 698271
Blocking:
issue 698266



Sign in to add a comment

[memory-infra] Support per-process and per-component heap profiling

Project Member Reported by ssid@chromium.org, Mar 10 2017 
Background context: go/memory-infra

For being able to enable heap profiler in the field there should be a way to enable heap profiler only on specific processes. The process choice could be random renderer process or browser or gpu. We should also be able to specify which dump providers should enable heap profiling: malloc or blink.
These should be configured by either the command line flag options or maybe directly set by background tracing config.

Comment 1 by ssid@chromium.org, Mar 10 2017 

Marking it blocked on 698271 since we probably could use similar feature to turn on heap profiler in background mode also.
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 24 2017 

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

commit 114dc8babbde99d54f3f4f41246a19bb1a4c1605
Author: ssid <ssid@chromium.org>
Date: Fri Mar 24 21:29:19 2017

[tracing] Use same code path for category filtering for recording and event filtering

Previously there was not way to filter just default categories using the
category filtering logic of event filters. This CL refactors the
category filtering logic out of TraceConfig and both EventFilterConfig
and TraceConfig uses this new class.

BUG=700245

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

[modify] https://crrev.com/114dc8babbde99d54f3f4f41246a19bb1a4c1605/base/BUILD.gn
[modify] https://crrev.com/114dc8babbde99d54f3f4f41246a19bb1a4c1605/base/trace_event/heap_profiler_allocation_context_tracker_unittest.cc
[modify] https://crrev.com/114dc8babbde99d54f3f4f41246a19bb1a4c1605/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/114dc8babbde99d54f3f4f41246a19bb1a4c1605/base/trace_event/trace_config.cc
[modify] https://crrev.com/114dc8babbde99d54f3f4f41246a19bb1a4c1605/base/trace_event/trace_config.h
[add] https://crrev.com/114dc8babbde99d54f3f4f41246a19bb1a4c1605/base/trace_event/trace_config_category_filter.cc
[add] https://crrev.com/114dc8babbde99d54f3f4f41246a19bb1a4c1605/base/trace_event/trace_config_category_filter.h
[modify] https://crrev.com/114dc8babbde99d54f3f4f41246a19bb1a4c1605/base/trace_event/trace_config_unittest.cc
[modify] https://crrev.com/114dc8babbde99d54f3f4f41246a19bb1a4c1605/base/trace_event/trace_event_unittest.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 29 2017 

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

commit 88d07c763d35621da2c62b9f18822109bbd7a261
Author: kolos <kolos@chromium.org>
Date: Wed Mar 29 10:44:32 2017

Revert of Add EnabledStateObserver to BackgroundTracingManager (patchset #3 id:130001 of https://codereview.chromium.org/2769963005/ )

Reason for revert:
This CL causes tests failures (browser_side_navigation_content_browsertests, content_browsertests, site_per_process_content_browsertests).

https://findit-for-me.appspot.com/waterfall/build-failure?url=https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/24171

Original issue's description:
> Add EnabledStateObserver to BackgroundTracingManager
>
> This CL removes the memory-infra related code into memory-tracing
> observer. Also the tests implement the observer instead of setting
> callbacks.
>
> BUG=700245
>
> Review-Url: https://codereview.chromium.org/2769963005
> Cr-Commit-Position: refs/heads/master@{#460293}
> Committed: https://chromium.googlesource.com/chromium/src/+/f64ac5cb273e3abab3564f1a5997b3da164ab9ff

TBR=oysteine@chromium.org,alexmos@chromium.org,ssid@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=700245

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

[modify] https://crrev.com/88d07c763d35621da2c62b9f18822109bbd7a261/content/browser/BUILD.gn
[delete] https://crrev.com/1690551c93020ef0ebee1e0a1a523c089ce3d62f/content/browser/tracing/background_memory_tracing_observer.cc
[delete] https://crrev.com/1690551c93020ef0ebee1e0a1a523c089ce3d62f/content/browser/tracing/background_memory_tracing_observer.h
[modify] https://crrev.com/88d07c763d35621da2c62b9f18822109bbd7a261/content/browser/tracing/background_tracing_manager_browsertest.cc
[modify] https://crrev.com/88d07c763d35621da2c62b9f18822109bbd7a261/content/browser/tracing/background_tracing_manager_impl.cc
[modify] https://crrev.com/88d07c763d35621da2c62b9f18822109bbd7a261/content/browser/tracing/background_tracing_manager_impl.h
[modify] https://crrev.com/88d07c763d35621da2c62b9f18822109bbd7a261/content/public/browser/background_tracing_manager.h
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 29 2017 

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

commit eb549628eb3e134f66bebea123a27d89dcf43dbb
Author: ssid <ssid@chromium.org>
Date: Wed Mar 29 21:46:00 2017

Reland of Add EnabledStateObserver to BackgroundTracingManager (patchset #1 id:1 of https://codereview.chromium.org/2785663002/ )

Reason for revert:
|config_| can be null. So, do not dereference it.

Original issue's description:
> Revert of Add EnabledStateObserver to BackgroundTracingManager (patchset #3 id:130001 of https://codereview.chromium.org/2769963005/ )
>
> Reason for revert:
> This CL causes tests failures (browser_side_navigation_content_browsertests, content_browsertests, site_per_process_content_browsertests).
>
> https://findit-for-me.appspot.com/waterfall/build-failure?url=https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/24171
>
> Original issue's description:
> > Add EnabledStateObserver to BackgroundTracingManager
> >
> > This CL removes the memory-infra related code into memory-tracing
> > observer. Also the tests implement the observer instead of setting
> > callbacks.
> >
> > BUG=700245
> >
> > Review-Url: https://codereview.chromium.org/2769963005
> > Cr-Commit-Position: refs/heads/master@{#460293}
> > Committed: https://chromium.googlesource.com/chromium/src/+/f64ac5cb273e3abab3564f1a5997b3da164ab9ff
>
> TBR=oysteine@chromium.org,alexmos@chromium.org,ssid@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=700245
>
> Review-Url: https://codereview.chromium.org/2785663002
> Cr-Commit-Position: refs/heads/master@{#460343}
> Committed: https://chromium.googlesource.com/chromium/src/+/88d07c763d35621da2c62b9f18822109bbd7a261

TBR=alexmos@chromium.org,kolos@chromium.org
BUG=700245

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

[modify] https://crrev.com/eb549628eb3e134f66bebea123a27d89dcf43dbb/content/browser/BUILD.gn
[add] https://crrev.com/eb549628eb3e134f66bebea123a27d89dcf43dbb/content/browser/tracing/background_memory_tracing_observer.cc
[add] https://crrev.com/eb549628eb3e134f66bebea123a27d89dcf43dbb/content/browser/tracing/background_memory_tracing_observer.h
[modify] https://crrev.com/eb549628eb3e134f66bebea123a27d89dcf43dbb/content/browser/tracing/background_tracing_manager_browsertest.cc
[modify] https://crrev.com/eb549628eb3e134f66bebea123a27d89dcf43dbb/content/browser/tracing/background_tracing_manager_impl.cc
[modify] https://crrev.com/eb549628eb3e134f66bebea123a27d89dcf43dbb/content/browser/tracing/background_tracing_manager_impl.h
[modify] https://crrev.com/eb549628eb3e134f66bebea123a27d89dcf43dbb/content/public/browser/background_tracing_manager.h

Comment 6 by primiano@chromium.org, May 10 2017 

Issue 717993 has been merged into this issue.
Project Member

Comment 7 by sheriffbot@chromium.org, May 14 2017

Labels: OS-Windows Fracas FoundIn-M-60
Users experienced this crash on the following builds:

Win Canary 60.0.3099.0 -  0.52 CPM, 3 reports, 2 clients (signature base::trace_event::internal::AllocateGuardedVirtualMemory)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas
Project Member

Comment 8 by sheriffbot@chromium.org, May 31 2017

Labels: FoundIn-M-61
Users experienced this crash on the following builds:

Win Canary 61.0.3115.0 -  0.46 CPM, 3 reports, 2 clients (signature base::trace_event::internal::AllocateGuardedVirtualMemory)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

Comment 9 by msrchandra@chromium.org, Jun 12 2017 

 Issue 731345  has been merged into this issue.
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 20 2017 

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

commit ab6ecd2501b200ba7dd674674a66f0170ed99cbc
Author: Siddhartha <ssid@chromium.org>
Date: Sun Aug 20 03:54:15 2017

[memory-infra] Introduce NO_STACK mode in heap profiling

This CL changes:
1. Add ability to enable heap profiling in process without command line
   flag, by calling MDM::EnableHeapProfiling.
2. Introduce NO_STACK mode in allocation context tracker. This mode
   records only thread names and task locations.
3. MDP::OnHeapProfilingEnabled() is not called for task profiler.

Bug: 700245
Change-Id: I50b6eda36cfe397c3c67208da99c5657cdb026b6
Reviewed-on: https://chromium-review.googlesource.com/617778
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Dmitry Skiba <dskiba@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495834}
[modify] https://crrev.com/ab6ecd2501b200ba7dd674674a66f0170ed99cbc/base/trace_event/heap_profiler_allocation_context_tracker.cc
[modify] https://crrev.com/ab6ecd2501b200ba7dd674674a66f0170ed99cbc/base/trace_event/heap_profiler_allocation_context_tracker.h
[modify] https://crrev.com/ab6ecd2501b200ba7dd674674a66f0170ed99cbc/base/trace_event/heap_profiler_allocation_context_tracker_unittest.cc
[modify] https://crrev.com/ab6ecd2501b200ba7dd674674a66f0170ed99cbc/base/trace_event/malloc_dump_provider.cc
[modify] https://crrev.com/ab6ecd2501b200ba7dd674674a66f0170ed99cbc/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/ab6ecd2501b200ba7dd674674a66f0170ed99cbc/base/trace_event/memory_dump_manager.h
[modify] https://crrev.com/ab6ecd2501b200ba7dd674674a66f0170ed99cbc/base/trace_event/memory_dump_manager_unittest.cc
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 22 2017 

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

commit 24ef3c47fa011acb9abc56f6dcc879370584cef7
Author: Siddhartha <ssid@chromium.org>
Date: Tue Aug 22 00:19:20 2017

Introduce config for enabling heap profiler in background tracing

This CL introduces args in background tracing rule for being able to
enable heap profiler from finch config. Also add "args" dictionary to
config rule.

BUG=700245

Change-Id: I93b73c628314369109070df177aff31e6e9ab517
Reviewed-on: https://chromium-review.googlesource.com/619611
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Oystein Eftevaag <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496138}
[modify] https://crrev.com/24ef3c47fa011acb9abc56f6dcc879370584cef7/content/browser/tracing/background_memory_tracing_observer.cc
[modify] https://crrev.com/24ef3c47fa011acb9abc56f6dcc879370584cef7/content/browser/tracing/background_memory_tracing_observer.h
[modify] https://crrev.com/24ef3c47fa011acb9abc56f6dcc879370584cef7/content/browser/tracing/background_tracing_manager_browsertest.cc
[modify] https://crrev.com/24ef3c47fa011acb9abc56f6dcc879370584cef7/content/browser/tracing/background_tracing_manager_impl.cc
[modify] https://crrev.com/24ef3c47fa011acb9abc56f6dcc879370584cef7/content/browser/tracing/background_tracing_manager_impl.h
[modify] https://crrev.com/24ef3c47fa011acb9abc56f6dcc879370584cef7/content/browser/tracing/background_tracing_rule.cc
[modify] https://crrev.com/24ef3c47fa011acb9abc56f6dcc879370584cef7/content/browser/tracing/background_tracing_rule.h
Project Member

Comment 12 by bugdroid1@chromium.org, Sep 6 2017 

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

commit 1e94989bc10b79261d67ccfeaf0e7e6308733747
Author: Siddhartha <ssid@chromium.org>
Date: Wed Sep 06 23:52:15 2017

Memory-infra: Heap profiler can be enabled when tracing

This is needed because new process starting when tracing and heap
profiler are enabled can get heap profiling notification later than
tracing enabled notification.
The heap profiler state should be setup when both tracing and heap
profiler are enabled. Do not call EnableHeapprofilingIfNeeded() from
constructor since the pseudo stack profiling needs to enable filtering
on TraceLog and MDM could be constructed by TraceLog in case of startup
tracing. This does not miss allocations since the heap profiling is
enabled by EnableStartupTracingIfNeeded very early (ContentMainRunner on
desktop and library loader on Android).

BUG= 757747 , 700245, 625170

Change-Id: I6fc593838dc1fa1c8450addefec782dd37ae9e5f
Reviewed-on: https://chromium-review.googlesource.com/651908
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500138}
[modify] https://crrev.com/1e94989bc10b79261d67ccfeaf0e7e6308733747/base/trace_event/heap_profiler_serialization_state.h
[modify] https://crrev.com/1e94989bc10b79261d67ccfeaf0e7e6308733747/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/1e94989bc10b79261d67ccfeaf0e7e6308733747/base/trace_event/memory_dump_manager.h
[modify] https://crrev.com/1e94989bc10b79261d67ccfeaf0e7e6308733747/base/trace_event/memory_dump_manager_unittest.cc
[modify] https://crrev.com/1e94989bc10b79261d67ccfeaf0e7e6308733747/base/trace_event/process_memory_dump.cc
Project Member

Comment 13 by bugdroid1@chromium.org, Sep 12 2017 

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

commit 698ff7dd46abdc8734c4598f1757b6e1073b6f2a
Author: Siddhartha <ssid@chromium.org>
Date: Tue Sep 12 23:04:28 2017

Memory-infra: Add browser tests for heap profiling

1. Adds browser tests for heap profiling with pseudo and no stack mode.
2. TraceLog does not dcheck if filters are added multiple times.
Reason: The EnableHeapProfiling() mojo message from browser to renderer
is sent before OnBeginTracing IPC message. The tracing is being
converted to service and maybe we will have better fix in future.
3. Whitelist heap profiling overhead dumps for background mode.

BUG=700245

Change-Id: I73289e4f7ec5de1421ac44e87e7c30d363869b5b
Reviewed-on: https://chromium-review.googlesource.com/653782
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501439}
[modify] https://crrev.com/698ff7dd46abdc8734c4598f1757b6e1073b6f2a/base/trace_event/memory_infra_background_whitelist.cc
[modify] https://crrev.com/698ff7dd46abdc8734c4598f1757b6e1073b6f2a/base/trace_event/trace_log.cc
[modify] https://crrev.com/698ff7dd46abdc8734c4598f1757b6e1073b6f2a/chrome/test/base/memory_tracing_browsertest.cc
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 5 2017 

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

commit af517105b92b50274ce33f85933bfbd430f93b40
Author: Siddhartha <ssid@chromium.org>
Date: Thu Oct 05 07:56:29 2017

Whitelist tracing metadata events for heap profiling

Bug: 700245
Change-Id: Id316cb680cecc5755ab52026ccff790e12d1d133
Reviewed-on: https://chromium-review.googlesource.com/702019
Reviewed-by: oysteine <oysteine@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506675}
[modify] https://crrev.com/af517105b92b50274ce33f85933bfbd430f93b40/chrome/common/trace_event_args_whitelist.cc
Project Member

Comment 15 by bugdroid1@chromium.org, Oct 14 2017 

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

commit 1d1c59a1befcdb14ece11ea532ffcf727cc3571b
Author: Siddhartha <ssid@chromium.org>
Date: Sat Oct 14 01:27:29 2017

Slow-reports: Do not disable heap profiling after first dump

We actaully capture multiple traces from the same user. It is harder to
find the first trace with heap dumps. So, keep profiling enabled till
scenario is aborted.

BUG=700245

Change-Id: I0e490da52439cc051e6d038a7dc7634e6056c546
Reviewed-on: https://chromium-review.googlesource.com/710743
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508903}
[modify] https://crrev.com/1d1c59a1befcdb14ece11ea532ffcf727cc3571b/content/browser/tracing/background_memory_tracing_observer.cc
Project Member

Comment 16 by bugdroid1@chromium.org, Jan 4 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/95f3b4dbddc9c6d5b56d6179579308320800ba81

commit 95f3b4dbddc9c6d5b56d6179579308320800ba81
Author: Siddhartha <ssid@chromium.org>
Date: Thu Jan 04 22:01:25 2018

Do not fail trace processing if trace config is stripped

Traces uploaded in slow reports have trace config stripped. JSON.Parse
throws error if the config is missing. Handle this error.

BUG=chromium:700245
Change-Id: Ib38aa67aeaad04214d992e607dc6c774a5243157
Reviewed-on: https://chromium-review.googlesource.com/828699
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Reviewed-by: oysteine <oysteine@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>

[modify] https://crrev.com/95f3b4dbddc9c6d5b56d6179579308320800ba81/tracing/tracing/metrics/metric_map_function_test.html
[modify] https://crrev.com/95f3b4dbddc9c6d5b56d6179579308320800ba81/tracing/tracing/metrics/metric_map_function.html
Project Member

Comment 17 by bugdroid1@chromium.org, Jan 5 2018 

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

commit 5efa13548a31dde80fba046c98aa66589bba24c3
Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org>
Date: Fri Jan 05 00:57:58 2018

Roll src/third_party/catapult/ 634b18ca3..95f3b4dbd (1 commit)

https://chromium.googlesource.com/catapult.git/+log/634b18ca3700..95f3b4dbddc9

$ git log 634b18ca3..95f3b4dbd --date=short --no-merges --format='%ad %ae %s'
2018-01-04 ssid Do not fail trace processing if trace config is stripped

Created with:
  roll-dep src/third_party/catapult
BUG=700245


The AutoRoll server is located here: https://catapult-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=sullivan@chromium.org

Change-Id: I3647d44e48e372a71a63de07148f5a56ecbf0b49
Reviewed-on: https://chromium-review.googlesource.com/851334
Reviewed-by: <catapult-deps-roller@chromium.org>
Commit-Queue: <catapult-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527178}
[modify] https://crrev.com/5efa13548a31dde80fba046c98aa66589bba24c3/DEPS
►

Sign in to add a comment