[PATCH] D46145: Use a buffer when allocating relocations

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/Writer.cpp:875
+  };
+
   // Scan all relocations. Each relocation goes through a series
----------------
espindola wrote:
> grimar wrote:
> > It can be a bit simpler I think:
> > (RAM profiling shows that the version below consume the same amount of memory).
> > 
> > ```
> > std::vector<Relocation> Buffer;
> > 
> > // <new comment>
> > auto WithBuffer = [&](InputSectionBase &Sec) {
> >   Fn(Sec);
> >   Buffer = std::move(Sec.Relocations);
> >   Sec.Relocations = Buffer;
> > };
> > ```
> That would reduce peak allocation, but each call to Fn is still allocating a new buffer, no?
> 
> But does suggest a way to simplify it a bit.
CPU time was about the same as the original code for my way I think.
Your way should be slightly faster though:

I observed a minor speed up between original and your version. It was:
Original CPU time total (3 runs): 2632, 2637, 2622.
Patch: 2604, 2610, 2592.

So it is about 0.5% I think. Results seem was more or less stable. I do not have precise numbers for my version underhand (I can retest it if you think it worth, but it was something about original code numbers). I think the simplicity of implementation makes more sense here probably (and I hope we will be able to use `shrink_to_fit` one day). 


https://reviews.llvm.org/D46145