[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D77621#2000237 <https://reviews.llvm.org/D77621#2000237>, @nikic wrote:

> > Perhaps you could move the value computation into a constexpr variable & just return that as needed. (could be a static local constexpr, I guess - to avoid the issues around linkage of constexpr member variables)
>
> The use of a function rather than a static constexpr for SizeTypeMax() was my first thought as well. It seems pretty weird to me, but maybe it's enough to fall one the wrong side of some inlining heuristic.
>
> The only other thing that comes to mind is that grow_pod() moved into the header, which might have negative effects. It should be possible to avoid that by providing explicit template instantiations for uint32_t and uintptr_t in the cpp file.
>
> I'll try to figure out what the cause is, but might take me a few days.


I've tried those two things: results <http://llvm-compile-time-tracker.com/?config=O3&stat=instructions&branch=nikic%2Fperf%2Fsmallvector-size>. From the bottom, the first commit is a rebased version of the original change, the second one makes `SizeTypeMax` a constant instead of a function and the last one moves `grow_pod` back into the C++ file (I forgot to replace the UINT32_MAX references in grow_pod, but I don't think it has an impact on the conclusion). The first change is a `+0.75%` regression, the second is neutral and the last one is a `-0.70%` improvement, the remaining difference is likely noise. So it looks like the move of `grow_pod` into the header was the culprit.

What is rather peculiar is that the picture is similar for the max-rss numbers <http://llvm-compile-time-tracker.com/?config=O3&stat=max-rss&branch=nikic%2Fperf%2Fsmallvector-size>. I believe this is because max-rss is also influenced by the size of the clang binary itself, and apparently the move of grow_pod into the header increased it a lot. (I should probably collect clang binary size to make this easy to verify.) That means that there doesn't seem to be much of an increase in terms of actually allocated heap memory due to this change.

Taking the max-rss numbers <http://llvm-compile-time-tracker.com/compare.php?from=367229e100eca714276253bf95a0dd3d084a9624&to=a583ddc0a3ec034165d2762f791f5a88acc55a17&stat=max-rss> across all three commits, the only part where memory usage increases non-trivially is the LTO `-g` link step, by about ~1%. Possibly some debuginfo related stuff uses `SmallVector<char>`.

So tl;dr looks like as long as we keep `grow_pod` outside the header file, this change seems to be approximately free in terms of compile-time and memory usage both.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621