[PATCH] D38433: Introduce a specialized data structure to be used in a subsequent change

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

(Haven't addressed the code comments yet since the design isn't settled)

In https://reviews.llvm.org/D38433#893426, @chandlerc wrote:

> Have you considered building a `ChunkedVector` instead of a `ChunkedList`? Specifically, there is a great trick where you use a single index with the low bits being an index into the chunk and the high bits being an index into a vector of pointers. It has many of the benefits you list and is a bit simpler I think. It also supports essentially the entire vector API if desired. Both bi-directional and even random access are reasonably efficient. Good locality, etc.


With a vector-of-buffers implementation, I'm a bit worried about the space overhead on the smaller cases.  For instance, this is the histogram of how this data structure is populated from a clang-bootstrap (also in https://reviews.llvm.org/D38434):

       Count: 731310
         Min: 1
        Mean: 8.555150
  50th %tile: 4
  95th %tile: 25
  99th %tile: 53
         Max: 433

If I used a vector-of-buffers, I will either have to recompute the capacity and end (of the last buffer) on every insert (which will require an additional deref and some computation) or have to keep two words in the data structure over the three that smallvector keeps anyway.  This adds a lot of relative overhead on the median case (4 elements).  In fact, the current situation of two extra words also qualifies as a "lot" of relative overhead IMO, and I want to think of an SSO to improve the situation.

> The only downside I can see is that is is possible to implement insertion into the middle of `ChunkedList` with bounded complexity, but with `ChunkedVector` it would still be linear. However, you don't seem to have implemented arbitrary insertion and so I'm hoping that isn't necessary.
> 
> If you go the route of `ChunkedVector`, I'd also suggest thinking about what a useful SSO would look like... At the least an SmallVector for the pointers to chunks, but maybe also the ability to put the first chunk inline? Might make it too big in essentially all cases.
> 
> My guess is that `SmallVector<std::array<T, 4096/sizeof(T)>, N>` (for some small `N`) is going to be hard to beat.

Just to be clear, you mean `SmallVector<std::array<T, 4096/sizeof(T)> *, N>`, right?  Btw, `4096` seems a bit high based on the histogram in https://reviews.llvm.org/D38434, but we can obviously lower it.


https://reviews.llvm.org/D38433