[PATCH] D77621: ADT: SmallVector size & capacity use word-size integers when elements are small.

Duncan P. N. Exon Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 13 13:37:05 PDT 2020


dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

Thanks for your patience, I missed the updates on Friday.

I have a couple of optional comments inline that I don't feel strongly about.  LGTM either way.

In D77621#1972764 <https://reviews.llvm.org/D77621#1972764>, @dblaikie wrote:

> Fair enough - that complexity seems reasonably acceptable to me if you reckon the memory size benefits are still worthwhile (did you measure them on any particular workloads? Do we have lots of fairly empty SmallVectors, etc?) if they don't apply to smaller types like this?


I haven't measured anything recently.  Last I looked there were a number of SmallVectors inside other data structures (sometimes, sadly, SmallVector) on the heap (or stack).  In some cases the main reason not to use `std::vector` is the exception pessimizations.  It's nice to keep them small if it's reasonable to.



================
Comment at: llvm/include/llvm/ADT/SmallVector.h:52
+  // The maximum size depends on size_type used.
+  static constexpr size_t SizeMax() {
+    return std::numeric_limits<Size_T>::max();
----------------
STL data structures have a name for this called `max_size()`.  Should we be consistent with that?


================
Comment at: llvm/include/llvm/ADT/SmallVector.h:179
+  using Base::empty;
+  using Base::size;
+
----------------
Optionally we could expose `max_size()` as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621





More information about the cfe-commits mailing list