[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