TraceOnTap should use the Tracing.RequestMemoryDump devtools api |
||||||||
Issue descriptionToday go/TraceOnTap does the following: - start tracing wit the default config (periodic dumps) - wait 3 seconds - stop tracing This assumes that we get a periodic dump within the 3 seconds. we should change it to be: A. Start tracing with an empty memory-infra trace config. this will disable periodic dumps. This means using the |traceConfig| argument of Tracing.Start [2] instead of |categories| B. Use requestMemoryDump API to get a dump C. Wait for 3 seconds AND the ACK from B. (the rational of 3 seconds is that we still want to use TraceOnTap for non-memory perf-related issues. 3 seconds should be enough to spot things like a storm of JS events or whatnot) D. Stop tracing Now, there is a possible caveat that is: I don't remember if using |traceConfig| in the devtools Tracing.Start API works when attaching to the renderer devtools endpoint, as opposite to the browser (Extensions like TraceOnTap can only attach to the renderer. AFAIK only telemetry can attach to the browser endpoint). So A might recursively need to do the plumbing beween the devtools agent to propagate the traceConfig. However, it is possible that this issue has been fixed, so before looking at devtools code, check that the traceConfig API actually doesn't work :) for more details on how to use the traceConfig with memory-infra see https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_infra_startup_tracing.md essentially, in order to enable memory-infra without periodic dumps you want to pass this trace config: ----- { "included_categories": ["disabled-by-default-memory-infra"], "excluded_categories": [""], "memory_dump_config": { "triggers": [] } } ----- [1] https://chromedevtools.github.io/devtools-protocol/tot/Tracing/#method-requestMemoryDump [2] https://chromedevtools.github.io/devtools-protocol/tot/Tracing/#method-start
,
May 25 2017
Issue 698400 has been merged into this issue.
,
May 25 2017
Sadly it looks like setting the traceconfig from the renderer doesn't work :( I get the following error message:
{"code":-32000,"message":"Using trace config on renderer targets is not supported yet."}
,
May 26 2017
Alternative plan to avoid using the trace config and plumbing thorugh devtools, given that turned out to be more complicated than expected: - Today when the category filter has "memory-infra", in lack of a full trace config, we implicitly turn on the periodic dump (a dump every 2s, recently moved to 5s) - While that behavior is theoretically part of our API, realistically the chrome://tracing UI is the only one relying on that. AFAIK all other benchmark pass the full trace config, specifying explicit memory-infra dump intervals. My proposal is to change the behavior such that: - Just enabling "memory-infra" in the category filter does NOT enable the periodic dump scheduler - The chrome://tracing UI can already use the full trace config. So that's should be a matter of a few line change in content/browser/tracing/tracing_ui.cc of the form (if 'memory-infra' in categories: trace_config.triggers.push_back(detailed, 5s)) This will allow the TraceOnTap extension to rely on the fact that just specifying "memory-infra" in the categories will not trigger time-based dumps. The only risk is that if something out there is relying on the implicit "memory-infra" -> periodic dumps, we are going break them. +ssid opinions ?
,
May 26 2017
The plan in comment #5 sounds good! I can't think of anything that depends on periodic dumping.
,
May 26 2017
,
May 26 2017
,
May 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6f0aaabe91efa551a0c60d99b76319e13549e83a commit 6f0aaabe91efa551a0c60d99b76319e13549e83a Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org> Date: Sat May 27 18:46:04 2017 Roll src/third_party/catapult/ cb612d831..ea7d9cf8f (1 commit) https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/cb612d831847..ea7d9cf8f40e $ git log cb612d831..ea7d9cf8f --date=short --no-merges --format='%ad %ae %s' 2017-05-27 erikchen Update TraceOnTap to grab exactly 1 memory dump. Created with: roll-dep src/third_party/catapult BUG=726393 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, see: http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel TBR=sullivan@chromium.org Change-Id: I711ec95c4721f78c168918c7be6eee0b52f89efd Reviewed-on: https://chromium-review.googlesource.com/517320 Reviewed-by: <catapult-deps-roller@chromium.org> Commit-Queue: <catapult-deps-roller@chromium.org> Cr-Commit-Position: refs/heads/master@{#475248} [modify] https://crrev.com/6f0aaabe91efa551a0c60d99b76319e13549e83a/DEPS
,
May 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0529413d84c06f3720bf4a56b28bb2c7358cefd1 commit 0529413d84c06f3720bf4a56b28bb2c7358cefd1 Author: erikchen <erikchen@chromium.org> Date: Tue May 30 18:37:30 2017 Change memory dumps to not use periodic dumps by default. The only consumer that requires periodic dumps [and doesn't specify an explicit dump config] is Chrome tracing, so explicitly specify a periodic dump config there. Remove by default the periodic dump config. BUG=726393 Review-Url: https://codereview.chromium.org/2912483002 Cr-Commit-Position: refs/heads/master@{#475608} [modify] https://crrev.com/0529413d84c06f3720bf4a56b28bb2c7358cefd1/base/trace_event/memory_dump_manager_unittest.cc [modify] https://crrev.com/0529413d84c06f3720bf4a56b28bb2c7358cefd1/base/trace_event/trace_config.cc [modify] https://crrev.com/0529413d84c06f3720bf4a56b28bb2c7358cefd1/base/trace_event/trace_config_unittest.cc [modify] https://crrev.com/0529413d84c06f3720bf4a56b28bb2c7358cefd1/content/browser/tracing/tracing_ui.cc
,
Jun 2 2017
,
Jun 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bd7a01773b15d07a9bfea44177e3793f2a778d21 commit bd7a01773b15d07a9bfea44177e3793f2a778d21 Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org> Date: Sat Jun 03 06:11:27 2017 Roll src/third_party/catapult/ b0384fe60..64ea47945 (3 commits) https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/b0384fe60f10..64ea47945411 $ git log b0384fe60..64ea47945 --date=short --no-merges --format='%ad %ae %s' 2017-06-02 benjhayden Move the feedback form from group-report-page to report-page. 2017-06-02 eakuefner [Tracing] Add Python equality methods for sparse diagnostics 2017-06-02 erikchen Increase TraceOnTap flush timeout. Created with: roll-dep src/third_party/catapult BUG=726393 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, see: http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel TBR=sullivan@chromium.org Change-Id: I66c5fb3e4d19cfe709d475d0dd72d1dae1ce68bf Reviewed-on: https://chromium-review.googlesource.com/523437 Reviewed-by: <catapult-deps-roller@chromium.org> Commit-Queue: <catapult-deps-roller@chromium.org> Cr-Commit-Position: refs/heads/master@{#476893} [modify] https://crrev.com/bd7a01773b15d07a9bfea44177e3793f2a778d21/DEPS
,
Jun 3 2017
Assigning to me to remember to roll a new version in the store
,
Jun 12 2017
,
Jun 20 2017
,
Jun 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8b0b2a16a1e141102620e7d173271197cd6b7f3d commit 8b0b2a16a1e141102620e7d173271197cd6b7f3d Author: erikchen <erikchen@chromium.org> Date: Tue Jun 20 20:42:14 2017 Revert of Change memory dumps to not use periodic dumps by default. (patchset #3 id:40001 of https://codereview.chromium.org/2912483002/ ) Reason for revert: This breaks periodic dumping on Android. Reverting. Original issue's description: > Change memory dumps to not use periodic dumps by default. > > The only consumer that requires periodic dumps [and doesn't specify an explicit > dump config] is Chrome tracing, so explicitly specify a periodic dump config > there. Remove by default the periodic dump config. > > BUG=726393 > > Review-Url: https://codereview.chromium.org/2912483002 > Cr-Commit-Position: refs/heads/master@{#475608} > Committed: https://chromium.googlesource.com/chromium/src/+/0529413d84c06f3720bf4a56b28bb2c7358cefd1 TBR=primiano@chromium.org,ssid@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=726393 Review-Url: https://codereview.chromium.org/2952693002 Cr-Commit-Position: refs/heads/master@{#480941} [modify] https://crrev.com/8b0b2a16a1e141102620e7d173271197cd6b7f3d/base/trace_event/memory_dump_manager_unittest.cc [modify] https://crrev.com/8b0b2a16a1e141102620e7d173271197cd6b7f3d/base/trace_event/trace_config.cc [modify] https://crrev.com/8b0b2a16a1e141102620e7d173271197cd6b7f3d/base/trace_event/trace_config_unittest.cc [modify] https://crrev.com/8b0b2a16a1e141102620e7d173271197cd6b7f3d/content/browser/tracing/tracing_ui.cc
,
Jun 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c72902d0a30ee1fc71e7890d14c9ca79c3d387b3 commit c72902d0a30ee1fc71e7890d14c9ca79c3d387b3 Author: erikchen <erikchen@chromium.org> Date: Fri Jun 23 17:53:11 2017 Change memory dumps to not use periodic dumps by default. All consumers that require periodic dumps now explicitly specify a dump config. Remove by default the periodic dump config. BUG=726393 Review-Url: https://codereview.chromium.org/2952083004 Cr-Commit-Position: refs/heads/master@{#481951} [modify] https://crrev.com/c72902d0a30ee1fc71e7890d14c9ca79c3d387b3/base/trace_event/memory_dump_manager_unittest.cc [modify] https://crrev.com/c72902d0a30ee1fc71e7890d14c9ca79c3d387b3/base/trace_event/trace_config.cc [modify] https://crrev.com/c72902d0a30ee1fc71e7890d14c9ca79c3d387b3/base/trace_event/trace_config_unittest.cc
,
Jun 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0f0e44869e9812ed0c00baf812c866cb85520dfe commit 0f0e44869e9812ed0c00baf812c866cb85520dfe Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org> Date: Fri Jun 23 21:13:34 2017 Roll src/third_party/catapult/ 8d57d4e49..3d3b6d368 (3 commits) https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/8d57d4e4986b..3d3b6d368d4a $ git log 8d57d4e49..3d3b6d368 --date=short --no-merges --format='%ad %ae %s' 2017-06-23 phsilva Update the documentation on Ownership Diagnostics 2017-06-23 benjhayden Document Google Analytics metrics reported by results.html. 2017-06-23 erikchen Update record_controller to pass memoryDumpConfig to all controller clients. Created with: roll-dep src/third_party/catapult BUG=726393 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, see: http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel TBR=sullivan@chromium.org Change-Id: Ic1c9494e47eb82c800ca6dd1b2c1784cd6944780 Reviewed-on: https://chromium-review.googlesource.com/546576 Reviewed-by: <catapult-deps-roller@chromium.org> Commit-Queue: <catapult-deps-roller@chromium.org> Cr-Commit-Position: refs/heads/master@{#482032} [modify] https://crrev.com/0f0e44869e9812ed0c00baf812c866cb85520dfe/DEPS
,
Jun 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d4aa831fb4a764cbfa2c58c318849c8a967d7735 commit d4aa831fb4a764cbfa2c58c318849c8a967d7735 Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org> Date: Wed Jun 28 21:48:11 2017 Roll src/third_party/catapult/ 8114d33c7..9d1bd9e64 (4 commits) https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/8114d33c7d55..9d1bd9e6442b $ git log 8114d33c7..9d1bd9e64 --date=short --no-merges --format='%ad %ae %s' 2017-06-28 etienneb Fix coding style issue with diff_heap_profiler script 2017-06-28 erikchen symbolize_trace: Always symbolize all symbols by default. 2017-06-28 erikchen Increase flush timeout for fetching and compressing a memory dump. 2017-06-28 etienneb Add traces diffing tool for memory dumps between chrome traces Created with: roll-dep src/third_party/catapult BUG= 733056 , 737658 ,726393, 733056 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, see: http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel TBR=sullivan@chromium.org Change-Id: I918fd6b10c13bd20a170680c102ff30de6beebf6 Reviewed-on: https://chromium-review.googlesource.com/552873 Reviewed-by: <catapult-deps-roller@chromium.org> Commit-Queue: <catapult-deps-roller@chromium.org> Cr-Commit-Position: refs/heads/master@{#483151} [modify] https://crrev.com/d4aa831fb4a764cbfa2c58c318849c8a967d7735/DEPS |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by w...@chromium.org
, May 25 2017