[llvm] r214135 - IR: Optimize size of use-list order shuffle vectors

[Chandler Carruth]

On Tue, Jul 29, 2014 at 9:23 AM, David Blaikie <dblaikie at gmail.com> wrote:

> Generally I'd suggest skipping emplace_back where push_back will have
> the same performance characteristics (since you don't need to call
> anything other than the move ctor, which push_back can do just fine)
>
> But given the use of the small-size optimization, pushing back the
> result isn't ideal either, it's probably better to allocate the
> element in the Stack then initialize it there so you don't have to
> copy the small portion over:
>
>   // Is there an easy way to make a sequence one larger and return a
> reference to that new element? It seems such a common operation...
>   Stack.resize(Stack.size() + 1);
>   UseListOrder &O = Stack.back();
>   ...
>
> In addition/alternatively, I'm not sure how valuable the small-size
> optimization is if all of them are going in another container anyway.
> Usually the small optimization is most valuable when it avoids malloc
> allocation entirely (by using stack space) but here each element in
> Stack will have the small footprint in use - so wasted small buffers
> will be costing heap space...
>
> (Chandler, again - thoughts on this tradeoff? I'm perhaps too readily
> open to avoiding premature optimization and thus avoid small-size
> optimizations until there's a really good reason to add them, but I
> haven't done as much profiling/performance work to have an intuition
> about when they're not premature and just good practice)
>

I disagree with the entire patch.

1) We should do this unless its showing up in a profile.
2) We shouldn't be rolling our own datastructure here even then. We already
have a TinyPtrVector which might be more appropriate *if* this is a
critical memory usage problem.

I tend to agree about the small-size optimization being of dubious use when
the container is being moved. I suspect std::vector is the correct
datastructure until a profile shows otherwise. If it ever does, the better
alternative to this might be to use pooled allocation and array refs....
But I'm skeptical about this being in the profile, and if it is in the
profile, that worsens my feelings about the entire approach.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140729/d07cd72b/attachment.html>