Have num_cores-1 workers since the main thread is always busy |
|||||||
Issue descriptionIn renderers: workers are always used to help the main thread, which is always busy and the highest priority. Reserving a core for the main thread makes sense (v8 traces show that the additional worker doesn't get scheduled much by the OS and when it does it preempts the main thread). https://docs.google.com/document/d/1bdlWAWeP3j2yo2DYfeok6URqFCrt57yx-nucGMybGGQ/edit#heading=h.rwvxc6ljp9og
,
Feb 6 2018
,
Feb 6 2018
Actually, hmmm, turns out we had one too many worker. But then V8 was converting the "num workers" into a "num cores" signal... So now V8 is generating one fewer task than it should... will fix API in V8.
,
Feb 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/cdecc37500e6afb4275c68387da21b6faa5d5a01 commit cdecc37500e6afb4275c68387da21b6faa5d5a01 Author: Gabriel Charette <gab@chromium.org> Date: Tue Feb 06 16:22:17 2018 [v8::heap] Include main thread in num_tasks computations. The num_tasks computation has long been based on NumberOfAvailableBackgroundThreads() We used to have one background worker per core, stealing cycles from the main thread. I fixed that @ crrev.com/534414. But now this computation is wrong and generates one less task than it should (one per worker but the main thread takes task #0 in practice). Other usage of NumberOfAvailableBackgroundThreads() in V8 seem correct already so this is the only tweak required. R=mlippautz@chromium.org Bug: chromium:808028 Change-Id: I784ed9b764017f146931547d30be4a3b180b5a2c Reviewed-on: https://chromium-review.googlesource.com/904662 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Cr-Commit-Position: refs/heads/master@{#51121} [modify] https://crrev.com/cdecc37500e6afb4275c68387da21b6faa5d5a01/src/heap/heap.cc [modify] https://crrev.com/cdecc37500e6afb4275c68387da21b6faa5d5a01/src/heap/mark-compact.cc
,
Feb 6 2018
,
Feb 12 2018
,
Feb 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/8996e262cc880d6d12ac7dc468964e6313fde023 commit 8996e262cc880d6d12ac7dc468964e6313fde023 Author: Gabriel Charette <gab@chromium.org> Date: Mon Feb 12 18:07:09 2018 Fix 'num cores' in ConcurrentMarking as well. This is a follow-up to https://chromium-review.googlesource.com/904662 as I forgot this callsite there. The perf tests still haven't recovered from decreasing the worker count by 1 to account for main thread ( crbug.com/809961 ) and I assume this line is at fault. If this is correct, it also indicates ConcurrentMarking as a great area to focus since a single extra worker appears to be making a significant difference. R=mlippautz@chromium.org Bug: chromium:809961 , chromium:808028 Change-Id: I9df933a4193fb25ea4e857f589e2164c8a2859b4 Reviewed-on: https://chromium-review.googlesource.com/911670 Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#51249} [modify] https://crrev.com/8996e262cc880d6d12ac7dc468964e6313fde023/src/heap/concurrent-marking.cc
,
Feb 13 2018
,
Feb 14 2018
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by bugdroid1@chromium.org
, Feb 5 2018