[PATCH] D32853: [ELF] - Speedup readAddressArea() implementation.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: ELF/SyntheticSections.cpp:1722
+    for (std::pair<uint64_t, uint64_t> &R : Ranges) {
+      auto I = std::lower_bound(V.begin(), V.end(), R.first,
+                                [](InputSectionBase *IS, uint64_t Offset) {
----------------
grimar wrote:
> dblaikie wrote:
> > Is 'V' guaranteed to be sorted?
> We initialize File->Sections in the same order they are in objects section table:
> https://github.com/llvm-mirror/lld/blob/master/ELF/InputFiles.cpp#L282
> 
> From my observations input sections in objects are always being sorted by file offset.
> (just because usual code seems assigns file offsets for sections first and then writes them in the same order or something like that)
> That probably not a mandatory requirement, but looks to be reasonable assumption.
> 
> I can probably add some additional assert-check to verify array is sorted or call sort explicitly, not sure if latter worth to do.
> 
> Not relative to this patch:
> Another Idea I had in mind is to make DWARFAddressRangesVector to store not only Low/High index, but also a object::SectionRef for example.
> That would remove the need of returning fake section address from llvm::LoadedObjectInfo::getSectionLoadAddress and should allow LLD to get
> section index from SectionRef somehow. Then we will not need any loops for doing a range section search and approach would be cleaner and faster.
> But that would require DWARF parsers API change, I would probably not do that change right now, because it is not yet clear if we want it or not, because for example using relocated output for building index solves this problem in a different way, but at the same time I guess we can build index using multithreading and it is probably easier to not use relocated output for that and just parse each file in a parrallel loop. I am going to investigate that all and for now would go with current patch as it is simple LLD side solution that gives solid instant speedup.
If it's not guaranteed by ELF to be sorted - best not to rely on it. You might be able to construct a test case by modifying LLVM's MC layer and having it produce the headers in reverse order (or just swapping the first two, etc).


https://reviews.llvm.org/D32853