Skip to content

Commit 6b44cea

Browse files
Revert "[Support][Jobserver][Tests] Simplify default executor init and make (#165264)"
This reverts commit b196c52. This broke premerge both within the PR and afterwards: 1. https://github.com/llvm/llvm-project/actions/runs/19351188603 2. https://lab.llvm.org/staging/#/builders/21/builds/8845 Other buildbots were failing as well: 1. https://lab.llvm.org/buildbot/#/builders/46/builds/26346
1 parent 757dc70 commit 6b44cea

File tree

2 files changed

+36
-68
lines changed

2 files changed

+36
-68
lines changed

llvm/lib/Support/Parallel.cpp

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,16 @@ class ThreadPoolExecutor : public Executor {
193193
JobserverClient *TheJobserver = nullptr;
194194
};
195195

196-
Executor *Executor::getDefaultExecutor() {
196+
// A global raw pointer to the executor. Lifetime is managed by the
197+
// objects created within createExecutor().
198+
static Executor *TheExec = nullptr;
199+
static std::once_flag Flag;
200+
201+
// This function will be called exactly once to create the executor.
202+
// It contains the necessary platform-specific logic. Since functions
203+
// called by std::call_once cannot return value, we have to set the
204+
// executor as a global variable.
205+
void createExecutor() {
197206
#ifdef _WIN32
198207
// The ManagedStatic enables the ThreadPoolExecutor to be stopped via
199208
// llvm_shutdown() which allows a "clean" fast exit, e.g. via _exit(). This
@@ -212,19 +221,27 @@ Executor *Executor::getDefaultExecutor() {
212221
// are more frequent with the debug static runtime.
213222
//
214223
// This also prevents intermittent deadlocks on exit with the MinGW runtime.
224+
215225
static ManagedStatic<ThreadPoolExecutor, ThreadPoolExecutor::Creator,
216226
ThreadPoolExecutor::Deleter>
217227
ManagedExec;
218-
return &*ManagedExec;
228+
static std::unique_ptr<ThreadPoolExecutor> Exec(&(*ManagedExec));
229+
TheExec = Exec.get();
219230
#else
220231
// ManagedStatic is not desired on other platforms. When `Exec` is destroyed
221232
// by llvm_shutdown(), worker threads will clean up and invoke TLS
222233
// destructors. This can lead to race conditions if other threads attempt to
223234
// access TLS objects that have already been destroyed.
224235
static ThreadPoolExecutor Exec(strategy);
225-
return &Exec;
236+
TheExec = &Exec;
226237
#endif
227238
}
239+
240+
Executor *Executor::getDefaultExecutor() {
241+
// Use std::call_once to lazily and safely initialize the executor.
242+
std::call_once(Flag, createExecutor);
243+
return TheExec;
244+
}
228245
} // namespace
229246
} // namespace detail
230247

llvm/unittests/Support/JobserverTest.cpp

Lines changed: 16 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
#include "llvm/Config/llvm-config.h"
1616
#include "llvm/Support/Debug.h"
1717
#include "llvm/Support/Parallel.h"
18-
#include "llvm/Support/Program.h"
1918
#include "llvm/Support/ThreadPool.h"
2019
#include "llvm/Support/raw_ostream.h"
2120
#include "gtest/gtest.h"
@@ -41,14 +40,8 @@
4140

4241
using namespace llvm;
4342

44-
// Provided by the unit test main to locate the current test binary.
45-
extern const char *TestMainArgv0;
46-
4743
namespace {
4844

49-
// Unique anchor whose address helps locate the current test binary.
50-
static int JobserverTestAnchor = 0;
51-
5245
// RAII helper to set an environment variable for the duration of a test.
5346
class ScopedEnvironment {
5447
std::string Name;
@@ -389,93 +382,51 @@ TEST_F(JobserverStrategyTest, ThreadPoolConcurrencyIsLimited) {
389382
EXPECT_EQ(CompletedTasks, NumTasks);
390383
}
391384

392-
// Parent-side driver that spawns a fresh process to run the child test which
393-
// validates that parallelFor respects the jobserver limit when it is the first
394-
// user of the default executor in that process.
395-
TEST_F(JobserverStrategyTest, ParallelForIsLimited_Subprocess) {
396-
// Mark child execution.
397-
setenv("LLVM_JOBSERVER_TEST_CHILD", "1", 1);
398-
399-
// Find the current test binary and build args to run only the child test.
400-
std::string Executable =
401-
sys::fs::getMainExecutable(TestMainArgv0, &JobserverTestAnchor);
402-
ASSERT_FALSE(Executable.empty()) << "Failed to get main executable path";
403-
SmallVector<StringRef, 4> Args{Executable,
404-
"--gtest_filter=JobserverStrategyTest."
405-
"ParallelForIsLimited_SubprocessChild"};
406-
407-
std::string Error;
408-
bool ExecFailed = false;
409-
int RC = sys::ExecuteAndWait(Executable, Args, std::nullopt, {}, 0, 0, &Error,
410-
&ExecFailed);
411-
unsetenv("LLVM_JOBSERVER_TEST_CHILD");
412-
ASSERT_FALSE(ExecFailed) << Error;
413-
ASSERT_EQ(RC, 0) << "Executable failed with exit code " << RC;
414-
}
415-
416-
// Child-side test: create FIFO and make-proxy in this process, set the
417-
// jobserver strategy, and then run parallelFor.
418-
TEST_F(JobserverStrategyTest, ParallelForIsLimited_SubprocessChild) {
419-
if (!getenv("LLVM_JOBSERVER_TEST_CHILD"))
420-
GTEST_SKIP() << "Not running in child mode";
421-
385+
TEST_F(JobserverStrategyTest, ParallelForIsLimited) {
422386
// This test verifies that llvm::parallelFor respects the jobserver limit.
423387
const int NumExplicitJobs = 3;
424388
const int ConcurrencyLimit = NumExplicitJobs + 1; // +1 implicit
425389
const int NumTasks = 20;
426390

391+
LLVM_DEBUG(dbgs() << "Calling startMakeProxy with " << NumExplicitJobs
392+
<< " jobs.\n");
427393
startMakeProxy(NumExplicitJobs);
394+
LLVM_DEBUG(dbgs() << "MakeProxy is running.\n");
428395

429-
// Set the global strategy before any default executor is created.
396+
// Set the global strategy. parallelFor will use this.
430397
parallel::strategy = jobserver_concurrency();
431398

432399
std::atomic<int> ActiveTasks{0};
433400
std::atomic<int> MaxActiveTasks{0};
434401

435-
parallelFor(0, NumTasks, [&]([[maybe_unused]] int i) {
402+
parallelFor(0, NumTasks, [&](int i) {
436403
int CurrentActive = ++ActiveTasks;
404+
LLVM_DEBUG(dbgs() << "Task " << i << ": Active tasks: " << CurrentActive
405+
<< "\n");
437406
int OldMax = MaxActiveTasks.load();
438407
while (CurrentActive > OldMax)
439408
MaxActiveTasks.compare_exchange_weak(OldMax, CurrentActive);
409+
440410
std::this_thread::sleep_for(std::chrono::milliseconds(20));
441411
--ActiveTasks;
442412
});
443413

414+
LLVM_DEBUG(dbgs() << "ParallelFor finished. Max active tasks was "
415+
<< MaxActiveTasks << ".\n");
444416
EXPECT_LE(MaxActiveTasks, ConcurrencyLimit);
445417
}
446418

447-
// Parent-side driver for parallelSort child test.
448-
TEST_F(JobserverStrategyTest, ParallelSortIsLimited_Subprocess) {
449-
setenv("LLVM_JOBSERVER_TEST_CHILD", "1", 1);
450-
451-
std::string Executable =
452-
sys::fs::getMainExecutable(TestMainArgv0, &JobserverTestAnchor);
453-
ASSERT_FALSE(Executable.empty()) << "Failed to get main executable path";
454-
SmallVector<StringRef, 4> Args{Executable,
455-
"--gtest_filter=JobserverStrategyTest."
456-
"ParallelSortIsLimited_SubprocessChild"};
457-
458-
std::string Error;
459-
bool ExecFailed = false;
460-
int RC = sys::ExecuteAndWait(Executable, Args, std::nullopt, {}, 0, 0, &Error,
461-
&ExecFailed);
462-
unsetenv("LLVM_JOBSERVER_TEST_CHILD");
463-
ASSERT_FALSE(ExecFailed) << Error;
464-
ASSERT_EQ(RC, 0) << "Executable failed with exit code " << RC;
465-
}
466-
467-
// Child-side test: ensure parallelSort runs and completes correctly under the
468-
// jobserver strategy when it owns default executor initialization.
469-
TEST_F(JobserverStrategyTest, ParallelSortIsLimited_SubprocessChild) {
470-
if (!getenv("LLVM_JOBSERVER_TEST_CHILD"))
471-
GTEST_SKIP() << "Not running in child mode";
472-
419+
TEST_F(JobserverStrategyTest, ParallelSortIsLimited) {
420+
// This test serves as an integration test to ensure parallelSort completes
421+
// correctly when running under the jobserver strategy. It doesn't directly
422+
// measure concurrency but verifies correctness.
473423
const int NumExplicitJobs = 3;
474424
startMakeProxy(NumExplicitJobs);
475425

476426
parallel::strategy = jobserver_concurrency();
477427

478428
std::vector<int> V(1024);
429+
// Fill with random data
479430
std::mt19937 randEngine;
480431
std::uniform_int_distribution<int> dist;
481432
for (int &i : V)

0 commit comments

Comments
 (0)