Repository navigation
Conversation
da204ae to
db2e5c2
Compare
| const bool RunParallel = Reloc->shouldScanRelocations() && !IsPartialLink && | ||
| ThisConfig.options().numThreads() > 1 && | ||
| ThisConfig.isScanRelocationsMultiThreaded() && | ||
| PluginVect.empty(); |
There was a problem hiding this comment.
Why PluginVector empty causes ScanRelocations to be processed serially ?
With plugins being autoloaded, we will not get the benefits of running in parallel ?
There was a problem hiding this comment.
This was done for plugins that have a reloc callback; it did not affect autoloaded plugins. The issue was that a plugin's relocation callback could modify symbol/relocation state while another worker scans them. This was also a latent issue in the old parallel linker.
I removed the plugin restriction and moved reloc callback processing before phase 1 (still done in parallel) so scanning and reloc callbacks can never run concurrently.
| bool IsPartialLink, | ||
| LinkerScript::PluginVectorT PVect, | ||
| Relocator::CopyRelocs &CopyRelocs) { | ||
| Relocator::CopyRelocs *CopyRelocSet) { |
There was a problem hiding this comment.
Instead of using CopyRelocSet, can we use a boolean to hint if this needs to be multithreaded.
The caller uses a predicate to see if scanRelocationsHelper is called by thread pool or serially.
| } else { | ||
| getTargetBackend().getRelocator()->scanRelocationParallel( | ||
| *Relocation, *RelocSection, *Input, InputIndex, ThisSectIndex, | ||
| ThisRelocIndex); |
There was a problem hiding this comment.
it looks like every relocation that is scanned by scanRelocationsParallel sets the DeferredScan relocation index, and so the replayScanRelocations function run scan relocations serially.
Should the relocation index be only set for those that need GOT/PLT slots ?
Am I mis-reading something ?
There was a problem hiding this comment.
This is a good idea. I would like to handle performance improvements in a follow-up once the base design is in place. I think the parallel phase could classify/defer more stuff:
- GOT/PLT allocation
- dynamic relocation creation
- copy relocations
- TLS
- IFUNC
- Other target specific decisions, if any
Relocations with no serial side effects could be fully handled for scan purposes in phase 1. The serial phase would then apply only the deferred requests instead of walking every relocation.
| // allocated and nothing is appended to a shared section off the main thread. | ||
| assert(InputIndex < DeferredScan.size() && | ||
| "input missing from scan bookkeeping"); | ||
| DeferredScan[InputIndex].Sections[SectIndex].set(RelocIndex); |
There was a problem hiding this comment.
Would you want to use a DenseMap instead of a SmallVector ?
It becomes little difficult to track what Input/Section/Relocation is being deferred.
There was a problem hiding this comment.
The vectors avoid hashing and preserve determinism and parallel insertion. I renamed them to DeferredRelocations and RelocationsBySection. Hopefully that makes the mapping clearer.
| if (Sym->visibility() != ResolveInfo::Default) | ||
| issueInvisibleRef(Reloc, Input); | ||
| issueUndefRef(Reloc, Input, &Section); | ||
| } |
There was a problem hiding this comment.
return false ?
There was a problem hiding this comment.
We should not do this for --noinhibit-exec and to show complete diagnostics. The previous implementations also continued scanning instead of returning false.
Parth (parth-07)
left a comment
There was a problem hiding this comment.
I think we can do more work in the parallel phase. For example, we can mark the symbols which needs PLT/GOT in the parallel phase, and then in the phase 2 actually allocate slot for these symbols. This way, we will not need to traverse all the relocations in the phase 2. Please let me know your thoughts on this.
Also, can you please update PR description and the commit message with some performance numbers with this patch?
db2e5c2 to
4f48805
Compare
Restores parallel scanRelocations phase in a two-phase approach similar to lld: Phase 1 (scanRelocationParallel) may run concurrently over input files. It only performs the checks that are common to every backend and records which relocations need further processing. It creates no GOT/PLT slots and no dynamic relocations. Phase 2 (replayScanRelocations) is serial and visits the recorded relocations in input order. Slot allocation happens here and retains determinism. Relocation callbacks run before phase 1. Callbacks for different relocations still run in parallel, but all callbacks finish before backend scanning begins so callback mutations cannot race with scanner reads. This was possible in the old parallel scanRelocations implementation. Benchmarks: 16 threads, hyperfine 10 runs 2 warmups. Show below are scan time, wall clock time, for this PR, before this PR, and the old parallel scanRelocations implementation: Workload This commit origin/main (serial) Old parallel ld.eld 22.115 ms, 1.607 s 32.205 ms, 1.573 s 33.317 ms, 1.656 s Linux kernel 7315.164 ms, 16.850 s 7404.781 ms, 16.934 s 7790.618 ms, 17.229 s Future work: Move more relocation classification into the parallel phase. The serial phase should process only relocations that update shared linker state, such as GOT/PLT and dynamic-relocation handling, instead of walking every relocation. Fixes qualcomm#1938 Signed-off-by: quic-areg <aregmi@qti.qualcomm.com>
4f48805 to
23f7268
Compare
This is a great idea; I will do this in a follow-up.
Done |
Steven Ramirez Rosa (Steven6798)
left a comment
There was a problem hiding this comment.
after merging this patch monitor the next thread sanitizer run for any failures. I will be adding support for testing the sanitizer in PRs next week.
Restores the parallel
scanRelocationsphase in a two-phase approach similar to lld:Phase 1 (
scanRelocationParallel) may run concurrently over input files. It only performs the checks that are common to every backend and records which relocations need further processing. It creates no GOT/PLT slots and no dynamic relocations.Phase 2 (
replayScanRelocations) is serial and visits the recorded relocations in input order. Slot allocation happens here and retains determinism.Relocation callbacks run before phase 1. Callbacks for different relocations still run in parallel, but all callbacks finish before backend scanning begins so callback mutations cannot race with scanner reads. This was possible in the old parallel
scanRelocationsimplementation.Benchmarks: 16 threads, hyperfine 10 runs and 2 warmups. Below are scan time and wall clock time for this PR,
origin/main, and the old parallelscanRelocationsimplementation:origin/main(serial)ld.eldFuture work: Move more relocation classification into the parallel phase. The serial phase should process only relocations that update shared linker state, such as GOT/PLT and dynamic-relocation handling, instead of walking every relocation.
Fixes #1938