Skip to content

Use a thread pool when building runtimes #6133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Sep 30, 2025
Merged

Use a thread pool when building runtimes #6133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cd5b7b7
Use a thread pool when building runtimes
chandlerc Sep 13, 2025
a732ec4
disable more free
chandlerc Sep 25, 2025
40f5e61
Apply suggestions from code review
chandlerc Sep 25, 2025
df546ea
review feedback and fixes
chandlerc Sep 25, 2025
cc7cb77
format
chandlerc Sep 25, 2025
94370ce
Apply suggestions from code review
chandlerc Sep 26, 2025
1f90e49
tweak
chandlerc Sep 26, 2025
13289a2
adjust API
chandlerc Sep 27, 2025
5fbb837
Update toolchain/driver/clang_runner.cpp
chandlerc Sep 30, 2025
1833476
fixes
chandlerc Sep 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions toolchain/driver/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,11 @@ cc_library(
"//toolchain/install:install_paths",
"@llvm-project//clang:basic",
"@llvm-project//clang:clang-driver",
"@llvm-project//clang:codegen",
"@llvm-project//clang:driver",
"@llvm-project//clang:frontend",
"@llvm-project//clang:frontend_tool",
"@llvm-project//clang:serialization",
"@llvm-project//llvm:Core",
"@llvm-project//llvm:Object",
"@llvm-project//llvm:Support",
Expand Down
7 changes: 4 additions & 3 deletions toolchain/driver/build_runtimes_subcommand.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ auto BuildRuntimesSubcommand::Run(DriverEnv& driver_env) -> DriverResult {

auto BuildRuntimesSubcommand::RunInternal(DriverEnv& driver_env)
-> ErrorOr<std::filesystem::path> {
ClangRunner runner(driver_env.installation, &driver_env.runtimes_cache,
driver_env.fs, driver_env.vlog_stream);
ClangRunner runner(driver_env.installation, driver_env.fs,
driver_env.vlog_stream);

Runtimes::Cache::Features features = {
.target = options_.codegen_options.target.str()};
Expand All @@ -92,7 +92,8 @@ auto BuildRuntimesSubcommand::RunInternal(DriverEnv& driver_env)
: Runtimes::Make(explicit_output_path, driver_env.vlog_stream));
CARBON_ASSIGN_OR_RETURN(auto tmp_dir, Filesystem::MakeTmpDir());

return runner.BuildTargetResourceDir(features, runtimes, tmp_dir.abs_path());
return runner.BuildTargetResourceDir(features, runtimes, tmp_dir.abs_path(),
*driver_env.thread_pool);
}

} // namespace Carbon
318 changes: 239 additions & 79 deletions toolchain/driver/clang_runner.cpp
View file
Open in desktop

Large diffs are not rendered by default.

73 changes: 48 additions & 25 deletions toolchain/driver/clang_runner.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "common/ostream.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/ThreadPool.h"
#include "llvm/Support/VirtualFileSystem.h"
#include "llvm/TargetParser/Triple.h"
#include "toolchain/driver/runtimes_cache.h"
Expand All @@ -25,11 +26,15 @@ namespace Carbon {
// incorporating custom command line flags from user invocations that we don't
// parse, but will pass transparently along to Clang itself.
//
// This class is thread safe, allowing multiple threads to share a single runner
// and concurrently invoke Clang.
//
// This doesn't literally use a subprocess to invoke Clang; it instead tries to
// directly use the Clang command line driver library. We also work to simplify
// how that driver operates and invoke it in an opinionated way to get the best
// behavior for our expected use cases in the Carbon driver:
//
// - Ensure thread-safe invocation of Clang to enable concurrent usage.
// - Minimize canonicalization of file names to try to preserve the paths as
// users type them.
// - Minimize the use of subprocess invocations which are expensive on some
Expand All @@ -42,43 +47,57 @@ namespace Carbon {
// standard output and standard error, and otherwise can only read and write
// files based on their names described in the arguments. It doesn't provide any
// higher-level abstraction such as streams for inputs or outputs.
//
// TODO: Switch the diagnostic machinery to buffer and do locked output so that
// concurrent invocations of Clang don't intermingle their diagnostic output.
//
// TODO: If support for thread-local overrides of `llvm::errs` and `llvm::outs`
// becomes available upstream, also buffer and synchronize those streams to
// further improve the behavior of concurrent invocations.
class ClangRunner : ToolRunnerBase {
public:
// Build a Clang runner that uses the provided `exe_name` and `err_stream`.
// Build a Clang runner that uses the provided installation and filesystem.
//
// If `verbose` is passed as true, will enable verbose logging to the
// `err_stream` both from the runner and Clang itself.
// Optionally accepts a `vlog_stream` to enable verbose logging from Carbon to
// that stream. The verbose output from Clang goes to stderr regardless.
ClangRunner(const InstallPaths* install_paths,
Runtimes::Cache* on_demand_runtimes_cache,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs,
llvm::raw_ostream* vlog_stream = nullptr,
bool build_runtimes_on_demand = false);
llvm::raw_ostream* vlog_stream = nullptr);

// Run Clang with the provided arguments.
// Run Clang with the provided arguments and a runtime cache for on-demand
// runtime building.
//
// This works to support all of the Clang commandline, including commands that
// use target-dependent resources like linking. When it detects such commands,
// it will either use the provided target resource-dir path, or if building
// runtimes on demand is enabled it will build the needed resource-dir.
// it will use runtimes from the provided cache. If not available in the
// cache, it will build the necessary runtimes using the provided thread pool
// both to use and incorporate into the cache.
//
// Returns an error only if unable to successfully run Clang with the
// arguments. If able to run Clang, no error is returned a bool indicating
// whether than Clang invocation succeeded is returned.
//
// TODO: Eventually, this will need to accept an abstraction that can
// represent multiple different pre-built runtimes.
auto Run(llvm::ArrayRef<llvm::StringRef> args,
Runtimes* prebuilt_runtimes = nullptr) -> ErrorOr<bool>;
Runtimes::Cache& runtimes_cache,
llvm::ThreadPoolInterface& runtimes_build_thread_pool)
-> ErrorOr<bool>;

// Run Clang with the provided arguments and prebuilt runtimes.
//
// Similar to `Run`, but requires and uses pre-built runtimes rather than a
// cache or building them on demand.
auto RunWithPrebuiltRuntimes(llvm::ArrayRef<llvm::StringRef> args,
Runtimes& prebuilt_runtimes) -> ErrorOr<bool>;

// Run Clang with the provided arguments and without any target-dependent
// resources.
// Run Clang with the provided arguments and without any target runtimes.
//
// This method can be used to avoid building target-dependent resources when
// unnecessary, but not all Clang command lines will work correctly.
// Specifically, compile-only commands will typically work, while linking will
// not.
auto RunTargetIndependentCommand(llvm::ArrayRef<llvm::StringRef> args)
-> bool;
//
// This function simply returns true or false depending on whether Clang runs
// successfully, as it should display any needed error messages.
auto RunWithNoRuntimes(llvm::ArrayRef<llvm::StringRef> args) -> bool;

// Builds the target-specific resource directory for Clang.
//
Expand All @@ -88,7 +107,8 @@ class ClangRunner : ToolRunnerBase {
// return the path.
auto BuildTargetResourceDir(const Runtimes::Cache::Features& features,
Runtimes& runtimes,
const std::filesystem::path& tmp_path)
const std::filesystem::path& tmp_path,
llvm::ThreadPoolInterface& threads)
-> ErrorOr<std::filesystem::path>;

// Enable leaking memory.
Expand All @@ -103,6 +123,14 @@ class ClangRunner : ToolRunnerBase {
auto EnableLeakingMemory() -> void { enable_leaking_ = true; }

private:
// Emulates `cc1_main` but in a way that doesn't assume it is running in the
// main thread and can more easily fit into library calls to do compiles.
//
// TODO: Much of the logic here should be factored out of the CC1
// implementation in Clang's driver and into a reusable part of its libraries.
// That should allow reducing the code here to a minimal amount.
auto RunCC1(llvm::SmallVectorImpl<const char*>& cc1_args) -> int;

// Handles building the Clang driver and passing the arguments down to it.
auto RunInternal(llvm::ArrayRef<llvm::StringRef> args, llvm::StringRef target,
std::optional<llvm::StringRef> target_resource_dir_path)
Expand All @@ -125,16 +153,11 @@ class ClangRunner : ToolRunnerBase {
auto BuildBuiltinsLib(llvm::StringRef target,
const llvm::Triple& target_triple,
const std::filesystem::path& tmp_path,
Filesystem::DirRef lib_dir) -> ErrorOr<Success>;

Runtimes::Cache* runtimes_cache_;
Filesystem::DirRef lib_dir,
llvm::ThreadPoolInterface& threads) -> ErrorOr<Success>;

llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs_;
llvm::IntrusiveRefCntPtr<clang::DiagnosticIDs> diagnostic_ids_;

std::optional<std::filesystem::path> prebuilt_runtimes_path_;

bool build_runtimes_on_demand_;
bool enable_leaking_ = false;
};

Expand Down
31 changes: 15 additions & 16 deletions toolchain/driver/clang_runner_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,12 @@ class ClangRunnerTest : public ::testing::Test {

TEST_F(ClangRunnerTest, Version) {
RawStringOstream test_os;
ClangRunner runner(&install_paths_, &runtimes_cache_, vfs_, &test_os);
ClangRunner runner(&install_paths_, vfs_, &test_os);

std::string out;
std::string err;
EXPECT_TRUE(Testing::CallWithCapturedOutput(out, err, [&] {
return runner.RunTargetIndependentCommand({"--version"});
}));
EXPECT_TRUE(Testing::CallWithCapturedOutput(
out, err, [&] { return runner.RunWithNoRuntimes({"--version"}); }));
// The arguments to Clang should be part of the verbose log.
EXPECT_THAT(test_os.TakeStr(), HasSubstr("--version"));

Expand All @@ -101,13 +100,13 @@ TEST_F(ClangRunnerTest, DashC) {
std::filesystem::path test_output = *Testing::WriteTestFile("test.o", "");

RawStringOstream verbose_out;
ClangRunner runner(&install_paths_, &runtimes_cache_, vfs_, &verbose_out);
ClangRunner runner(&install_paths_, vfs_, &verbose_out);
std::string out;
std::string err;
EXPECT_TRUE(Testing::CallWithCapturedOutput(
out, err,
[&] {
return runner.RunTargetIndependentCommand(
return runner.RunWithNoRuntimes(
{"-c", test_file.string(), "-o", test_output.string()});
}))
<< "Verbose output from runner:\n"
Expand All @@ -130,13 +129,13 @@ TEST_F(ClangRunnerTest, BuitinHeaders) {
std::filesystem::path test_output = *Testing::WriteTestFile("test.o", "");

RawStringOstream verbose_out;
ClangRunner runner(&install_paths_, &runtimes_cache_, vfs_, &verbose_out);
ClangRunner runner(&install_paths_, vfs_, &verbose_out);
std::string out;
std::string err;
EXPECT_TRUE(Testing::CallWithCapturedOutput(
out, err,
[&] {
return runner.RunTargetIndependentCommand(
return runner.RunWithNoRuntimes(
{"-c", test_file.string(), "-o", test_output.string()});
}))
<< "Verbose output from runner:\n"
Expand All @@ -157,13 +156,13 @@ TEST_F(ClangRunnerTest, CompileMultipleFiles) {
std::filesystem::path output = *Testing::WriteTestFile(output_file, "");

RawStringOstream verbose_out;
ClangRunner runner(&install_paths_, &runtimes_cache_, vfs_, &verbose_out);
ClangRunner runner(&install_paths_, vfs_, &verbose_out);
std::string out;
std::string err;
EXPECT_TRUE(Testing::CallWithCapturedOutput(
out, err,
[&] {
return runner.RunTargetIndependentCommand(
return runner.RunWithNoRuntimes(
{"-c", file.string(), "-o", output.string()});
}))
<< "Verbose output from runner:\n"
Expand All @@ -180,8 +179,7 @@ TEST_F(ClangRunnerTest, CompileMultipleFiles) {
}

TEST_F(ClangRunnerTest, BuildResourceDir) {
ClangRunner runner(&install_paths_, &runtimes_cache_, vfs_, &llvm::errs(),
/*build_runtimes_on_demand=*/true);
ClangRunner runner(&install_paths_, vfs_, &llvm::errs());

// Note that we can't test arbitrary targets here as we need to be able to
// compile the builtin functions for the target. We use the default target as
Expand All @@ -191,8 +189,9 @@ TEST_F(ClangRunnerTest, BuildResourceDir) {
Runtimes::Cache::Features features = {.target = target};
auto runtimes = *runtimes_cache_.Lookup(features);
auto tmp_dir = *Filesystem::MakeTmpDir();
auto build_result =
runner.BuildTargetResourceDir(features, runtimes, tmp_dir.abs_path());
llvm::DefaultThreadPool threads(llvm::optimal_concurrency());
auto build_result = runner.BuildTargetResourceDir(
features, runtimes, tmp_dir.abs_path(), threads);
ASSERT_TRUE(build_result.ok()) << build_result.error();
std::filesystem::path resource_dir_path = std::move(*build_result);

Expand Down Expand Up @@ -270,7 +269,7 @@ TEST_F(ClangRunnerTest, LinkCommandEcho) {
std::filesystem::path bar_file = *Testing::WriteTestFile("bar.o", "");

RawStringOstream verbose_out;
ClangRunner runner(&install_paths_, &runtimes_cache_, vfs_, &verbose_out);
ClangRunner runner(&install_paths_, vfs_, &verbose_out);
std::string out;
std::string err;
EXPECT_TRUE(Testing::CallWithCapturedOutput(
Expand All @@ -280,7 +279,7 @@ TEST_F(ClangRunnerTest, LinkCommandEcho) {
// we're just getting the echo-ed output back. For this to actually
// link, we'd need to have the target-dependent resources, but those are
// expensive to build so we only want to test them once (above).
return runner.RunTargetIndependentCommand(
return runner.RunWithNoRuntimes(
{"-###", "-o", "binary", foo_file.string(), bar_file.string()});
}))
<< "Verbose output from runner:\n"
Expand Down
19 changes: 12 additions & 7 deletions toolchain/driver/clang_subcommand.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,8 @@ ClangSubcommand::ClangSubcommand() : DriverSubcommand(SubcommandInfo) {}
// add more.
// https://github.com/llvm/llvm-project/blob/main/clang/tools/driver/driver.cpp
auto ClangSubcommand::Run(DriverEnv& driver_env) -> DriverResult {
ClangRunner runner(
driver_env.installation, &driver_env.runtimes_cache, driver_env.fs,
driver_env.vlog_stream,
/*build_runtimes_on_demand=*/options_.build_runtimes_on_demand);
ClangRunner runner(driver_env.installation, driver_env.fs,
driver_env.vlog_stream);

// Don't run Clang when fuzzing, it is known to not be reliable under fuzzing
// due to many unfixed issues.
Expand All @@ -85,9 +83,16 @@ auto ClangSubcommand::Run(DriverEnv& driver_env) -> DriverResult {
runner.EnableLeakingMemory();
}

ErrorOr<bool> run_result = runner.Run(
options_.args,
driver_env.prebuilt_runtimes ? &*driver_env.prebuilt_runtimes : nullptr);
ErrorOr<bool> run_result = false;
if (driver_env.prebuilt_runtimes) {
run_result = runner.RunWithPrebuiltRuntimes(options_.args,
*driver_env.prebuilt_runtimes);
} else if (options_.build_runtimes_on_demand) {
run_result = runner.Run(options_.args, driver_env.runtimes_cache,
*driver_env.thread_pool);
} else {
run_result = runner.RunWithNoRuntimes(options_.args);
}
if (!run_result.ok()) {
// This is not a Clang failure, but a failure to even run Clang, so we need
// to diagnose it here.
Expand Down
28 changes: 28 additions & 0 deletions toolchain/driver/driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ struct Options {
bool verbose = false;
bool fuzzing = false;
bool include_diagnostic_kind = false;
bool threads = true;

llvm::StringRef runtimes_cache_path;
llvm::StringRef prebuilt_runtimes_path;
Expand Down Expand Up @@ -124,6 +125,25 @@ applies to each message that forms a diagnostic, not just the primary message.
},
[&](auto& arg_b) { arg_b.Set(&include_diagnostic_kind); });

b.AddFlag(
{
.name = "threads",
.help = R"""(
Controls whether threads are used to build runtimes.

When enabled (the default), Carbon will try to build runtime libraries using
threads to parallelize the operation. How many threads is controlled
automatically by the system.

Disabling threads ensures a single threaded build of the runtimes which can help
when there are errors or other output.
)""",
},
[&](auto& arg_b) {
arg_b.Default(true);
arg_b.Set(&threads);
});

runtimes.AddTo(b, &selected_subcommand);
clang.AddTo(b, &selected_subcommand);
compile.AddTo(b, &selected_subcommand);
Expand Down Expand Up @@ -208,6 +228,14 @@ auto Driver::RunCommand(llvm::ArrayRef<llvm::StringRef> args) -> DriverResult {
driver_env_.fuzzing = true;
}

llvm::SingleThreadExecutor single_thread({.ThreadsRequested = 1});
std::optional<llvm::DefaultThreadPool> threads;
driver_env_.thread_pool = &single_thread;
if (options.threads) {
threads.emplace(llvm::optimal_concurrency());
driver_env_.thread_pool = &*threads;
}
Comment on lines +231 to +237
Copy link
Contributor

@danakj danakj Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're are creating this SingleThreadExecutor at multiple levels it seems, both here and inside the ClangRunner. Could we consolidate to a single place - maybe making ClangRunner expect to receive a non-null executor always?

Copy link
Contributor Author

@chandlerc chandlerc Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could... I was just a bit torn doing so as it adds quite a bit of complexity to building and using the runner which is only needed if you're actually building runtimes.

Another alternative would be to have ClangRunner either accept a pre-built path or a thread pool to use for on-demand building, and remove the boolean option.

Copy link
Contributor

@danakj danakj Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you forsee wanting to use other thread pools than SingleThreadExecutor and DefaultThreadPool? Maybe we could tell it if we want threads or not and have ClangRunner make what it needs?

enum class BuildRuntimesOnDemand {
   UsePrebuiltOnly,
   BuildOnSingleThreaded,
   BuildOnWorkerThreads,
}

Taking either a path XOR a thread pool also sounds good, whichever you pref

Copy link
Contributor Author

@chandlerc chandlerc Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you forsee wanting to use other thread pools than SingleThreadExecutor and DefaultThreadPool?

Some possibility. One thing I wonder is if we'll want a (much) larger thread pool to fully absorb the latency, but would like to avoid it given the overhead.

Maybe we could tell it if we want threads or not and have ClangRunner make what it needs?

When using threads, my expectation is that it'll be very desirable to use the existing thread pool to avoid paying the cost of spinning one up and forking all the threads.

It also allows more global management of the load.

This is why I somewhat like the driver either accepting or building a thread pool, and then making it available for any commands or subcommands to use.

Taking either a path XOR a thread pool also sounds good, whichever you pref

I think what I'm liking is to take one of:

  • A thread pool, enabling on-demand building in that pool.
  • A pre-built path that will be used.
  • Nothing, disabling on-demand building, and its up to the caller to use the runner in a way compatible with that.

Copy link
Contributor Author

@chandlerc chandlerc Sep 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I thought more about this and I think I had the fundamental wrong structure of this API.

I've restructured everything so that we have a simple constructor that only accepts the necessary components. Then there are three variations on Run -- using on-demand runtimes with a cache and thread pool, using pre-built runtimes, and using no runtimes. I've also updated comments and callers accourdingly. I think this ends up more clear, but PTAL and let me know.


CARBON_CHECK(options.selected_subcommand != nullptr);
return options.selected_subcommand->Run(driver_env_);
}
Expand Down
5 changes: 5 additions & 0 deletions toolchain/driver/driver_env.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include <utility>

#include "common/ostream.h"
#include "llvm/Support/ThreadPool.h"
#include "llvm/Support/Threading.h"
#include "llvm/Support/VirtualFileSystem.h"
#include "toolchain/diagnostics/diagnostic_emitter.h"
#include "toolchain/driver/runtimes_cache.h"
Expand Down Expand Up @@ -62,6 +64,9 @@ struct DriverEnv {
// A diagnostic emitter that has no locations.
Diagnostics::NoLocEmitter emitter;

// Thread pool available for use when concurrency is needed.
llvm::ThreadPoolInterface* thread_pool;

// For CARBON_VLOG.
llvm::raw_pwrite_stream* vlog_stream = nullptr;

Expand Down
7 changes: 4 additions & 3 deletions toolchain/driver/link_subcommand.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,10 @@ auto LinkSubcommand::Run(DriverEnv& driver_env) -> DriverResult {
clang_args.append(options_.object_filenames.begin(),
options_.object_filenames.end());

ClangRunner runner(driver_env.installation, &driver_env.runtimes_cache,
driver_env.fs, driver_env.vlog_stream);
ErrorOr<bool> run_result = runner.Run(clang_args);
ClangRunner runner(driver_env.installation, driver_env.fs,
driver_env.vlog_stream);
ErrorOr<bool> run_result = runner.Run(clang_args, driver_env.runtimes_cache,
*driver_env.thread_pool);
if (!run_result.ok()) {
// This is not a Clang failure, but a failure to even run Clang, so we need
// to diagnose it here.
Expand Down
Loading
Loading