manugoyal-nuro
commented
6 years ago Yes! Both good suggestions. We had actually run some experiments in the past with doing precisely what you're suggesting, which is moving buckets lazily by storing state in the spinlock class, and I think we observed minimal performance difference (when running on all cores).
Definitely agreed that eating up all CPUs is not very friendly in a real-world deployment, and so it probably makes sense to revisit this lazy-resizing strategy.
manugoyal
There is no need to increment at that line: https://github.com/efficient/libcuckoo/blob/86d5ef0da4fe52ba431704f374d1a7c79df99acb/libcuckoo/cuckoohash_map.hh#L1645
Also, when resizing the hash map triggers as many thread as there are CPU core on the system for a very intensive tasks. I think this is good for benchmark but not so for real code. In a real application, it should only schedule work on the threads actually using the map and not bother the other (which might be unrelated). I think the spinlock class should call a callback when it's trying to acquire the lock and fails for numerous time. The callback, by default does nothing. When a resizing operation is started by one thread, the other thread that wants to use the map will be blocked on the locks (because resizing takes all locks) and will call the callback. The callback function will see a resizing is happening and will then assign some work to the querying thread. Please notice that, since spinlocks are using alignas(64) there is plenty of space left to store a pointer to the map without actually loosing anything.
Thus, instead of currently where, when a resizing happens, one thread (a cpu core) is actually resizing/moving bucket and the other threads are burning CPU in spinlocking (and eventually will be scheduled to perform some resizing), the scheme above will have all the participating threads to help instead and avoid burning CPU time for nothing.