[llvm-dev] RFC: [SmallVector] Adding SVec<T> and Vec<T> convenience wrappers.

Sean Silva via llvm-dev llvm-dev at lists.llvm.org
Fri Nov 13 14:06:01 PST 2020


We've pretty happy now with a patch that adds two wrappers around
SmallVector that make it 1) more convenient to use and 2) will tend to
mitigate misuse of SmallVector. We think it's ready for wider discussion:
https://reviews.llvm.org/D90884

SVec<T> is a convenience alias for SmallVector<T, N> with N chosen
automatically to keep its size under 64 Bytes (that heuristic is easy to
change though). The recommendation in the patch is to use this "on the
stack, where a "small" number of elements are expected".

Vec<T> is a convenience alias for SmallVector<T, 0>. It lets us get the
(little-known?) benefits of SmallVector even when it has no inline elements
(see https://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h).
The recommendation in the patch is to use this when the SmallVector is on
the heap.

A lot of this is boiled out from the discussion in
https://groups.google.com/g/llvm-dev/c/q1OyHZy8KVc/m/1l_AasOLBAAJ?pli=1

The goals here are twofold:

1. convenience: not having to read/write "N", or do an extra edit/recompile
cycle if you forgot it

2. avoiding pathological cases: The choice of N is usually semi-arbitrary
in our experience, and if one isn't careful, can result in
sizeof(SmallVector) becoming huge, especially in the case of nested
SmallVectors. This patch avoids pathological cases in two ways:
  A. SVec<T>'s heuristic keeps sizeof(SVec<T>) bounded, which prevents
pathological size amplifications like in `SmallVector<struct
{SmallVector<T, 4> a, b; }, 4>`, where the small sizes effectively multiply
together. Of course, users can always write SmallVector<T, N> explicitly to
bypass this, but the need for that seems rare.
  B. SmallVector<T, 0> feels "weird to write" for most folks, even though
it is frequently the right choice. Vec<T> mitigates that by "looking
natural".

I'm surfacing this as an RFC to get feedback on a couple higher-level
points:
- does everybody agree that SVec<T> and Vec<T> are useful to have?
- get wider consensus around suggesting these as "defaults" (see my updates
to ProgrammersManual.rst in the patch)
- how much we want to bulk migrate code vs let it organically grow.
Replacing SmallVector<T, 0> with Vec<T> should be completely mechanical.
Replacing SmallVector<T, N> for general N would be a lot more work.
- of course: naming. SVector/Vector were floated in the patch as well and
seem ok. SmallVec was rejected as it was a prefix of SmallVector (messes up
autocomplete).

Looking forward to a world with fewer guessed SmallVector sizes,

-- Sean Silva
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20201113/82706d43/attachment.html>


More information about the llvm-dev mailing list