[v8] ItemParallelJob can launch more Tasks than there are Items to process |
||
Issue descriptionIn https://chromium-review.googlesource.com/c/v8/v8/+/887084 I had assumed we never had more |task_| than |items_| scheduled in an ItemParallelJob. Turns out this is not true (though most callers make an attempt at doing this, not all do: e.g. NumberOfScavengeTasks() isn't capped by the number of work items). I plan to clean that up by : 1) Quick patch to mark all |tasks_| beyond |items_.size()| as completed. 2) Follow-up cleanup to make it impossible to schedule more tasks than desired (ideally ItemParallelJob -- or even TaskScheduler with a jobs API -- decides how many tasks to schedule, the caller merely provides a sample task to be duplicated as needed). (1) is blocking issue 805932
,
Jan 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/2407b2bd1bcde3d064f897a73bac491604213fb2 commit 2407b2bd1bcde3d064f897a73bac491604213fb2 Author: Gabriel Charette <gab@chromium.org> Date: Mon Jan 29 16:03:46 2018 Revert "v8::ItemParallelJob : Do not launch more Tasks than there are Items to process." This reverts commit 8a27c7d396e30003f10277f8f8c5415115fa2cab. Reason for revert: Having more tasks then work items is intentional in some use cases, i.e. Scavenging where RunInParallel() does parallel processing on a dynamic workload *after* the initial set of work items: { barrier_->Start(); TimedScope scope(&scavenging_time); PageScavengingItem* item = nullptr; while ((item = GetItem<PageScavengingItem>()) != nullptr) { item->Process(scavenger_); item->MarkFinished(); } do { scavenger_->Process(barrier_); } while (!barrier_->Wait()); scavenger_->Process(); } Original change's description: > v8::ItemParallelJob : Do not launch more Tasks than there are Items to process. > > Except when there are 0 items. For some reason I don't quite understand yet, not > calling Run() on tasks_[0] when there are 0 items results in DCHECKs... > > Bug: chromium:806237 > Change-Id: I38c8fffde64a42f93f4efda492832651137eebd7 > Reviewed-on: https://chromium-review.googlesource.com/888704 > Commit-Queue: Gabriel Charette <gab@chromium.org> > Reviewed-by: Michael Lippautz <mlippautz@chromium.org> > Cr-Commit-Position: refs/heads/master@{#50924} TBR=gab@chromium.org,mlippautz@chromium.org Change-Id: Iad2ab16bb41f339de8e3fbca1c08c5d26b8a0111 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:806237 Reviewed-on: https://chromium-review.googlesource.com/891186 Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#50928} [modify] https://crrev.com/2407b2bd1bcde3d064f897a73bac491604213fb2/src/heap/item-parallel-job.h [modify] https://crrev.com/2407b2bd1bcde3d064f897a73bac491604213fb2/test/unittests/heap/item-parallel-job-unittest.cc
,
Jan 30 2018
Discovered that it is intentional to have more tasks than items sometimes for workloads that merely use items as an initial coarse grain seed to a dynamically distributed workload (e.g. scavenging [1]).
[1]
void RunInParallel() final {
(...)
{
barrier_->Start();
TimedScope scope(&scavenging_time);
PageScavengingItem* item = nullptr;
while ((item = GetItem<PageScavengingItem>()) != nullptr) {
item->Process(scavenger_);
item->MarkFinished();
}
***** // need all tasks to contribute here, there may even be 0 initial work items above but a parallelizable load for this second phase
do {
scavenger_->Process(barrier_);
} while (!barrier_->Wait());
scavenger_->Process();
*****
}
(...)
};
|
||
►
Sign in to add a comment |
||
Comment 1 by bugdroid1@chromium.org
, Jan 29 2018