Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Concurrent increment of value keep getting unexpected results #255

Closed
dakabang opened this issue Oct 22, 2024 · 8 comments
Closed

Concurrent increment of value keep getting unexpected results #255

dakabang opened this issue Oct 22, 2024 · 8 comments

Comments

@dakabang
Copy link

Hi @greg7mdp

I found out that concurrent increment of value using parallel_flat_hash_map keep getting unexpected results, in main.cc,
I use 10 threads to increase value of same key 1000 times, instead of getting 10 * 1000 = 10000, the program keep getting wrong results.

compile command

g++ -o main main.cc -isystem parallel-hashmap/ -std=c++17 -lpthread

main.cc

#include <assert.h>
#include <parallel_hashmap/phmap.h>
#include <shared_mutex>
#include <stdio.h>
#include <thread>
#include <vector>

template <typename K> using HashEqual = phmap::priv::hash_default_eq<K>;

template <typename V> using Hash = phmap::priv::hash_default_hash<V>;

template <typename K> using Allocator = phmap::priv::Allocator<K>;

template <typename K, typename V, size_t N>
using parallel_flat_hash_map =
    phmap::parallel_flat_hash_map<K, V, Hash<K>, HashEqual<K>,
                                  Allocator<phmap::priv::Pair<K, V>>, N,
                                  std::shared_mutex>;

using Table = parallel_flat_hash_map<int, int, 10>;

static constexpr int THREADS = 10;
static constexpr int EPOCH = 1000;
static constexpr int KEY = 12345;

void Incr(Table *table) {
  auto exist_fn = [](typename Table::value_type &value) { value.second += 1; };
  auto emplace_fn = [](const typename Table::constructor &ctor) {
    ctor(KEY, 1);
  };
  for (int i = 0; i < EPOCH; ++i) {
    (void)table->lazy_emplace_l(KEY, exist_fn, emplace_fn);
  }
}

int main() {
  Table table;
  std::vector<std::thread> threads;
  threads.reserve(THREADS);
  for (int i = 0; i < THREADS; ++i) {
    threads.emplace_back([&table]() { Incr(&table); });
  }

  for (auto &thread : threads) {
    thread.join();
  }

  printf("table[%d]=%d\n", KEY, table[KEY]);
  assert(table[KEY] == 10000);
  return 0;
}
@dakabang dakabang changed the title Concurrent increment of value not as expected. Concurrent increment of value keep getting unexpected results Oct 22, 2024
@greg7mdp
Copy link
Owner

Hi @dakabang , you are right, there is an issue with std::shared_mutex.
If you replace it with std::mutex you do get the expected result.

Thanks for reporting the issue. I'll have a look at it.

@dakabang
Copy link
Author

Thanks for your reply, I'm concerned that the implementation of std::mutex may have performance degradation compared to std::shared_mutex, I'm looking forward for this issue to be fixed.

greg7mdp added a commit that referenced this issue Oct 28, 2024
@greg7mdp
Copy link
Owner

Resolved with c9520f1

greg7mdp added a commit to greg7mdp/gtl that referenced this issue Oct 28, 2024
@dakabang
Copy link
Author

Excellent! Thanks for your help.

@dakabang
Copy link
Author

@greg7mdp One more request, could you please release a latest version?

@greg7mdp
Copy link
Owner

Sure, @dakabang , I will, but I may not be able to do it before next weekend though. Thanks for the bug report (and using phmap :-)

@greg7mdp
Copy link
Owner

Actually, I can tag a new release now, but will update vcpkg and conan later.

@greg7mdp
Copy link
Owner

New release 1.4.1 tagged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants