[PATCH] D46145: Use a buffer when allocating relocations

[Rafael Avila de Espindola via Phabricator via llvm-commits]

espindola added inline comments.


================
Comment at: ELF/Writer.cpp:875
+  };
+
   // Scan all relocations. Each relocation goes through a series
----------------
grimar wrote:
> 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). 
Testing the suggestion the peak memory is indeed the same, but the total memory allocated is actually higher:

master 765.92  564.2
patch  748.16, 545.7
patch2 865.48, 545.7


https://reviews.llvm.org/D46145