[llvm] r271669 - Adding reserve and capacity methods to FoldingSet

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 3 10:45:47 PDT 2016


> On Jun 3, 2016, at 10:35 AM, Craig, Ben <ben.craig at codeaurora.org> wrote:
> 
> 
>> You may be able to mimic "struct CountCopyAndMove" and the associated tests in unittests/ADT/DenseMapTest.cpp
> FoldingSet doesn't copy or move it's elements on a rehash.  It just splices them into a different singly linked list.  There isn't currently a user hook for telling when an item is relinked.  You could check a node before and after a resize operation to see if the link changed, but you need to generate a bucket collision first.

Ok I missed this.

> 
>> Doxygen on public API
> Will do.
> 
>> I was surprised that we target a load factor >1, but I'm still unsure about the right use-case for FoldingSet compare to a DenseSet for instance. Is it about consuming less memory?
> DenseSet contains and owns the elements.  It works best with small keys and values, because it can be a lot nicer about cache locality.  FoldingSet works better with large key/value pairs, because in those situations you know you're going to get some cache misses anyway.

Thanks, it makes sense.

> 
> I'm unclear on the rationale for the high load factor, but I didn't change that in this review.

Sure.

-- 
Mehdi



More information about the llvm-commits mailing list