Fix ThreadControllerImpl::DoWork trace event. |
||||||||||||
Issue descriptionThreadControllerImpl::DoWork trace event can intersect with SequenceManager::RunTask trace event.
,
Aug 8
We want this in M69 as well. This is trace-only change, so it's safe.
,
Aug 8
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 8
Pls apply appropriate OSs label. Thank you.
,
Aug 9
,
Aug 9
Approving merge to M69 branch 3497 based on comment #2 and per offline chat with altimin@.
,
Aug 9
,
Aug 13
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 14
Requesting merge to M69 ASAP. Thank you.
,
Aug 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/be51c1084af1b0369e77e5c00a82a5546e70bb7d commit be51c1084af1b0369e77e5c00a82a5546e70bb7d Author: Alexander Timin <altimin@chromium.org> Date: Tue Aug 14 20:27:44 2018 [scheduler] Fix ThreadControllerImpl::DoWork trace event. Replace ThreadControllerImpl:DoWork which has strange scope with ThreadControllerImpl::DoWork with batch-level scope and ThreadControllerImpl::RunTask with task-level scope. R=skyostil@chromium.org TBR=altimin@chromium.org BUG= 871204 (cherry picked from commit 27620cc5abb803c373ea3fdf27d463971f1509c1) Change-Id: Ifdf410b552143b3e0b9c603ed0ded5f621c3ea5f Reviewed-on: https://chromium-review.googlesource.com/1163611 Reviewed-by: Sami Kyöstilä <skyostil@chromium.org> Commit-Queue: Alexander Timin <altimin@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#581244} Reviewed-on: https://chromium-review.googlesource.com/1174840 Reviewed-by: Alexander Timin <altimin@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#628} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/be51c1084af1b0369e77e5c00a82a5546e70bb7d/base/task/sequence_manager/thread_controller_impl.cc
,
Aug 15
+lighthouse folks +traceviewer folks because of TOP_LEVEL_TASK_TITLES in https://cs.chromium.org/chromium/src/third_party/catapult/tracing/tracing/extras/chrome/event_finder_utils.html?type=cs&q=ThreadControllerImpl::DoWork&sq=package:chromium&g=0&l=25-34 The DoWork trace event is now under the 'sequence_manager' category and no longer has the same meaning. The new "ThreadControllerImpl::RunTask" event (on 'toplevel') reflects the same uninterruptible task used for our metric purposes. Also, altimin points out over IM: "The RunTask scope is refined and it should be slightly smaller than of the old DoWork."
,
Aug 15
FYI we'll have to fix this in Lighthouse, roll it into DevTools audits panel, and also merge back to m69. Not sure if it's important enough for traceviewer for them to fix and merge, too.
,
Aug 16
,
Oct 1
,
Oct 15
The following revision refers to this bug: https://chromium.googlesource.com/catapult/+/c2f72b8a9299ddd88999b154b797bce10201d853 commit c2f72b8a9299ddd88999b154b797bce10201d853 Author: Deepanjan Roy <dproy@chromium.org> Date: Mon Oct 15 19:30:00 2018 Fix top level scheduler task title The top level scheduler task was renamed, so changing all refs to that name. The old TaskQueueManager::DoWork event now has category 'sequence_manager', and has different semantics. To maintain backwards compatibility, we still consider TaskQueueManager::DoWork events with 'toplevel' category to be scheduler top level tasks. This also changes total EQT to use scheduler task. This was a case that was missed while fixing crbug.com/832651 . Bug: chromium:871204 , chromium:832651 Change-Id: I430ae8553e64ff30a94ab109a045e58849a99390 Reviewed-on: https://chromium-review.googlesource.com/c/1278111 Reviewed-by: oysteine <oysteine@chromium.org> Reviewed-by: Ben Hayden <benjhayden@chromium.org> Commit-Queue: Deepanjan Roy <dproy@chromium.org> [modify] https://crrev.com/c2f72b8a9299ddd88999b154b797bce10201d853/tracing/tracing/extras/chrome/event_finder_utils.html [modify] https://crrev.com/c2f72b8a9299ddd88999b154b797bce10201d853/tracing/tracing/importer/user_model_builder_test.html [modify] https://crrev.com/c2f72b8a9299ddd88999b154b797bce10201d853/tracing/tracing/metrics/v8/runtime_stats_metric_test.html [modify] https://crrev.com/c2f72b8a9299ddd88999b154b797bce10201d853/tracing/tracing/metrics/system_health/power_metric_test.html [modify] https://crrev.com/c2f72b8a9299ddd88999b154b797bce10201d853/trace_processor/experimental/mappers/long_running_tasks_mapper.html [modify] https://crrev.com/c2f72b8a9299ddd88999b154b797bce10201d853/tracing/tracing/metrics/system_health/expected_queueing_time_metric_test.html [modify] https://crrev.com/c2f72b8a9299ddd88999b154b797bce10201d853/tracing/tracing/metrics/system_health/loading_metric_test.html [modify] https://crrev.com/c2f72b8a9299ddd88999b154b797bce10201d853/tracing/tracing/extras/chrome/estimated_input_latency_test.html [modify] https://crrev.com/c2f72b8a9299ddd88999b154b797bce10201d853/tracing/tracing/extras/chrome/event_finder_utils_test.html [modify] https://crrev.com/c2f72b8a9299ddd88999b154b797bce10201d853/tracing/tracing/metrics/system_health/expected_queueing_time_metric.html
,
Oct 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e4b18306674979a2fffe59ae60e51320aa20ad6e commit e4b18306674979a2fffe59ae60e51320aa20ad6e Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Tue Oct 16 05:59:06 2018 Roll src/third_party/catapult 2be20fdd2d70..88afab4ff0f0 (4 commits) https://chromium.googlesource.com/catapult.git/+log/2be20fdd2d70..88afab4ff0f0 git log 2be20fdd2d70..88afab4ff0f0 --date=short --no-merges --format='%ad %ae %s' 2018-10-15 benjhayden@chromium.org Add ReportCacheRequest for v2spa service worker 2018-10-15 pasko@chromium.org event_finder_util: skip renderer events when no mainThread 2018-10-15 chiniforooshan@chromium.org Telemetry: fix a bug in percentage_smooth 2018-10-15 dproy@chromium.org Fix top level scheduler task title Created with: gclient setdep -r src/third_party/catapult@88afab4ff0f0 The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-autoroll 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=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel BUG=chromium:891797, chromium:895096 , chromium:871204 , chromium:832651 TBR=sullivan@chromium.org Change-Id: I2a5a1b05066c3c5f10bbec768c0bd430c80e5fa4 Reviewed-on: https://chromium-review.googlesource.com/c/1282342 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#599878} [modify] https://crrev.com/e4b18306674979a2fffe59ae60e51320aa20ad6e/DEPS |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by bugdroid1@chromium.org
, Aug 7