[PATCH] D60353: ELF: Add basic partition data structures and behaviours.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc marked an inline comment as done.
pcc added inline comments.


================
Comment at: lld/ELF/MarkLive.cpp:352
   // Follow the graph to mark all live sections.
-  MarkLive<ELFT>().run();
+  for (unsigned CurPart = 1; CurPart <= Partitions.size(); ++CurPart)
+    MarkLive<ELFT>(CurPart).run();
----------------
MaskRay wrote:
> MaskRay wrote:
> > I'm concerned with the performance.
> > 
> > Shall we just do one `MarkLive<ELFT>().run();` with all roots and one `MarkLive<ELFT>(1).moveToMain();`?
> > 
> > Before, we have npart+1 runs of DFS, now we have just 2 DFS.
> Or 1, if you can merge `moveToMain()` into `run()`.
I don't really see how that could lead to better performance, and the current approach is simpler anyway IMHO.

The important thing isn't how many runs of DFS we do but how many nodes we visit. With that approach we'd still be visiting the same number of nodes, just in a different order. In fact, it could lead to worse performance. Say we happen to visit a root for a loadable partition before visiting the roots of the main partition, and the loadable partition uses mostly the same code as the main partition. So we mark most of the main partition's code as belonging to the loadable partition. Now we move on to the main partition's roots. That will cause us to visit most of the main partition's code again to mark it as being in the main partition. With the current approach we would visit the main partition's code once and we don't visit it again while handling the loadable partition because of the code on lines 197-198. We only visit sections twice if code is used by multiple loadable partitions but not by the main partition, which is something that we'd need to do anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60353