greg7mdp / parallel-hashmap

A family of header-only, very fast and memory-friendly hashmap and btree containers.
https://greg7mdp.github.io/parallel-hashmap/
Apache License 2.0
2.53k stars 239 forks source link

Does `try_emplace_l` behave as expected when contains key #161

Closed xinyiZzz closed 2 years ago

xinyiZzz commented 2 years ago

Hi, the following code init A will print three times, when contains key, try_emplace_l will still call the constructor, although it will not be added to the map again.

Is it more expected not to call the constructor? This is more flexible when multithreaded, and seems to avoid invalid constructs

Similar issue #79, thanks

#include <string>
#include <iostream>
#include "parallel_hashmap/phmap.h"

class A {
public:
    A() { std::cout << "init A" << std::endl; }
};

using pmap = phmap::parallel_flat_hash_map<
        std::string, A*, phmap::priv::hash_default_hash<std::string>,
        phmap::priv::hash_default_eq<std::string>,
        std::allocator<std::pair<const std::string, A*>>, 12, std::mutex>;

int main() {
    pmap tm;
    tm.try_emplace_l("key1", [](auto) {}, new A() );
    tm.try_emplace_l("key1", [](auto) {}, new A() );
    tm.try_emplace_l("key1", [](auto) {}, new A());
}

________________
init A
init A
init A
________________

try_emplace_l documented

// if map does not contains key, it is inserted and the mapped value is value-constructed 
    // with the provided arguments (if any), as with try_emplace. 
    // if map already  contains key, then the lambda is called with the mapped value (under 
    // write lock protection) and can update the mapped value.
    // returns true if key was not already present, false otherwise.
greg7mdp commented 2 years ago

Hi @xinyiZzz , you cannot do what you want with try_emplace_l, because the call to new is evaluated before the function is called.

However you can use lazy_emplace_l which does what you want:

#include <string>
#include <iostream>
#include "parallel_hashmap/phmap.h"

class A {
public:
    A() { std::cout << "init A" << std::endl; }
};

using pmap = phmap::parallel_flat_hash_map<
        std::string, A*, phmap::priv::hash_default_hash<std::string>,
        phmap::priv::hash_default_eq<std::string>,
        std::allocator<std::pair<const std::string, A*>>, 12, std::mutex>;

int main() {
    pmap tm;

    const char *k = "key1";

    tm.lazy_emplace_l(k, [](auto) {}, [&](const pmap::constructor& ctor) { ctor(k, new A()); } );
    tm.lazy_emplace_l(k, [](auto) {}, [&](const pmap::constructor& ctor) { ctor(k, new A()); } );
    tm.lazy_emplace_l(k, [](auto) {}, [&](const pmap::constructor& ctor) { ctor(k, new A()); } );
    tm.lazy_emplace_l(k, [](auto) {}, [&](const pmap::constructor& ctor) { ctor(k, new A()); } );
}

The first lambda is called only when key was already present. The second one construct value_type in place when key was not present.

xinyiZzz commented 2 years ago

@greg7mdp Thanks for your quick reply, actually I ended up using lazy_emplace_l to solve it.

Thanks again xinyi