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

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.

LGTM.

I'm surprised at just how much slower we are than GNU objcopy still, but this is definitely a big win.



================
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)));
----------------
rupprecht wrote:
> 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.
> Switched, although it is only a minor improvement:
I'll take a 10% performance improvement, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58296