[PATCH] D44967: Initialize OffsetMap in a known location

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

espindola added inline comments.


================
Comment at: ELF/SyntheticSections.cpp:2578-2579
     MS->finalizeContents();
+    parallelForEach(MS->Sections,
+                    [](MergeInputSection *Sec) { Sec->initOffsetMap(); });
+  }
----------------
ruiu wrote:
> espindola wrote:
> > ruiu wrote:
> > > Is the point of this patch to parallelize `initOffsetMap`?
> > > 
> > > How many sections do you usually have MS->Sections? I thought it's not many.
> > > 
> > > Did you try parallelizing initOffsetMap itself? We have a huge number of Pieces, so there might be a large room for parallelization.
> > No, I used a parallelForEach mostly because it is used when splitting the sections and was the parallelism we would get before this patch (via call_once). I benchmarked without threads.
> > 
> > MS->Sections will probably have about one section per file for the common case of a simple string section.
> > 
> > Would you be OK with parallelizing initOffsetMap as a followup?
> > 
> I'm fine with doing it in a follow-up patch, but what I was wondering is something different. I just tried to understand this patch.
> 
> If you have to call initOffsetMap after finalizeContents, can you call initOffsetMap from finalizeContents?
I can call it from finalizeContens, but there are two versions, one in MergeNoTailSection and one in MergeTailSection. Doing the call in here avoids duplication.





https://reviews.llvm.org/D44967