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

Mehdi AMINI via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 10 14:10:24 PST 2020


mehdi_amini added a comment.

> I think setting an inline storage size without profiling is often a premature optimization.

I don't necessarily agree with this: I believe this is the kind of situation where you end up with flat profile where malloc shows up everywhere, and then we would spend a huge amount of resources in optimizing malloc() (like some company specialized in moving protos where optimizing memcpy is easier than avoiding the copy in the first place ;)).
I remember seeing this in clang in the past with `strlen` taking a few percent on the profile, but it was really hard to fix it because of how widespread it is in the codebase: this is why idioms/patterns around APIs are important.
(also it isn't only about the malloc time, but I suspect the caching behavior of using the stack for short lived value likely help).

At the moment, I much prefer the current behavior and our usage of SmallVector to a "std::vector with implicit inlined storage sometimes, maybe, but you don't really know": when I read or review LLVM code I know what it is doing without the need for too much more context (I see it a bit similarly to our guideline on not abusing `auto` for readability).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90884



More information about the llvm-commits mailing list