From fce5457a141901097d0a57986eb5d3a023528059 Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Sat, 2 May 2020 13:28:56 -0700 Subject: [PATCH] [COFF] Avoid allocating temporary vectors during ICF Heap profiling with ETW shows that LLD performs 4,053,721 heap allocations 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. Reviewed By: hans Differential Revision: https://reviews.llvm.org/D79297 --- lld/COFF/Chunks.h | 3 ++- lld/COFF/ICF.cpp | 20 ++++++++++++-------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/lld/COFF/Chunks.h b/lld/COFF/Chunks.h index 2be2a72c4a1e..0528143383c5 100644 --- a/lld/COFF/Chunks.h +++ b/lld/COFF/Chunks.h @@ -269,7 +269,8 @@ public: 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; diff --git a/lld/COFF/ICF.cpp b/lld/COFF/ICF.cpp index c821569e3457..330379869cf9 100644 --- a/lld/COFF/ICF.cpp +++ b/lld/COFF/ICF.cpp @@ -127,15 +127,19 @@ void ICF::segregate(size_t begin, size_t end, bool constant) { // 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 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