[PATCH] D87326: [ADT] Fix reference invalidation when self referencing a SmallVector

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D87326#2273410 <https://reviews.llvm.org/D87326#2273410>, @dblaikie wrote:

> In D87326#2273355 <https://reviews.llvm.org/D87326#2273355>, @nikic wrote:
>
>> Tested the newest version of this patch and I'm still seeing **massive** regressions, even larger than before.
>>
>> Compile-time (instructions retired): https://llvm-compile-time-tracker.com/compare.php?from=cc947207283f934c72af0eb0b1a08978c59d40a2&to=d99c6d441764431519c1c11d490e7e88ffe06775&stat=instructions This is now a 1.4% geomean regression at O3 <https://reviews.llvm.org/owners/package/3/>, with 2% at O3 <https://reviews.llvm.org/owners/package/3/>, with tramp3d-v4 hitting 3%.
>>
>> Max RSS: https://llvm-compile-time-tracker.com/compare.php?from=cc947207283f934c72af0eb0b1a08978c59d40a2&to=d99c6d441764431519c1c11d490e7e88ffe06775&stat=max-rss This is now a 1.6% geomean regression at O3 <https://reviews.llvm.org/owners/package/3/>, with 2.1% at O0.
>>
>> Clang text size goes from 80560905 to 82179908, a 2% regression (non-LTO build using GCC 9.3).
>
> @njames93 - mind looking at the memory usage a bit? that's probably relatively easy to check (well, maybe not, since it'll be smeared over all the different instantiations of this template) & seems a bit surprising. Code growth is probably just what it is - but if there's a way to quantify it and check we've got minimal extra instantiations, that'd be good.
>
> The compile-time stat: @nikic: can you measure a wall performance difference? (otherwise possible they're shorter instructions, etc/ doesn't necessarily mean this makes compile times worse, perhaps?)

Oh, also - any chance you can get measurements with a self-host clang? (either a full two-stage bootstrap, or an LLVM 10 or 11 used to build)



================
Comment at: llvm/unittests/ADT/SmallVectorTest.cpp:478-480
+  EXPECT_EQ(Constructable::getNumCopyAssignmentCalls() +
+                Constructable::getNumCopyConstructorCalls(),
+            2);
----------------
njames93 wrote:
> dblaikie wrote:
> > Worth testing for the specific amounts separately, or does it vary over the test variations?
> If the container grows there will be 2 copy constructors called, if it doesn't, there is one copy constructor and one copy assign. 
Ah, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87326