[PATCH] D131247: [ELF] Parallelize writes of different OutputSections

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D131247#3715321 <https://reviews.llvm.org/D131247#3715321>, @peter.smith wrote:

> Not a lot to add over what has already been said. IIUC we do have code to detect when an Output Section overlaps and the user has to explicitly choose `--noinhibit-exec` to force the write? I think that non-deterministic output is reasonable in that case. Perhaps `--noinhibit-exec` could imply `--threads=1` if we were concerned about people relying on order of output.

The `--check-sections` error almost assuredly suggests a fatal style error. The user can get an output with `--noinhibit-exec`. This is fair corner case and I think letting it non-deterministic is fine.
If we want to avoid that, we can change `checkSections` to always run and return a value whether we should disable async parallel write.



================
Comment at: lld/ELF/OutputSections.cpp:471
+  // non-deterministic.
+  asyncParallelFor(tg, 128, 0, sections.size(), fn);
 }
----------------
andrewng wrote:
> MaskRay wrote:
> > ikudrin wrote:
> > > Where does the value of `128` come from? It looks like, with the hardcoded value, large projects will benefit from the parallelization more than small ones. And it also does not take into account the `llvm::parallel::detail::MaxTasksPerGroup`; can it somehow be derived from the common setting?
> > I don't think it can be derived from the common setting. It may not be bad to have a call site specific value like `SmallVector`. If we derived this automatically, tuning the library parameter may be more difficult.
> I would actually suggest that the task size could be even larger than `128` for output sections that have many input sections. I think as a "minimum" setting it feels about right. It's usually hard to derive a good task size value without incurring some cost in gathering appropriate metrics in order to make a good estimate.
> 
> Perhaps the task size could be calculated based on the number of sections and the number of threads in the task pool?
Thanks. Changed to a simple heuristic.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131247/new/

https://reviews.llvm.org/D131247