New issue
Advanced search Search tips

Issue 905788 link

Starred by 1 user
Status: Assigned
Owner:
fdoray@chromium.org
Cc:
etiennep@chromium.org
gab@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Test launcher should not rely on TaskScheduler to control num_parallel_jobs.

Project Member Reported by etiennep@chromium.org, Nov 15 
Test launcher relies on the fact that TaskScheduler won't have more concurrency than max_tasks to control |num_parallel_jobs| [1].
This causes problem in [2]; The TaskScheduler is allowed to run more tasks concurrently than |max_tasks| when some of these tasks are blocked. Considering Process:WaitForExitWithTimeout() as blocking (as it probably should) causes the test launcher to go over |num_parallel_jobs| and ultimately timeout in some tests.

A future jobs API for the TaskScheduler might offer a simple alternative.
Until then, I don't see any good alternative, so we'll keep current
behavior for Process:WaitForExitWithTimeout(), and use this bug as a reminder.

[1]: https://cs.chromium.org/chromium/src/base/test/launcher/test_launcher.cc?dr=CSs&g=0&l=435
[2]: https://chromium-review.googlesource.com/c/chromium/src/+/1324418
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 16 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2f97450641fdc43f7ac6555fad0d47c83f540646

commit 2f97450641fdc43f7ac6555fad0d47c83f540646
Author: Etienne Pierre-doray <etiennep@chromium.org>
Date: Fri Nov 16 14:32:49 2018

[TaskScheduler]: Document Process::WaitForExitWithTimeout as not considered blocking.

Original CL intention was:
Use ScopedBlockingCallWithBaseSyncPrimitives to annotate sync primitives.

Migrate AssertBaseSyncPrimitivesAllowed() to
ScopedBlockingCallWithBaseSyncPrimitives in base::Process.

Bug: 903957, 905788
Change-Id: Ief5072d7e9b9feaf1ee7b817add26545bf80b78a
Reviewed-on: https://chromium-review.googlesource.com/c/1324418
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608768}
[modify] https://crrev.com/2f97450641fdc43f7ac6555fad0d47c83f540646/base/process/process_posix.cc
[modify] https://crrev.com/2f97450641fdc43f7ac6555fad0d47c83f540646/base/process/process_win.cc

Comment 2 by benhenry@google.com, Jan 11

Status: Assigned (was: Untriaged)
This issue has an owner, a component and a priority, but is still listed as untriaged or unconfirmed. By definition, this bug is triaged. Changing status to "assigned". Please reach out to me if you disagree with how I've done this.
►

Sign in to add a comment