[PATCH] D90884: [SmallVector] Add a default small size.

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

In D90884#2377497 <https://reviews.llvm.org/D90884#2377497>, @mehdi_amini wrote:

> Seems like it'll always have a default payload equivalent of 6 pointers? Maybe you can document this? Also that makes the vector 72B, which isn't cache line aligned, should we make it 64 or 128 instead?

Right, the header is usually two pointers big (64-bit), so 3x the header is 72. Maybe keeping the size under 128 makes more sense.

> I'm not sure it makes sense to every get a default of a very small N in general. I guess a default like here makes sense for smaller object, what about `static_assert` if `sizeof(T)>sizeof(void*)`?

I'm not sure I understand this suggestion. There are lots of `SmallVector<LargeTy>` instantiations.

FWIW, I'm in favor of a conservative, low default size here. LLVM developers tend to use SmallVector for everything, even when it's inappropriate (many heap cases).



================
Comment at: llvm/unittests/ADT/SmallVectorTest.cpp:1049
+
+TYPED_TEST(SmallVectorDefaultSmallSizeTest, DefaultSmallSize) {
+  using ValueT = typename decltype(this->theVector)::value_type;
----------------
If there's no runtime code, is there any reason not to stamp this out as a series of static_asserts, or a series of explicit template instantiations of a class template that does nothing but static_assert about its template arguments? Gtest's test registration is very heavyweight.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90884