Adjust delay for forcible worker thread shutdown tasks |
|||
Issue descriptionThis is a follow-up issue for issue 487050 . To determine appropriate delay for forcible worker thread shutdown tasks, we should add UMA to record how WorkerThread is terminated (i.e. WorkerThread::getExitCode()). If the number of |AsyncForciblyTerminated| is high, the delay may be too short. Also, UMA to record duration of worker script execution would be helpful to determine it.
,
Jun 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/efb87570a17e42c56ed50b71a83f5861678ca2e9 commit efb87570a17e42c56ed50b71a83f5861678ca2e9 Author: nhiroki <nhiroki@chromium.org> Date: Thu Jun 09 09:22:38 2016 Worker: Add UMA to record an exit code of WorkerThread BUG= 616721 Review-Url: https://codereview.chromium.org/2045403002 Cr-Commit-Position: refs/heads/master@{#398824} [modify] https://crrev.com/efb87570a17e42c56ed50b71a83f5861678ca2e9/third_party/WebKit/Source/core/workers/WorkerThread.cpp [modify] https://crrev.com/efb87570a17e42c56ed50b71a83f5861678ca2e9/third_party/WebKit/Source/core/workers/WorkerThread.h [modify] https://crrev.com/efb87570a17e42c56ed50b71a83f5861678ca2e9/tools/metrics/histograms/histograms.xml
,
Jun 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/efb87570a17e42c56ed50b71a83f5861678ca2e9 commit efb87570a17e42c56ed50b71a83f5861678ca2e9 Author: nhiroki <nhiroki@chromium.org> Date: Thu Jun 09 09:22:38 2016 Worker: Add UMA to record an exit code of WorkerThread BUG= 616721 Review-Url: https://codereview.chromium.org/2045403002 Cr-Commit-Position: refs/heads/master@{#398824} [modify] https://crrev.com/efb87570a17e42c56ed50b71a83f5861678ca2e9/third_party/WebKit/Source/core/workers/WorkerThread.cpp [modify] https://crrev.com/efb87570a17e42c56ed50b71a83f5861678ca2e9/third_party/WebKit/Source/core/workers/WorkerThread.h [modify] https://crrev.com/efb87570a17e42c56ed50b71a83f5861678ca2e9/tools/metrics/histograms/histograms.xml
,
Jun 23 2016
[Status Update] According to the UMA, 99.7% of WorkerThread shutdown is done by graceful termination and 0.3% is done by asynchronous forcible termination. I noticed this UMA is not sufficient for confirming effect of graceful shutdown because this is recorded in the dtor of WorkerThread. Renderer crashes by forcible termination may happen before the dtor. Maybe we should have another UMA like WorkerThread.TerminationAttempts. If there is a difference between the the number of WorkerThread.ExitCode and the number of WorkerThread.TerminationAttempts, probably some termination attempts crashed. > Also, UMA to record duration of worker script execution would be helpful to determine it. I have a WIP patch for this: https://codereview.chromium.org/2077153002/
,
Jun 23 2016
Reg: the shutdown UMA-- for forcible termination case during process shutdown I started to worry that we might not be able to really send the UMA info to the browser process, which might skew the histogram too...? (At least that used to be a problem when collecting less noisy UMA for loading. I vaguely remember we started to send UMA data more reliably than before but I'm not too sure about that esp. things that are collected during process shutdown)
,
Jun 24 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cc65b94db2ebe5b99396f1cabb520f4dc1d8c259 commit cc65b94db2ebe5b99396f1cabb520f4dc1d8c259 Author: nhiroki <nhiroki@chromium.org> Date: Fri Jun 24 00:45:42 2016 Worker: Record time taken for running a debugger/worker task on WorkerThread UMA to record duration of a debugger/worker task would be helpful to determine a delay for a task of forcible worker thread shutdown. BUG= 616721 Review-Url: https://codereview.chromium.org/2077153002 Cr-Commit-Position: refs/heads/master@{#401759} [modify] https://crrev.com/cc65b94db2ebe5b99396f1cabb520f4dc1d8c259/third_party/WebKit/Source/core/workers/WorkerThread.cpp [modify] https://crrev.com/cc65b94db2ebe5b99396f1cabb520f4dc1d8c259/tools/metrics/histograms/histograms.xml
,
Jun 28 2016
Reg: recording ExitCode UMA on shutdown, separating the UMA into ExitCode_OnShutdown and ExitCode_OnRunning could be helpful to reduce a noise?
,
Jun 28 2016
[Status Update] According to WorkerThread.Task.Time UMA on Win Canary, 96% of regular tasks are complete in 2 sec, 98% in 5 sec and 99% in 14 sec. The current delay (2 sec) would be quite a reasonable value on desktop. On the other hand, I guess running a task on mobile may take more time. We might want to slightly increase the delay for mobile. I'm going to wait until the UMA gets in Dev etc on Android.
,
Jun 28 2016
Not directly related, but do you know what percentage of the renderers is going through the shutdown sequence? I think that some (most?) renderers are forcibly killed by the browser process without being asked to shut down.
,
Jul 4 2016
Reg: c#9-19, does "Fast Shutdown" mean forcible process termination? I don't know the percentage of renderers to be gracefully terminated. AFAICS, if a renderer process has SharedWorker or ServiceWorker, fast shutdown does not run: https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_process_host_impl.cc?sq=package:chromium&type=cs&rcl=1458862641&l=1670 (see |worker_ref_count_| handling)
,
Jul 4 2016
[Status Update] I checked UMAs on Android as of Jul 02 2016 (7 day aggregation): > WorkerThread.ExitCode GracefullyTerminated 99.86% AsyncForciblyTerminated 00.14% SyncForciblyTerminated 00.00% > WorkerThread.Task.Time 79.9%tile 2 secs 82.9%tile 2.8 secs 85.6%tile 3.9 secs 87.3%tile 5 secs 89.3%tile 10 secs 91.2%tile 19.9 secs The current delay (2 secs) seems to be short for Android, but 10 secs delay for 90%tile would be too long... 4 or 5 secs could be a point of compromise?? Note: Tweaking the delay does not affect synchronous forcible termination (i.e. terminateAndWait(), terminateAndWaitForAllWorkers()) requested by renderer shutdown etc. It affects only regular termination to schedule a delayed task.
,
Jul 7 2016
Moving this nonessential bug to the next milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 9 2018
I think we can archive this, it's been 1+ years. I don't think there's plans to adjust the delay unless we get evidence that it's problematic. And the histogram for task times is getting overflowed because of too many samples.
,
Feb 13 2018
Memo of the UMA before deletion (Feb 11 2018 - 7DA). The UMA of WorkerThread.Task.Time might be suspect because of issue 809672 , also we've had bugs where the worker thread is in a tight loop of very small tasks due to misuse of the Mojo handle watcher API. WorkerThread.Task.Time on Android Stable: 50.00% 52.78 75.00% 297.9 95.00% 6950 99.00% 399632 99.50% 1045913 WorkerThread.Task.Time on Win/Mac/Linux/CrOS Stable: 50.00% 3.021 75.00% 8.246 95.00% 135.0 99.00% 84247 99.50% 180172 WorkerThread.DebuggerTask.Time on Android Stable 50.00% 277.2 75.00% 624.5 95.00% 46261 99.00% 105016 99.50% 137481 WorkerThread.DebuggerTask.Time on Win/Mac/Linux/CrOS Stable 50.00% 67.53 75.00% 146.0 95.00% 17603 99.00% 56126 99.50% 80864
,
Feb 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0335b2141d9938dfb8417f600165fcc9b2addf17 commit 0335b2141d9938dfb8417f600165fcc9b2addf17 Author: Matt Falkenhagen <falken@chromium.org> Date: Tue Feb 13 06:09:00 2018 Remove WorkerThread.Task.Time histogram. This is overflowing causing confusion. Since we don't use it we can delete it. Also delete the related histogram WorkerThread.DebuggerTask.Time. Bug: 616721 , 809672 Change-Id: I33895fdf033ab2c07ccecfb05c75085a45569011 Reviewed-on: https://chromium-review.googlesource.com/911229 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Ilya Sherman <isherman@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Commit-Queue: Matt Falkenhagen <falken@chromium.org> Cr-Commit-Position: refs/heads/master@{#536264} [modify] https://crrev.com/0335b2141d9938dfb8417f600165fcc9b2addf17/third_party/WebKit/Source/core/workers/WorkerThread.cpp [modify] https://crrev.com/0335b2141d9938dfb8417f600165fcc9b2addf17/third_party/WebKit/Source/platform/scheduler/child/worker_scheduler_impl.cc [modify] https://crrev.com/0335b2141d9938dfb8417f600165fcc9b2addf17/tools/metrics/histograms/histograms.xml |
|||
►
Sign in to add a comment |
|||
Comment 1 by nhiroki@chromium.org
, Jun 3 2016Status: Started (was: Assigned)