[PATCH] D94555: [LLD][COFF] Avoid std::vector resizes during type merging

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: lld/COFF/DebugTypes.cpp:632-634
   merged.recs.resize(offset + newSize);
   auto newRec = makeMutableArrayRef(&merged.recs[offset], newSize);
   memcpy(newRec.data(), ty.data().data(), newSize);
----------------
I know this is a bit of a stretch - but given the somewhat non-trivial extra work required to determine the capacity, I'm wondering if this issue might be partly due to this incremental/repeated resizing.

I'm guessing that maybe the resize is thwarting the usual growth function of the vectors (eg: resize brings the size up to exactly the size specified, whereas repeated push_back hits the usual growth factor to offset the next push_backs, etc). The notes here: https://en.cppreference.com/w/cpp/container/vector/reserve discuss this sort of issue with reserve, though I'm pretty sure it applies to resize as well.

If it's easy to do, I'd be curious to know whether removing the fix in this patch, and instead changing this resize+memcpy to loop+push_back or... oh, reference invalidation concerns. What about recs.insert(recs.begin(), recs.end()) ? Hmm, yeah, seems the spec for std::vector doesn't allow that, so I guess the nearest code would be:
```
for (int i = 0; i != newSize; ++i)
  merged.recs.push_back(merged.recs[i]);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94555