[PATCH] D79297: [COFF] Avoid allocating temporary vectors during ICF

[Reid Kleckner via Phabricator via llvm-commits]

rnk created this revision.
rnk added reviewers: pcc, thakis, hans, aganea.
Herald added a project: LLVM.

Heap profiling with ETW shows that LLD allocates 4,053,721 bytes of heap
memory over its lifetime, and ~800,000 of them come from assocEquals.
These vectors are created just to do a comparison, so fuse the
comparison into the loop and avoid the allocation.

ICF is overall a small portion of the time spent linking, and I did not
measure overall throughput improvements from this change above the noise
threshold. However, these show up in the heap profiler, and the work is
done, so we might as well land it if the code is clear enough.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79297

Files:
  lld/COFF/Chunks.h
  lld/COFF/ICF.cpp


Index: lld/COFF/ICF.cpp
===================================================================
--- lld/COFF/ICF.cpp
+++ lld/COFF/ICF.cpp
@@ -127,15 +127,19 @@
 
 // Returns true if two sections' associative children are equal.
 bool ICF::assocEquals(const SectionChunk *a, const SectionChunk *b) {
-  auto childClasses = [&](const SectionChunk *sc) {
-    std::vector<uint32_t> classes;
-    for (const SectionChunk &c : sc->children())
-      if (!c.getSectionName().startswith(".debug") &&
-          c.getSectionName() != ".gfids$y" && c.getSectionName() != ".gljmp$y")
-        classes.push_back(c.eqClass[cnt % 2]);
-    return classes;
+  // Ignore associated metadata sections that don't participate in ICF, such as
+  // debug info and CFGuard metadata.
+  auto considerForICF = [](const SectionChunk &assoc) {
+    StringRef Name = assoc.getSectionName();
+    return !(Name.startswith(".debug") || Name == ".gfids$y" ||
+             Name == ".gljmp$y");
   };
-  return childClasses(a) == childClasses(b);
+  auto ra = make_filter_range(a->children(), considerForICF);
+  auto rb = make_filter_range(b->children(), considerForICF);
+  return std::equal(ra.begin(), ra.end(), rb.begin(), rb.end(),
+                    [&](const SectionChunk &ia, const SectionChunk &ib) {
+                      return ia.eqClass[cnt % 2] == ib.eqClass[cnt % 2];
+                    });
 }
 
 // Compare "non-moving" part of two sections, namely everything
Index: lld/COFF/Chunks.h
===================================================================
--- lld/COFF/Chunks.h
+++ lld/COFF/Chunks.h
@@ -269,7 +269,8 @@
     AssociatedIterator() = default;
     AssociatedIterator(SectionChunk *head) : cur(head) {}
     bool operator==(const AssociatedIterator &r) const { return cur == r.cur; }
-    const SectionChunk &operator*() const { return *cur; }
+    // FIXME: Wrong const-ness, but it makes filter ranges work.
+    SectionChunk &operator*() const { return *cur; }
     SectionChunk &operator*() { return *cur; }
     AssociatedIterator &operator++() {
       cur = cur->assocChildren;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79297.261668.patch
Type: text/x-patch
Size: 2084 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200502/f4b0c987/attachment.bin>