Initially we thought the workload of foreground workers in the renderers all came from GC so that it didn't matter. This appears to be far from true. Renderer.UserVisibleTaskPriority handles 10X more tasks than V8.GC posts. https://uma.googleplex.com/histograms?endDate=20180226&dayCount=7&histograms=TaskScheduler.TaskLatencyMicroseconds.Renderer.UserVisibleTaskPriority%2CV8.GC.ParallelTaskLatencyMicroSeconds&fixupData=true&showMax=true&filters=platform%2Ceq%2CW%2Cchannel%2Ceq%2C1%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial and V8.GC tends to happen in more contentious scenarios on average too because latency is significantly slower than the average Renderer.UserVisibleTaskPriority task: V8.GC task latency: - 37us median - 200us 90th'ile - 3ms 99'ile Renderer.UserVisibleTaskPriority latency: - 8us median - 50us 90th'ile - 3ms 99th'ile
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/2600038ddbca59d1b706ada5da5707d9d84f5e78 commit 2600038ddbca59d1b706ada5da5707d9d84f5e78 Author: Gabriel Charette <gab@chromium.org> Date: Thu Mar 01 13:00:07 2018 [v8 platform] Introduce CallBlockingTaskFromBackgroundThread() and use it for parallel GC as it blocks the main thread. On Chromium this will result in a TaskPriority::USER_BLOCKING task, allowing TaskScheduler to prioritize them above all other async work. R=ahaas@chromium.org Bug: chromium:817421 Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng Change-Id: I8c782eb045035ce2899214528deae5a45f0e6600 Reviewed-on: https://chromium-review.googlesource.com/941946 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: Andreas Haas <ahaas@chromium.org> Cr-Commit-Position: refs/heads/master@{#51650} [modify] https://crrev.com/2600038ddbca59d1b706ada5da5707d9d84f5e78/include/v8-platform.h [modify] https://crrev.com/2600038ddbca59d1b706ada5da5707d9d84f5e78/src/heap/item-parallel-job.cc
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/1983f3055d48275cfbf8a1d7c842c4224ecd3125 commit 1983f3055d48275cfbf8a1d7c842c4224ecd3125 Author: Gabriel Charette <gab@chromium.org> Date: Mon Mar 26 17:43:22 2018 [V8 Platform] Make CallOnWorkerThread use std::unique_ptr This is done now while embedders have yet to adapt to the new API before it becomes hard to migrate. Also renamed variable/methods to use "worker threads" rather than "background" nomenclature. Extracted from https://chromium-review.googlesource.com/c/v8/v8/+/978443/7 while resolving the more contentious bits around using task runners. TBR=rmcilroy@chromium.org Bug: chromium:817421 Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng Change-Id: Ie3ddf15a708e829c0f718d89bebf3e96d1990c16 Reviewed-on: https://chromium-review.googlesource.com/980953 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#52231} [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/BUILD.gn [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/include/v8-platform.h [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/src/compiler-dispatcher/compiler-dispatcher.cc [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/src/compiler-dispatcher/compiler-dispatcher.h [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/src/compiler-dispatcher/optimizing-compile-dispatcher.cc [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/src/d8.cc [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/src/heap/array-buffer-collector.cc [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/src/heap/concurrent-marking.cc [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/src/heap/item-parallel-job.cc [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/src/heap/item-parallel-job.h [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/src/heap/spaces.cc [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/src/heap/store-buffer.cc [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/src/heap/sweeper.cc [delete] https://crrev.com/7e78e45b909fd499bb03ceae8fa89cd712ab30a5/src/libplatform/default-background-task-runner.cc [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/src/libplatform/default-platform.cc [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/src/libplatform/default-platform.h [add] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/src/libplatform/default-worker-threads-task-runner.cc [rename] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/src/libplatform/default-worker-threads-task-runner.h [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/test/cctest/cctest.h [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/test/cctest/heap/test-incremental-marking.cc [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/test/cctest/wasm/test-streaming-compilation.cc [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/test/unittests/api/isolate-unittest.cc [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc [modify] https://crrev.com/1983f3055d48275cfbf8a1d7c842c4224ecd3125/test/unittests/compiler-dispatcher/unoptimized-compile-job-unittest.cc
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/4b13a22ff477d56ead2ee452ac4bba52d8f05e92 commit 4b13a22ff477d56ead2ee452ac4bba52d8f05e92 Author: Gabriel Charette <gab@chromium.org> Date: Mon Apr 30 18:39:51 2018 [V8 Platform] Introduce CallDelayedOnWorkerThread() GetWorkerThreadsTaskRunner() was about to be phased out [1] but v8 r52818 landed ahead of it. Add CallDelayedOnWorkerThread() to the new worker thread API to support this use case before phasing out GetWorkerThreadsTaskRunner() [1] https://chromium-review.googlesource.com/c/v8/v8/+/978443 Implemented it in d8+cctest+default-platform right away to avoid requiring a non-null Isolate* (and yet another transitional API). R=ahaas@chromium.org, kozyatinskiy@chromium.org Bug: chromium:817421 Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I2bee08fee08cf15a664d31cc6817e21cebe1d140 Reviewed-on: https://chromium-review.googlesource.com/1033584 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org> Reviewed-by: Yang Guo <yangguo@chromium.org> Reviewed-by: Andreas Haas <ahaas@chromium.org> Cr-Commit-Position: refs/heads/master@{#52892} [modify] https://crrev.com/4b13a22ff477d56ead2ee452ac4bba52d8f05e92/include/v8-platform.h [modify] https://crrev.com/4b13a22ff477d56ead2ee452ac4bba52d8f05e92/src/d8.cc [modify] https://crrev.com/4b13a22ff477d56ead2ee452ac4bba52d8f05e92/src/inspector/v8-inspector-impl.cc [modify] https://crrev.com/4b13a22ff477d56ead2ee452ac4bba52d8f05e92/src/libplatform/default-platform.cc [modify] https://crrev.com/4b13a22ff477d56ead2ee452ac4bba52d8f05e92/src/libplatform/default-platform.h [modify] https://crrev.com/4b13a22ff477d56ead2ee452ac4bba52d8f05e92/test/cctest/cctest.h
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/4ac96190f74f94f6950c3c8c817ae444efc22b6b commit 4ac96190f74f94f6950c3c8c817ae444efc22b6b Author: Gabriel Charette <gab@chromium.org> Date: Mon Apr 30 19:05:40 2018 [V8 Platform] Better WorkerThreads APIs. As discussed @ https://chromium-review.googlesource.com/c/chromium/src/+/957761#message-4ba6c1bf637f91507544efc89a31e3e4dd407715 and again @ https://chromium-review.googlesource.com/c/chromium/src/+/957761#message-6d0430e640c82f2d5463259fecdc7fabf945b958 Get rid of task runners for WorkerThreads API (use case is always a one-off task in which case a static call is fine -- just like in Chromium's base/task_scheduler/post_task.h) Calling into V8Platform* from any worker thread is safe, what was previously unsafe was using an Isolate* from worker threads but Isolate* was dropped from the new worker threads APIs so this is now irrelevant. Bug: chromium:817421 Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng Change-Id: Idd2dbc081edfbcb8985eeb45eb64ffb2555fcf7c Reviewed-on: https://chromium-review.googlesource.com/978443 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: Yang Guo <yangguo@chromium.org> Reviewed-by: Andreas Haas <ahaas@chromium.org> Reviewed-by: Hannes Payer <hpayer@chromium.org> Cr-Commit-Position: refs/heads/master@{#52893} [modify] https://crrev.com/4ac96190f74f94f6950c3c8c817ae444efc22b6b/include/v8-platform.h [modify] https://crrev.com/4ac96190f74f94f6950c3c8c817ae444efc22b6b/src/d8.cc [modify] https://crrev.com/4ac96190f74f94f6950c3c8c817ae444efc22b6b/src/libplatform/default-platform.cc [modify] https://crrev.com/4ac96190f74f94f6950c3c8c817ae444efc22b6b/src/libplatform/default-platform.h [modify] https://crrev.com/4ac96190f74f94f6950c3c8c817ae444efc22b6b/src/wasm/module-compiler.cc [modify] https://crrev.com/4ac96190f74f94f6950c3c8c817ae444efc22b6b/src/wasm/module-compiler.h [modify] https://crrev.com/4ac96190f74f94f6950c3c8c817ae444efc22b6b/test/cctest/cctest.h [modify] https://crrev.com/4ac96190f74f94f6950c3c8c817ae444efc22b6b/test/cctest/wasm/test-streaming-compilation.cc [modify] https://crrev.com/4ac96190f74f94f6950c3c8c817ae444efc22b6b/test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc [modify] https://crrev.com/4ac96190f74f94f6950c3c8c817ae444efc22b6b/test/unittests/libplatform/default-platform-unittest.cc
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ca4884b93e197ef8826923bf5197998a408674ac commit ca4884b93e197ef8826923bf5197998a408674ac Author: Gabriel Charette <gab@chromium.org> Date: Fri May 04 17:35:06 2018 [v8 platform] Use modern v8 worker threads API in gin. Follow-up to v8 CLs: - https://chromium-review.googlesource.com/c/v8/v8/+/941946 - https://chromium-review.googlesource.com/c/v8/v8/+/941442 - https://chromium-review.googlesource.com/c/v8/v8/+/941944 - https://chromium-review.googlesource.com/c/v8/v8/+/1033584 - https://chromium-review.googlesource.com/c/v8/v8/+/978443 This will make Platform::CallBlockingTaskOnWorkerThread() actually result in a high-priority task which is expected to help blocking parallel GC tasks get ahead of other async work. And allow removal of deprecated "background threads" APIs in v8::Platform. Bug: 817421 Change-Id: Idb7f369538c23c9b065d90cc5ed48300a5c9724c Reviewed-on: https://chromium-review.googlesource.com/957761 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#556103} [modify] https://crrev.com/ca4884b93e197ef8826923bf5197998a408674ac/base/task_scheduler/task_scheduler.h [modify] https://crrev.com/ca4884b93e197ef8826923bf5197998a408674ac/gin/BUILD.gn [modify] https://crrev.com/ca4884b93e197ef8826923bf5197998a408674ac/gin/public/v8_platform.h [delete] https://crrev.com/a476611b3fbcc9ff552a2d504d7c58b58b4ff259/gin/v8_background_task_runner.cc [delete] https://crrev.com/a476611b3fbcc9ff552a2d504d7c58b58b4ff259/gin/v8_background_task_runner.h [modify] https://crrev.com/ca4884b93e197ef8826923bf5197998a408674ac/gin/v8_platform.cc
@GC folks: not sure the chromeperf dashboard caught much of a difference here [1] but UMA seems to have regressed around the same time [2] (not sure if it's actually related but it's suspicious). Any idea?! The only task marked as user-blocking in all of v8 are the ones posted from ItemParallelJob::Run() [3]. Do we need others? [1] https://chromeperf.appspot.com/group_report?rev=556103 [2] https://uma.googleplex.com/p/chrome/timeline_v2/?sid=4a8c92623d9aac535f77b390a1ac114e [3] https://cs.chromium.org/chromium/src/v8/src/heap/item-parallel-job.cc?type=cs&q=file:src/v8+CallBlockingTaskOnWorkerThread&sq=package:chromium I guess we could always revert https://chromium-review.googlesource.com/941946 to see if that's the cause.
Let's monitor UMA. It may be a flake. Chromeperf did not regress any GC jank metrics.
The NextAction date has arrived: 2018-05-14
Seems like there is a regression is foreground GC : https://uma.googleplex.com/p/chrome/timeline_v2/?sid=84718bad1e41262601ed1b1bef642222 My CL landed in 68.0.3420.0. The regression seems to correlate with 68.0.3419.0 : https://uma.googleplex.com/p/chrome/timeline_v2/?sid=9d9eb591b83cbdc7f5ba529e43e7c54d Thoughts?
(calling this fixed, filed issue 843771 to track the regression in 68.0.33419.0, cleanup of old APIs will follow shortly)
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/8f6ffbfca7fc09473ae551148e0a0b7b01b10b61 commit 8f6ffbfca7fc09473ae551148e0a0b7b01b10b61 Author: Gabriel Charette <gab@chromium.org> Date: Wed May 16 23:27:02 2018 [V8Platform] Remove deprecated Background threads APIs and make new APIs pure virtual. Also fixup some implementations that were lagging behind per the lack of pure virtual not having enforced everything yet. Also fixed recently introduced PredictablePlatform::CallDelayedOnWorkerThread() to ignore delayed tasks after realizing the intent is to intercept worker tasks instead of sending them to |platform_|. Node.js migrated off these APIs @ https://github.com/v8/node/pull/69 R=ahaas@chromium.org, yangguo@chromium.org Bug: chromium:817421 Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng Change-Id: I92171f213b5fc64ab1f21e8eec72738f5ce228bd Reviewed-on: https://chromium-review.googlesource.com/1045310 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Andreas Haas <ahaas@chromium.org> Cr-Commit-Position: refs/heads/master@{#53223} [modify] https://crrev.com/8f6ffbfca7fc09473ae551148e0a0b7b01b10b61/include/v8-platform.h [modify] https://crrev.com/8f6ffbfca7fc09473ae551148e0a0b7b01b10b61/src/d8.cc [modify] https://crrev.com/8f6ffbfca7fc09473ae551148e0a0b7b01b10b61/test/cctest/cctest.h [modify] https://crrev.com/8f6ffbfca7fc09473ae551148e0a0b7b01b10b61/test/cctest/heap/test-unmapper.cc [modify] https://crrev.com/8f6ffbfca7fc09473ae551148e0a0b7b01b10b61/test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc
Comment 1 by gab@chromium.org
, Feb 28 2018