search

efficient / libcuckoo

A high-performance, concurrent hash table
Other
1.61k stars 275 forks source link

examples/count_freq.cc does not work #16

Closed tewarfel closed 9 years ago

tewarfel commented 9 years ago

First of all, thank you for sharing this code with us, and for the C++ templates; I'm looking forward to playing around with this more in the near future. I wanted to share a minor problem I encountered with one of the examples in hope of sparing others some unnecessary frustration.

Problem: In examples/count_freq.cc, the update function called by upsert will insert "1" upon the first encounter of a key, but will not update the counter's value on the subsequent occurrences of that key.

Test Environment: Ubuntu x64 w/ gcc 4.8.4-2ubuntu1~14.04, and on OSX LLVM 6.1.0 (clang-602.0.53).

Issue: The update function in count_freq.cc (as provided) is:

auto updatefn = [](const size_t& num) { return num+1; };

The value returned by updatefn is equal to num+1, but the function doesn't change the actual value of num, and is precluded from doing so by the const declaration.

If I change the definition to 

auto updatefn = [](size_t& num) {num++; return num;};  // (losing the const, explicitly incrementing the counter)

then the program seems to execute as expected on both Ubuntu and on OSX.
(FWIW, I also get a ton of gcc -Wshadow warnings under Ubuntu gcc once I make this change, but none under Xcode clang.)

manugoyal commented 9 years ago

Thanks for pointing that out! Just pushed a couple of commits that should address these issues:

https://github.com/efficient/libcuckoo/commit/998e76401976c70a3ab9e5a17935b6eab7c085ed should fix the update signature problem, and https://github.com/efficient/libcuckoo/commit/fda62ced7b29a6819335a3d49d5b59549c433c39 should fix the warnings.