Skip to content

Commit

Permalink
Stop dividing the command-line-provided number of test jobs by 2.
Browse files Browse the repository at this point in the history
The value should be half of the number of CPU cores or whatever
--test-launcher-jobs says. This change makes base::NumParallelJobs
return the number of CPU cores divided by a cores_per_job factor
supplied by each caller.

This also adds error checking on the result where it wasn't already
present.



Bug: 1096909
Change-Id: I7c5bbf15697157993ebe07667e243201f6808fe5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2251568
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ilia Samsonov <isamsonov@google.com>
Reviewed-by: Mostyn Bramley-Moore <mostynb@chromium.org>
Commit-Queue: Fergal Daly <fergal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#784687}
  • Loading branch information
fergald authored and Commit Bot committed Jul 2, 2020
1 parent 47816b0 commit 903473e
Show file tree
Hide file tree
Showing 11 changed files with 30 additions and 38 deletions.
8 changes: 4 additions & 4 deletions base/test/launcher/test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1837,7 +1837,7 @@ void TestLauncher::OnOutputTimeout() {
watchdog_timer_.Reset();
}

size_t NumParallelJobs() {
size_t NumParallelJobs(unsigned int cores_per_job) {
const CommandLine* command_line = CommandLine::ForCurrentProcess();
if (command_line->HasSwitch(switches::kTestLauncherJobs)) {
// If the number of test launcher jobs was specified, return that number.
Expand All @@ -1860,15 +1860,15 @@ size_t NumParallelJobs() {
return 1U;
}

// Default to the number of processor cores.
#if defined(OS_WIN)
// Use processors in all groups (Windows splits more than 64 logical
// processors into groups).
return base::checked_cast<size_t>(
size_t cores = base::checked_cast<size_t>(
::GetActiveProcessorCount(ALL_PROCESSOR_GROUPS));
#else
return base::checked_cast<size_t>(SysInfo::NumberOfProcessors());
size_t cores = base::checked_cast<size_t>(SysInfo::NumberOfProcessors());
#endif
return std::max(size_t(1), cores / cores_per_job);
}

std::string GetTestOutputSnippet(const TestResult& result,
Expand Down
2 changes: 1 addition & 1 deletion base/test/launcher/test_launcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ class TestLauncher {
};

// Return the number of parallel jobs to use, or 0U in case of error.
size_t NumParallelJobs();
size_t NumParallelJobs(unsigned int cores_per_job);

// Extract part from |full_output| that applies to |result|.
std::string GetTestOutputSnippet(const TestResult& result,
Expand Down
4 changes: 2 additions & 2 deletions base/test/launcher/unit_test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ int LaunchUnitTests(int argc,
RunTestSuiteCallback run_test_suite,
size_t retry_limit) {
CommandLine::Init(argc, argv);
size_t parallel_jobs = NumParallelJobs();
size_t parallel_jobs = NumParallelJobs(/*cores_per_job=*/1);
if (parallel_jobs == 0U) {
return 1;
}
Expand Down Expand Up @@ -265,7 +265,7 @@ int LaunchUnitTests(int argc,
RunTestSuiteCallback run_test_suite) {
// Windows CommandLine::Init ignores argv anyway.
CommandLine::Init(argc, NULL);
size_t parallel_jobs = NumParallelJobs();
size_t parallel_jobs = NumParallelJobs(/*cores_per_job=*/1);
if (parallel_jobs == 0U) {
return 1;
}
Expand Down
7 changes: 2 additions & 5 deletions chrome/test/base/browser_tests_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,9 @@

int main(int argc, char** argv) {
base::CommandLine::Init(argc, argv);
size_t parallel_jobs = base::NumParallelJobs();
if (parallel_jobs == 0U) {
size_t parallel_jobs = base::NumParallelJobs(/*cores_per_job=*/2);
if (parallel_jobs == 0U)
return 1;
} else if (parallel_jobs > 1U) {
parallel_jobs /= 2U;
}

#if defined(OS_WIN)
// Many tests validate code that requires user32.dll to be loaded. Loading it,
Expand Down
7 changes: 2 additions & 5 deletions chrome/test/base/browser_tests_main_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,9 @@ class BrowserTestSuiteRunnerChromeOS : public ChromeTestSuiteRunner {

int main(int argc, char** argv) {
base::CommandLine::Init(argc, argv);
size_t parallel_jobs = base::NumParallelJobs();
if (parallel_jobs == 0U) {
size_t parallel_jobs = base::NumParallelJobs(/*cores_per_job=*/2);
if (parallel_jobs == 0U)
return 1;
} else if (parallel_jobs > 1U) {
parallel_jobs /= 2U;
}

BrowserTestSuiteRunnerChromeOS runner;
ChromeTestLauncherDelegate delegate(&runner);
Expand Down
7 changes: 3 additions & 4 deletions chromecast/app/cast_test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,9 @@ class CastTestLauncherDelegate : public content::TestLauncherDelegate {

int main(int argc, char** argv) {
base::CommandLine::Init(argc, argv);
size_t parallel_jobs = base::NumParallelJobs();
if (parallel_jobs > 1U) {
parallel_jobs /= 2U;
}
size_t parallel_jobs = base::NumParallelJobs(/*cores_per_job=*/2);
if (parallel_jobs == 0U)
return 1;
chromecast::shell::CastTestLauncherDelegate launcher_delegate;
mojo::core::Init();
content::ForceInProcessNetworkService(true);
Expand Down
8 changes: 4 additions & 4 deletions content/test/content_test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@ class ContentTestLauncherDelegate : public TestLauncherDelegate {

int main(int argc, char** argv) {
base::CommandLine::Init(argc, argv);
size_t parallel_jobs = base::NumParallelJobs();
if (parallel_jobs > 1U) {
parallel_jobs /= 2U;
}
size_t parallel_jobs = base::NumParallelJobs(/*cores_per_job=*/2);
if (parallel_jobs == 0U)
return 1;

#if defined(OS_WIN)
// Load and pin user32.dll to avoid having to load it once tests start while
// on the main thread loop where blocking calls are disallowed.
Expand Down
7 changes: 3 additions & 4 deletions extensions/shell/test/shell_tests_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@

int main(int argc, char** argv) {
base::CommandLine::Init(argc, argv);
size_t parallel_jobs = base::NumParallelJobs();
if (parallel_jobs > 1U) {
parallel_jobs /= 2U;
}
size_t parallel_jobs = base::NumParallelJobs(/*cores_per_job=*/2);
if (parallel_jobs == 0U)
return 1;
extensions::AppShellTestLauncherDelegate launcher_delegate;
return content::LaunchTests(&launcher_delegate, parallel_jobs, argc, argv);
}
7 changes: 3 additions & 4 deletions fuchsia/engine/test/web_engine_test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,9 @@ int main(int argc, char** argv) {
command_line->AppendSwitchASCII(switches::kEnableLogging, "stderr");
command_line->AppendSwitch(switches::kDisableGpu);

size_t parallel_jobs = base::NumParallelJobs();
if (parallel_jobs > 1U) {
parallel_jobs /= 2U;
}
size_t parallel_jobs = base::NumParallelJobs(/*cores_per_job=*/2);
if (parallel_jobs == 0U)
return 1;
::WebEngineTestLauncherDelegate launcher_delegate;
return LaunchTests(&launcher_delegate, parallel_jobs, argc, argv);
}
7 changes: 3 additions & 4 deletions headless/test/headless_test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,9 @@ class HeadlessTestLauncherDelegate : public content::TestLauncherDelegate {

int main(int argc, char** argv) {
base::CommandLine::Init(argc, argv);
size_t parallel_jobs = base::NumParallelJobs();
if (parallel_jobs > 1U) {
parallel_jobs /= 2U;
}
size_t parallel_jobs = base::NumParallelJobs(/*cores_per_job=*/2);
if (parallel_jobs == 0U)
return 1;

// Setup a working test environment for the network service in case it's used.
// Only create this object in the utility process, so that its members don't
Expand Down
4 changes: 3 additions & 1 deletion weblayer/test/browsertests_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@

int main(int argc, char** argv) {
base::CommandLine::Init(argc, argv);
size_t parallel_jobs = base::NumParallelJobs();
size_t parallel_jobs = base::NumParallelJobs(/*cores_per_job=*/1);
if (parallel_jobs == 0U)
return 1;

// Set up a working test environment for the network service in case it's
// used. Only create this object in the utility process, so that its members
Expand Down

0 comments on commit 903473e

Please sign in to comment.