[PATCH] D115416: [lld-macho] Prevent writing map files on the critical path
Jez Ng via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 9 21:18:15 PST 2021
int3 added a comment.
Ah, thanks for the sources @thevinster! I was actually gonna agree that `async | deferred` seemed like the best choice, but then I found this: https://eli.thegreenplace.net/2016/the-promises-and-challenges-of-stdasync-task-based-parallelism-in-c11/
It makes the argument that the default policy should "never" be used because of all the potential pitfalls. I don't think any of those pitfalls apply to our relatively simple use case, but maybe it's nice to just use the `async` policy regardless so we don't have to think about that? But in any case...
I agree with @oontvoo that using raw `std::async` is not ideal since it's semi-deprecated. I'll also point out (belatedly) that it's not great to be running an async job in parallel with a call to `parallelForEachN`. The latter spawns jobs based on the number of cores available because it assumes that it is the only thing spawning jobs. Ideally, we would be spawning jobs from a single thread pool instead.
Parallel.h is designed for simple fork-join patterns where the forking and joining all happen at a single point, but that's not really what we're doing here. ThreadPool.h seems to be a better fit for what we are trying to do; it exposes a very similar API to `std::async` but I think without its pitfalls. If I'm reading the implementation correctly, `Pool.async()` always launches the jobs immediately if there is a thread available, and always launches them in a separate thread even if they get deferred. And it's being used throughout LLVM's codebase, so we should probably use it too :)
Here's an example we could draw on: https://github.com/llvm/llvm-project/blob/be0a7e9f27083ada6072fcc0711ffa5630daa5ec/mlir/include/mlir/IR/Threading.h#L76. I think we'd create one ThreadPool, use it to dispatch writeMapFile, then pass the same instance to `finalizeLinkEditSegment`, which dispatches more jobs with it (but only waits on those jobs it dispatched, and not the writeMapFile job). Also the ThreadPool dtor is blocking, so we don't actually need to wait on the writeMapFile job.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115416/new/
https://reviews.llvm.org/D115416
More information about the llvm-commits
mailing list