[PATCH] Prevent infinite growth of DenseMap/DenseSet
Philip Reames
listmail at philipreames.com
Wed Oct 30 09:57:54 PDT 2013
On 10/29/13 12:59 PM, Howard Hinnant wrote:
> Please review:
>
> Rehash but don't grow when full of tombstones.
>
> This problem was found and fixed by José Fonseca in March 2011 for SmallPtrSet, committed r128566. But as far as I can tell, all other llvm hash tables retain the same problem: the bucket count can grow without bound while size() remains near constant by repeated insert/erase cycles that tend to fill the container with tombstones. Here is a demo that has been reduced to a trivial case:
>
> int
> main()
> {
> llvm::DenseSet<unsigned> d;
> for (unsigned i = 0; i < 0xFFFFFFF; ++i)
> {
> d.insert(i);
> d.erase(i);
> }
> }
>
> While the container size() never grows above 1, the bucket count grows like this:
>
> nb = 64
> nb = 128
> nb = 256
> nb = 512
> nb = 1024
> nb = 2048
> nb = 4096
> nb = 8192
> nb = 16384
> nb = 32768
> nb = 65536
> nb = 131072
> nb = 262144
> nb = 524288
> nb = 1048576
> nb = 2097152
> nb = 4194304
> nb = 8388608
> nb = 16777216
> nb = 33554432
> nb = 67108864
> nb = 134217728
> nb = 268435456
>
> The above program currently consumes a few GB ram. Here is a patch that keeps the bucket count commensurate with size():
Good catch. Two requests:
1) A comment explaining the edge case which makes the modified code
important. (i.e. so future readers don't make the same mistake.)
2) A test case which will break if this behavior regresses.
Neither are absolutely mandatory, both would be good to have.
Philip
More information about the llvm-commits
mailing list