-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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.
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
Compiler: enable parallel codegen with MT #14748
base: master
Are you sure you want to change the base?
Compiler: enable parallel codegen with MT #14748
Conversation
This looks great. But it also seems to be a mix of different changes. Could we extract the independent refactorings (such as extracting |
We only need to update unit.reused_compilation_unit? from forked process then have print_codegen_stats count & filter the units.
When compiled with -Dpreview_mt the compiler will take advantage of the MT environment to codegen the compilation units in parallel, avoiding fork (that's not supported with MT) and allowing parallel codegen on Windows.
3a08d9e
to
ac91f7c
Compare
Rebased on top of #14760. |
# We generate the bitcode in the main thread because LLVM contexts | ||
# must be unique per compilation unit, but we share different contexts | ||
# across many modules (or rely on the global context); trying to | ||
# codegen in parallel would segfault! | ||
# | ||
# Luckily generating the bitcode is quick and once the bitcode is | ||
# generated we don't need the global LLVM contexts anymore but can | ||
# parse the bitcode in an isolated context and we can parallelize the | ||
# slowest part: the optimization pass & compiling the object file. | ||
unit.generate_bitcode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docs ❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect example: behavior is so odd, document it, otherwise it will be incomprehensible.
private def compile_to_object | ||
temporary_object_name = self.temporary_object_name | ||
target_machine = compiler.create_target_machine | ||
compiler.optimize llvm_mod, target_machine unless compiler.optimization_mode.o0? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line above may be what fixed the LLVM segfault compared to the previous attempt: the target machine is local to the compilation unit, since as found out by @ggiraldez in #14496 (comment) the target machine itself is thread unsafe (not just the llvm context).
Refactors `Crystal::Compiler`: 1. extracts `#sequential_codegen`, `#parallel_codegen` and `#fork_codegen` methods; 2. merges `#codegen_many_units` into `#codegen` directly; 3. stops collecting reused units: `#fork_codegen` now updates `CompilationUnit#reused_compilation_unit?` state as reported by the forked processes, and `#print_codegen_stats` now counts & filters the reused units. Prerequisite for #14748 that will introduce `#mt_codegen`.
Implements parallel codegen of object files when MT is enabled in the compiler (
-Dpreview_mt
).It only impacts codegen for compilations with more than one compilation unit (module), that is when neither of
--single-module
,--release
or--cross-compile
is specified. This behavior is identical to the fork based codegen.Advantages:
The main points are increased portability and simpler logic, despite having to take care of LLVM thread safety quirks (see comments).
Issues:
The
threads
arg actually depicts the number of fibers, not threads, which is confusing and problematic: increasingthreads
but notCRYSTAL_WORKERS
will lead to more fibers than threads, with fibers being sheduled on the same threads, which won't bring any improvement.In fact
CRYSTAL_WORKERS
defaults to 4, whenthreads
defaulted to 8. With this patch it defaults toCRYSTAL_WORKERS
, so MT can end up being slower if we don't specifyCRYSTAL_WORKERS=8
.This is still not as efficient as it could be. The main fiber (that feeds the worker fibers) can get blocked by a worker fiber doing codegen, leading the other workers to starve. This is easily noticeable when compiling with
-O1
for example.Both issues will be fixable with RFC 2 where we can start an explicit context to run the worker fibers or start N isolated contexts (maybe a better idea). Until then, one should increase
CRYSTAL_WORKERS
.Supersedes #14227 and doesn't segfault (so far) with LLVM 12 or LLVM 18.1 🤞
TODO:
mt_parallel(units, n_threads)
CRYSTAL_CONFIG_WORKERS
to configure the default number of workers at compile time instead of the hardcoded 4 (in a distinct PR)