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

Issue 808588 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

Improve UMA sampling profiler continuous collection

Project Member Reported by wittman@chromium.org, Feb 2 2018

Issue description

Several issues exist with the initial continuous collection implementation:

- it's not extensible to non-browser processes
- it's tied to the startup sampling in an awkward way
- it sends more samples than we need for adequate analysis
- the 1Hz sampling rate is not ideal for analysis
- configuring it is fairly complicated

We should address these issues so that we can deploy the profiler for continuous collection more easily in more scenarios.
 
Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 7 2018

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

commit aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c
Author: Mike Wittman <wittman@chromium.org>
Date: Wed Feb 07 20:18:28 2018

Encapsulate Chrome thread profiling setup into a single class

Creates a simple abstraction around the fairly complex profiling setup.
This CL switches over the GPU and renderer process usage to this class.
The browser process usage will be converted to use this class after it
is extended to support periodic profiling (in a follow-on CL).

Bug:  808588 
Change-Id: I6195571bf3ddd08291f288f475569bd9a88795ef
Reviewed-on: https://chromium-review.googlesource.com/905451
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535125}
[modify] https://crrev.com/aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c/chrome/common/BUILD.gn
[modify] https://crrev.com/aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c/chrome/common/DEPS
[add] https://crrev.com/aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c/chrome/common/thread_profiler.cc
[add] https://crrev.com/aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c/chrome/common/thread_profiler.h
[modify] https://crrev.com/aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c/chrome/gpu/chrome_content_gpu_client.cc
[modify] https://crrev.com/aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c/chrome/gpu/chrome_content_gpu_client.h
[modify] https://crrev.com/aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c/components/metrics/BUILD.gn
[modify] https://crrev.com/aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c/components/metrics/call_stack_profile_metrics_provider.cc
[modify] https://crrev.com/aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c/components/metrics/call_stack_profile_metrics_provider.h
[modify] https://crrev.com/aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c/components/metrics/child_call_stack_profile_collector.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 7 2018

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

commit addede59555ab6c04cc072a2ab59cb11066c3a27
Author: Mike Wittman <wittman@chromium.org>
Date: Wed Feb 07 20:20:52 2018

Revert "Encapsulate Chrome thread profiling setup into a single class"

This reverts commit aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c.

Reason for revert: MSVC build is broken in the latest patchset

Original change's description:
> Encapsulate Chrome thread profiling setup into a single class
> 
> Creates a simple abstraction around the fairly complex profiling setup.
> This CL switches over the GPU and renderer process usage to this class.
> The browser process usage will be converted to use this class after it
> is extended to support periodic profiling (in a follow-on CL).
> 
> Bug:  808588 
> Change-Id: I6195571bf3ddd08291f288f475569bd9a88795ef
> Reviewed-on: https://chromium-review.googlesource.com/905451
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> Commit-Queue: Mike Wittman <wittman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#535125}

TBR=sky@chromium.org,wittman@chromium.org,asvitkine@chromium.org

Change-Id: Ib8433a51b288fb5613138a78803deb859c58a403
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  808588 
Reviewed-on: https://chromium-review.googlesource.com/907335
Reviewed-by: Mike Wittman <wittman@chromium.org>
Commit-Queue: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535127}
[modify] https://crrev.com/addede59555ab6c04cc072a2ab59cb11066c3a27/chrome/common/BUILD.gn
[modify] https://crrev.com/addede59555ab6c04cc072a2ab59cb11066c3a27/chrome/common/DEPS
[delete] https://crrev.com/df8926bba217b1d5bcf7e004b3767b035b9fe798/chrome/common/thread_profiler.cc
[delete] https://crrev.com/df8926bba217b1d5bcf7e004b3767b035b9fe798/chrome/common/thread_profiler.h
[modify] https://crrev.com/addede59555ab6c04cc072a2ab59cb11066c3a27/chrome/gpu/chrome_content_gpu_client.cc
[modify] https://crrev.com/addede59555ab6c04cc072a2ab59cb11066c3a27/chrome/gpu/chrome_content_gpu_client.h
[modify] https://crrev.com/addede59555ab6c04cc072a2ab59cb11066c3a27/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/addede59555ab6c04cc072a2ab59cb11066c3a27/components/metrics/BUILD.gn
[modify] https://crrev.com/addede59555ab6c04cc072a2ab59cb11066c3a27/components/metrics/call_stack_profile_metrics_provider.cc
[modify] https://crrev.com/addede59555ab6c04cc072a2ab59cb11066c3a27/components/metrics/call_stack_profile_metrics_provider.h
[modify] https://crrev.com/addede59555ab6c04cc072a2ab59cb11066c3a27/components/metrics/child_call_stack_profile_collector.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 7 2018

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

commit f8ad6e3ef816b7a971929637769230672bab8b11
Author: Mike Wittman <wittman@chromium.org>
Date: Wed Feb 07 22:46:23 2018

Reland "Encapsulate Chrome thread profiling setup into a single class"

This is a reland of aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c.

Original change's description:
> Encapsulate Chrome thread profiling setup into a single class
>
> Creates a simple abstraction around the fairly complex profiling setup.
> This CL switches over the GPU and renderer process usage to this class.
> The browser process usage will be converted to use this class after it
> is extended to support periodic profiling (in a follow-on CL).
>
> Bug:  808588 
> Change-Id: I6195571bf3ddd08291f288f475569bd9a88795ef
> Reviewed-on: https://chromium-review.googlesource.com/905451
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> Commit-Queue: Mike Wittman <wittman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#535125}

TBR=sky

Bug:  808588 
Change-Id: I34f1910116c4e7a500e353c27b85bf7534be43c7
Reviewed-on: https://chromium-review.googlesource.com/907508
Reviewed-by: Mike Wittman <wittman@chromium.org>
Commit-Queue: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535170}
[modify] https://crrev.com/f8ad6e3ef816b7a971929637769230672bab8b11/chrome/common/BUILD.gn
[modify] https://crrev.com/f8ad6e3ef816b7a971929637769230672bab8b11/chrome/common/DEPS
[add] https://crrev.com/f8ad6e3ef816b7a971929637769230672bab8b11/chrome/common/thread_profiler.cc
[add] https://crrev.com/f8ad6e3ef816b7a971929637769230672bab8b11/chrome/common/thread_profiler.h
[modify] https://crrev.com/f8ad6e3ef816b7a971929637769230672bab8b11/chrome/gpu/chrome_content_gpu_client.cc
[modify] https://crrev.com/f8ad6e3ef816b7a971929637769230672bab8b11/chrome/gpu/chrome_content_gpu_client.h
[modify] https://crrev.com/f8ad6e3ef816b7a971929637769230672bab8b11/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/f8ad6e3ef816b7a971929637769230672bab8b11/components/metrics/BUILD.gn
[modify] https://crrev.com/f8ad6e3ef816b7a971929637769230672bab8b11/components/metrics/call_stack_profile_metrics_provider.cc
[modify] https://crrev.com/f8ad6e3ef816b7a971929637769230672bab8b11/components/metrics/call_stack_profile_metrics_provider.h
[modify] https://crrev.com/f8ad6e3ef816b7a971929637769230672bab8b11/components/metrics/child_call_stack_profile_collector.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 8 2018

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

commit 77dff501ada3e737a014cb5c029530051b317e88
Author: David Trainor <dtrainor@chromium.org>
Date: Thu Feb 08 18:01:16 2018

Revert "Reland "Encapsulate Chrome thread profiling setup into a single class""

This reverts commit f8ad6e3ef816b7a971929637769230672bab8b11.

Reason for revert: Broke Android ToastHWATests (example: https://ci.chromium.org/buildbot/chromium.android/Lollipop%20Phone%20Tester/18596) 

Original change's description:
> Reland "Encapsulate Chrome thread profiling setup into a single class"
> 
> This is a reland of aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c.
> 
> Original change's description:
> > Encapsulate Chrome thread profiling setup into a single class
> >
> > Creates a simple abstraction around the fairly complex profiling setup.
> > This CL switches over the GPU and renderer process usage to this class.
> > The browser process usage will be converted to use this class after it
> > is extended to support periodic profiling (in a follow-on CL).
> >
> > Bug:  808588 
> > Change-Id: I6195571bf3ddd08291f288f475569bd9a88795ef
> > Reviewed-on: https://chromium-review.googlesource.com/905451
> > Reviewed-by: Scott Violet <sky@chromium.org>
> > Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> > Commit-Queue: Mike Wittman <wittman@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#535125}
> 
> TBR=sky
> 
> Bug:  808588 
> Change-Id: I34f1910116c4e7a500e353c27b85bf7534be43c7
> Reviewed-on: https://chromium-review.googlesource.com/907508
> Reviewed-by: Mike Wittman <wittman@chromium.org>
> Commit-Queue: Mike Wittman <wittman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#535170}

TBR=sky@chromium.org,wittman@chromium.org,asvitkine@chromium.org

Change-Id: I2e23bd8d60f6f91e058627ad9a1db41bdc9f4105
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  808588 
Reviewed-on: https://chromium-review.googlesource.com/909049
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535434}
[modify] https://crrev.com/77dff501ada3e737a014cb5c029530051b317e88/chrome/common/BUILD.gn
[modify] https://crrev.com/77dff501ada3e737a014cb5c029530051b317e88/chrome/common/DEPS
[delete] https://crrev.com/e12a039fc1488ef7926e4ab6cdd964fbf8801393/chrome/common/thread_profiler.cc
[delete] https://crrev.com/e12a039fc1488ef7926e4ab6cdd964fbf8801393/chrome/common/thread_profiler.h
[modify] https://crrev.com/77dff501ada3e737a014cb5c029530051b317e88/chrome/gpu/chrome_content_gpu_client.cc
[modify] https://crrev.com/77dff501ada3e737a014cb5c029530051b317e88/chrome/gpu/chrome_content_gpu_client.h
[modify] https://crrev.com/77dff501ada3e737a014cb5c029530051b317e88/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/77dff501ada3e737a014cb5c029530051b317e88/components/metrics/BUILD.gn
[modify] https://crrev.com/77dff501ada3e737a014cb5c029530051b317e88/components/metrics/call_stack_profile_metrics_provider.cc
[modify] https://crrev.com/77dff501ada3e737a014cb5c029530051b317e88/components/metrics/call_stack_profile_metrics_provider.h
[modify] https://crrev.com/77dff501ada3e737a014cb5c029530051b317e88/components/metrics/child_call_stack_profile_collector.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 15 2018

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

commit 106dd0717cd4c6310070d6e5d502b586086cb349
Author: Mike Wittman <wittman@chromium.org>
Date: Thu Feb 15 15:34:56 2018

Second reland of "Encapsulate Chrome thread profiling setup into a single class"

Fixes Android test failure by avoiding implicit creation of the render
thread. Also moves the decision on whether to profile extension processes into
the StackSamplingConfiguration, and cleans up comments.

This is a reland of f8ad6e3ef816b7a971929637769230672bab8b11.

Original change's description:
> Reland "Encapsulate Chrome thread profiling setup into a single class"
>
> This is a reland of aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c.
>
> Original change's description:
> > Encapsulate Chrome thread profiling setup into a single class
> >
> > Creates a simple abstraction around the fairly complex profiling setup.
> > This CL switches over the GPU and renderer process usage to this class.
> > The browser process usage will be converted to use this class after it
> > is extended to support periodic profiling (in a follow-on CL).
> >
> > Bug:  808588 
> > Change-Id: I6195571bf3ddd08291f288f475569bd9a88795ef
> > Reviewed-on: https://chromium-review.googlesource.com/905451
> > Reviewed-by: Scott Violet <sky@chromium.org>
> > Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> > Commit-Queue: Mike Wittman <wittman@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#535125}
>
> TBR=sky
>
> Bug:  808588 
> Change-Id: I34f1910116c4e7a500e353c27b85bf7534be43c7
> Reviewed-on: https://chromium-review.googlesource.com/907508
> Reviewed-by: Mike Wittman <wittman@chromium.org>
> Commit-Queue: Mike Wittman <wittman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#535170}

Bug:  808588 
Change-Id: Id34e6a3e57c144697413ecced43146bf3e6521e9
Reviewed-on: https://chromium-review.googlesource.com/917197
Commit-Queue: Mike Wittman <wittman@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537023}
[modify] https://crrev.com/106dd0717cd4c6310070d6e5d502b586086cb349/chrome/common/BUILD.gn
[modify] https://crrev.com/106dd0717cd4c6310070d6e5d502b586086cb349/chrome/common/DEPS
[modify] https://crrev.com/106dd0717cd4c6310070d6e5d502b586086cb349/chrome/common/stack_sampling_configuration.cc
[modify] https://crrev.com/106dd0717cd4c6310070d6e5d502b586086cb349/chrome/common/stack_sampling_configuration.h
[add] https://crrev.com/106dd0717cd4c6310070d6e5d502b586086cb349/chrome/common/thread_profiler.cc
[add] https://crrev.com/106dd0717cd4c6310070d6e5d502b586086cb349/chrome/common/thread_profiler.h
[modify] https://crrev.com/106dd0717cd4c6310070d6e5d502b586086cb349/chrome/gpu/chrome_content_gpu_client.cc
[modify] https://crrev.com/106dd0717cd4c6310070d6e5d502b586086cb349/chrome/gpu/chrome_content_gpu_client.h
[modify] https://crrev.com/106dd0717cd4c6310070d6e5d502b586086cb349/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/106dd0717cd4c6310070d6e5d502b586086cb349/components/metrics/BUILD.gn
[modify] https://crrev.com/106dd0717cd4c6310070d6e5d502b586086cb349/components/metrics/call_stack_profile_metrics_provider.cc
[modify] https://crrev.com/106dd0717cd4c6310070d6e5d502b586086cb349/components/metrics/call_stack_profile_metrics_provider.h
[modify] https://crrev.com/106dd0717cd4c6310070d6e5d502b586086cb349/components/metrics/child_call_stack_profile_collector.cc
[modify] https://crrev.com/106dd0717cd4c6310070d6e5d502b586086cb349/content/public/test/render_view_test.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 15 2018

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

commit 92ff5ccfaf16d5e5b662be28e5d655ddccef921e
Author: Matthew Jones <mdjones@chromium.org>
Date: Thu Feb 15 22:12:08 2018

Revert "Second reland of "Encapsulate Chrome thread profiling setup into a single class""

This reverts commit 106dd0717cd4c6310070d6e5d502b586086cb349.

Reason for revert: ToastHWATest still breaking because of this patch:
https://ci.chromium.org/buildbot/chromium.android/Lollipop%20Phone%20Tester/18803

Original change's description:
> Second reland of "Encapsulate Chrome thread profiling setup into a single class"
> 
> Fixes Android test failure by avoiding implicit creation of the render
> thread. Also moves the decision on whether to profile extension processes into
> the StackSamplingConfiguration, and cleans up comments.
> 
> This is a reland of f8ad6e3ef816b7a971929637769230672bab8b11.
> 
> Original change's description:
> > Reland "Encapsulate Chrome thread profiling setup into a single class"
> >
> > This is a reland of aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c.
> >
> > Original change's description:
> > > Encapsulate Chrome thread profiling setup into a single class
> > >
> > > Creates a simple abstraction around the fairly complex profiling setup.
> > > This CL switches over the GPU and renderer process usage to this class.
> > > The browser process usage will be converted to use this class after it
> > > is extended to support periodic profiling (in a follow-on CL).
> > >
> > > Bug:  808588 
> > > Change-Id: I6195571bf3ddd08291f288f475569bd9a88795ef
> > > Reviewed-on: https://chromium-review.googlesource.com/905451
> > > Reviewed-by: Scott Violet <sky@chromium.org>
> > > Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> > > Commit-Queue: Mike Wittman <wittman@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#535125}
> >
> > TBR=sky
> >
> > Bug:  808588 
> > Change-Id: I34f1910116c4e7a500e353c27b85bf7534be43c7
> > Reviewed-on: https://chromium-review.googlesource.com/907508
> > Reviewed-by: Mike Wittman <wittman@chromium.org>
> > Commit-Queue: Mike Wittman <wittman@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#535170}
> 
> Bug:  808588 
> Change-Id: Id34e6a3e57c144697413ecced43146bf3e6521e9
> Reviewed-on: https://chromium-review.googlesource.com/917197
> Commit-Queue: Mike Wittman <wittman@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#537023}

TBR=sky@chromium.org,wittman@chromium.org

Change-Id: I38fc0f48e73cb4f7c4ac1a31235f655c406f85b4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  808588 
Reviewed-on: https://chromium-review.googlesource.com/922942
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537106}
[modify] https://crrev.com/92ff5ccfaf16d5e5b662be28e5d655ddccef921e/chrome/common/BUILD.gn
[modify] https://crrev.com/92ff5ccfaf16d5e5b662be28e5d655ddccef921e/chrome/common/DEPS
[modify] https://crrev.com/92ff5ccfaf16d5e5b662be28e5d655ddccef921e/chrome/common/stack_sampling_configuration.cc
[modify] https://crrev.com/92ff5ccfaf16d5e5b662be28e5d655ddccef921e/chrome/common/stack_sampling_configuration.h
[delete] https://crrev.com/b36cc2b3601062d644b4a861b84e61d60744e335/chrome/common/thread_profiler.cc
[delete] https://crrev.com/b36cc2b3601062d644b4a861b84e61d60744e335/chrome/common/thread_profiler.h
[modify] https://crrev.com/92ff5ccfaf16d5e5b662be28e5d655ddccef921e/chrome/gpu/chrome_content_gpu_client.cc
[modify] https://crrev.com/92ff5ccfaf16d5e5b662be28e5d655ddccef921e/chrome/gpu/chrome_content_gpu_client.h
[modify] https://crrev.com/92ff5ccfaf16d5e5b662be28e5d655ddccef921e/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/92ff5ccfaf16d5e5b662be28e5d655ddccef921e/components/metrics/BUILD.gn
[modify] https://crrev.com/92ff5ccfaf16d5e5b662be28e5d655ddccef921e/components/metrics/call_stack_profile_metrics_provider.cc
[modify] https://crrev.com/92ff5ccfaf16d5e5b662be28e5d655ddccef921e/components/metrics/call_stack_profile_metrics_provider.h
[modify] https://crrev.com/92ff5ccfaf16d5e5b662be28e5d655ddccef921e/components/metrics/child_call_stack_profile_collector.cc
[modify] https://crrev.com/92ff5ccfaf16d5e5b662be28e5d655ddccef921e/content/public/test/render_view_test.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 17 2018

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

commit f140bee9f92369f964d5a1d7356f0ff6d9267452
Author: Mike Wittman <wittman@chromium.org>
Date: Sat Feb 17 00:26:47 2018

Third reland of "Encapsulate Chrome thread profiling setup into a single class"

Really fixes the Android test failure this time, by delaying the DCHECK on
process type until after we've considered whether the OS is supported by the
profiler. This has been validated by running the test on Android hardware. The
suspected issue is that Android test is not setting the process type command
line switch, which is being DCHECKed in the profiler code.

This is a reland of 106dd0717cd4c6310070d6e5d502b586086cb349.

Original change's description:
> Second reland of "Encapsulate Chrome thread profiling setup into a single class"
>
> Fixes Android test failure by avoiding implicit creation of the render
> thread. Also moves the decision on whether to profile extension processes into
> the StackSamplingConfiguration, and cleans up comments.
>
> This is a reland of f8ad6e3ef816b7a971929637769230672bab8b11.
>
> Original change's description:
> > Reland "Encapsulate Chrome thread profiling setup into a single class"
> >
> > This is a reland of aa63fc1d4b8e6add5ee7da79a4f2cc3871f8712c.
> >
> > Original change's description:
> > > Encapsulate Chrome thread profiling setup into a single class
> > >
> > > Creates a simple abstraction around the fairly complex profiling setup.
> > > This CL switches over the GPU and renderer process usage to this class.
> > > The browser process usage will be converted to use this class after it
> > > is extended to support periodic profiling (in a follow-on CL).
> > >
> > > Bug:  808588 
> > > Change-Id: I6195571bf3ddd08291f288f475569bd9a88795ef
> > > Reviewed-on: https://chromium-review.googlesource.com/905451
> > > Reviewed-by: Scott Violet <sky@chromium.org>
> > > Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> > > Commit-Queue: Mike Wittman <wittman@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#535125}
> >
> > TBR=sky
> >
> > Bug:  808588 
> > Change-Id: I34f1910116c4e7a500e353c27b85bf7534be43c7
> > Reviewed-on: https://chromium-review.googlesource.com/907508
> > Reviewed-by: Mike Wittman <wittman@chromium.org>
> > Commit-Queue: Mike Wittman <wittman@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#535170}
>
> Bug:  808588 
> Change-Id: Id34e6a3e57c144697413ecced43146bf3e6521e9
> Reviewed-on: https://chromium-review.googlesource.com/917197
> Commit-Queue: Mike Wittman <wittman@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#537023}

Bug:  808588 
Change-Id: Id93da3a3699b364e3d592823d4326b903b0c5a40
Reviewed-on: https://chromium-review.googlesource.com/923217
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537481}
[modify] https://crrev.com/f140bee9f92369f964d5a1d7356f0ff6d9267452/chrome/common/BUILD.gn
[modify] https://crrev.com/f140bee9f92369f964d5a1d7356f0ff6d9267452/chrome/common/DEPS
[modify] https://crrev.com/f140bee9f92369f964d5a1d7356f0ff6d9267452/chrome/common/stack_sampling_configuration.cc
[modify] https://crrev.com/f140bee9f92369f964d5a1d7356f0ff6d9267452/chrome/common/stack_sampling_configuration.h
[add] https://crrev.com/f140bee9f92369f964d5a1d7356f0ff6d9267452/chrome/common/thread_profiler.cc
[add] https://crrev.com/f140bee9f92369f964d5a1d7356f0ff6d9267452/chrome/common/thread_profiler.h
[modify] https://crrev.com/f140bee9f92369f964d5a1d7356f0ff6d9267452/chrome/gpu/chrome_content_gpu_client.cc
[modify] https://crrev.com/f140bee9f92369f964d5a1d7356f0ff6d9267452/chrome/gpu/chrome_content_gpu_client.h
[modify] https://crrev.com/f140bee9f92369f964d5a1d7356f0ff6d9267452/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/f140bee9f92369f964d5a1d7356f0ff6d9267452/components/metrics/BUILD.gn
[modify] https://crrev.com/f140bee9f92369f964d5a1d7356f0ff6d9267452/components/metrics/call_stack_profile_metrics_provider.cc
[modify] https://crrev.com/f140bee9f92369f964d5a1d7356f0ff6d9267452/components/metrics/call_stack_profile_metrics_provider.h
[modify] https://crrev.com/f140bee9f92369f964d5a1d7356f0ff6d9267452/components/metrics/child_call_stack_profile_collector.cc
[modify] https://crrev.com/f140bee9f92369f964d5a1d7356f0ff6d9267452/content/public/test/render_view_test.cc

Project Member

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

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

commit 8d74ea2d5a9b5e896fe542a038ec1ce7cb8bba0c
Author: Mike Wittman <wittman@chromium.org>
Date: Wed Feb 21 03:11:04 2018

Support periodic collection in the ThreadProfiler

Implements support for periodic collection of stack samples following
the initial startup profiling. This change effectively enables periodic
profiling for the GPU process main and IO threads and the renderer
compositor thread.

The periodic thread profiling support currently in use in the browser
process will be removed in a follow-on CL, and the UI and IO thread
users will be updated to use this implementation.


Bug:  808588 
Change-Id: I42dc32a7898f03dda0b0419f16171bf6f5d1b1c7
Reviewed-on: https://chromium-review.googlesource.com/907703
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538010}
[modify] https://crrev.com/8d74ea2d5a9b5e896fe542a038ec1ce7cb8bba0c/base/profiler/stack_sampling_profiler.cc
[modify] https://crrev.com/8d74ea2d5a9b5e896fe542a038ec1ce7cb8bba0c/base/threading/thread_restrictions.h
[modify] https://crrev.com/8d74ea2d5a9b5e896fe542a038ec1ce7cb8bba0c/chrome/common/thread_profiler.cc
[modify] https://crrev.com/8d74ea2d5a9b5e896fe542a038ec1ce7cb8bba0c/chrome/common/thread_profiler.h
[add] https://crrev.com/8d74ea2d5a9b5e896fe542a038ec1ce7cb8bba0c/chrome/common/thread_profiler_unittest.cc
[modify] https://crrev.com/8d74ea2d5a9b5e896fe542a038ec1ce7cb8bba0c/chrome/gpu/chrome_content_gpu_client.cc
[modify] https://crrev.com/8d74ea2d5a9b5e896fe542a038ec1ce7cb8bba0c/chrome/test/BUILD.gn

Project Member

Comment 10 by bugdroid1@chromium.org, Mar 1 2018

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

commit 4bb6af248ec6178da08cb97bba1511b96b9b1fea
Author: Mike Wittman <wittman@chromium.org>
Date: Thu Mar 01 00:22:06 2018

Use ThreadProfiler for browser process profiling

Converts the browser process profiling to use the ThreadProfiler
abstraction. This simplifies the browser process profiling
implementation and removes all users of the the current continuous
profiling implementation, allowing it to be cleaned up.

This also alters the way continuous profiling is done in the browser
process, from taking samples at 1Hz continuously to taking samples at
10Hz for 30 seconds during 2% of execution time. This will reduce
the number of samples by 80%.

Bug:  808588 
Change-Id: I2925dc75b27aaab093b4ea99ae5e8dabfa2b273c
Reviewed-on: https://chromium-review.googlesource.com/941730
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539954}
[modify] https://crrev.com/4bb6af248ec6178da08cb97bba1511b96b9b1fea/chrome/browser/chrome_browser_main.cc
[modify] https://crrev.com/4bb6af248ec6178da08cb97bba1511b96b9b1fea/chrome/browser/chrome_browser_main.h

Project Member

Comment 11 by bugdroid1@chromium.org, Apr 23 2018

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

commit 8b109669634848607de2eb8bee1d8f64acc1ff2f
Author: Mike Wittman <wittman@chromium.org>
Date: Mon Apr 23 17:14:26 2018

[Sampling profiler] Remove unused periodic profiling support code

This method for performing periodic profiling has been superseded by the
one in ThreadProfiler.

Bug:  808588 
Change-Id: I5554f2c2b46eebb4b08a490e36bdc64110013cc1
Reviewed-on: https://chromium-review.googlesource.com/1022721
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552738}
[modify] https://crrev.com/8b109669634848607de2eb8bee1d8f64acc1ff2f/components/metrics/call_stack_profile_metrics_provider.cc
[modify] https://crrev.com/8b109669634848607de2eb8bee1d8f64acc1ff2f/components/metrics/call_stack_profile_metrics_provider.h
[modify] https://crrev.com/8b109669634848607de2eb8bee1d8f64acc1ff2f/components/metrics/call_stack_profile_metrics_provider_unittest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Apr 27 2018

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

commit 458ae8b86b363a88de28c52520434a739e99702c
Author: Mike Wittman <wittman@chromium.org>
Date: Fri Apr 27 00:17:55 2018

[Sampling profiler] Remove StackSamplingProfiler continuation capability

This approach to continuous profiling has been superseded by the
continuous collection support at the ThreadProfiler level.

Bug:  808588 
Change-Id: I6144d04a3b94000d599a0823177bddaf2765f974
Reviewed-on: https://chromium-review.googlesource.com/1029192
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554210}
[modify] https://crrev.com/458ae8b86b363a88de28c52520434a739e99702c/base/profiler/stack_sampling_profiler.cc
[modify] https://crrev.com/458ae8b86b363a88de28c52520434a739e99702c/base/profiler/stack_sampling_profiler.h
[modify] https://crrev.com/458ae8b86b363a88de28c52520434a739e99702c/base/profiler/stack_sampling_profiler_unittest.cc
[modify] https://crrev.com/458ae8b86b363a88de28c52520434a739e99702c/chrome/common/thread_profiler.cc
[modify] https://crrev.com/458ae8b86b363a88de28c52520434a739e99702c/chrome/common/thread_profiler.h
[modify] https://crrev.com/458ae8b86b363a88de28c52520434a739e99702c/components/metrics/call_stack_profile_collector.cc
[modify] https://crrev.com/458ae8b86b363a88de28c52520434a739e99702c/components/metrics/call_stack_profile_metrics_provider.cc
[modify] https://crrev.com/458ae8b86b363a88de28c52520434a739e99702c/components/metrics/call_stack_profile_metrics_provider.h
[modify] https://crrev.com/458ae8b86b363a88de28c52520434a739e99702c/components/metrics/call_stack_profile_metrics_provider_unittest.cc
[modify] https://crrev.com/458ae8b86b363a88de28c52520434a739e99702c/components/metrics/call_stack_profile_params.h
[modify] https://crrev.com/458ae8b86b363a88de28c52520434a739e99702c/components/metrics/child_call_stack_profile_collector.cc
[modify] https://crrev.com/458ae8b86b363a88de28c52520434a739e99702c/components/metrics/child_call_stack_profile_collector.h
[modify] https://crrev.com/458ae8b86b363a88de28c52520434a739e99702c/components/metrics/child_call_stack_profile_collector_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment