[PATCH] D58296: [llvm-objcopy] Make removeSectionReferences batched

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht marked 2 inline comments as done.
rupprecht added a comment.

Looking at projects/test-suite; it doesn't seem to have much support for running other llvm tools, but I can add llvm-objcopy. Still planning to do that separately from this patch though.



================
Comment at: llvm/tools/llvm-objcopy/ELF/Object.cpp:1358
   // an error here instead.
+  std::vector<const SectionBase *> RemoveSections;
+  RemoveSections.reserve(std::distance(Iter, std::end(Sections)));
----------------
MaskRay wrote:
> rupprecht wrote:
> > jhenderson wrote:
> > > I'm staring at this and thinking that an unordered Set may be a better container for performance here, especially given our use of pointers, making the comparison and hashing operations cheap. It would remove the need for sort and binary_search in favour of a set lookup, which is (usually) constant time.
> > > 
> > > Have I missed something?
> > Switched, although it is only a minor improvement:
> > 
> > python -m timeit -n 3 -r 10 -v -s 'import os' 'os.system("llvm-objcopy -j .keep_me /tmp/huge-input.o /tmp/foo.o")'
> > 
> > w/ sorted vector:
> > raw times: 4.23 4.14 4.16 4.17 4.27 4.22 4.13 4.14 4.14 4.2
> > 3 loops, best of 10: 1.38 sec per loop
> > 
> > w/ unordered_set:
> > raw times: 3.83 3.75 3.85 3.77 3.76 3.87 3.81 3.82 3.84 3.83
> > 3 loops, best of 10: 1.25 sec per loop
> No preference here.. Is `SmallPtrSet<const SectionBase *, ?>` faster?
No, but it's not slower -- it produces numbers within noise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58296