-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Pool more JIT resources to reduce memory usage/contention #44912
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
Conversation
Master:
PR:
|
FYI: it is not very effective to ask for review while the PR does not yet apply to master |
23c6838
to
b1b92fb
Compare
Sorry about that, this PR should now be applicable to master directly. |
OptimizerResultT operator()(orc::ThreadSafeModule TSM, orc::MaterializationResponsibility &R) { | ||
TSM.withModuleDo([&](Module &M) { | ||
uint64_t start_time = 0; | ||
if (dump_llvm_opt_stream != NULL) { |
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.
It appears this might need some locking later (for dump_llvm_opt_stream)
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 should be addressed in the latest commit in #44914 to lock around bundles of stream operations.
src/jitlayers.cpp
Outdated
{ | ||
(***PMs).run(M); | ||
} |
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.
{ | |
(***PMs).run(M); | |
} | |
(***PMs).run(M); |
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.
SGTM
Rather than create a new TargetMachine/PassManager for every single compilation (which uses a lot of memory/construction+destruction time) or guarding a single one with a mutex (no parallelism), we can instead share PassManagers/TargetMachines between threads using a simple resource pool. This should hopefully reduce the latency impact in #44568 back to what it was before #44364.
Depends on #44605 for the resource pool implementation.