efficient / libcuckoo

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

wanted: upsert_fn, get a value to be inserted by assigned function, which may be called or not #35

Closed MRYingLEE closed 7 years ago

MRYingLEE commented 8 years ago

upsert is very useful.

But before calling upsert, a new value has to be created, even the value may be not needed for insert won't happen.

In order to simply the logic and to minimize the memory management, can you provide upsert_fn, such as:

template <typename Updater, typename K, typename Getter> typename std::enable_if< std::is_convertible<Updater, updater_type>::value, void>::type upsert_fn(K&& key, Updater fn, Getter gt);

Thank you in advance, Ying

manugoyal commented 8 years ago

I think with some C++11 trickery, you can get decent performance for upsert without changing the interface. Note that the value parameter type for upsert is Args&&... val. This is similar to the emplace syntax of STL containers, where you can pass constructor arguments as the value, and the value will only be constructed if the insert actually happens.

For example, if your value was something expensive to construct, like a vector<int>, you could write upsert(key, updater, 1, value), where 1 and value are the count and value that would be perfectly forwarded and passed to the vector constructor only if we ended up doing an insert. If the insert is not done, the vector constructor will never be called.

Is it possible for you to modify your call to upsert to take advantage of this feature?

MRYingLEE commented 8 years ago

Yes, your solution is helpful, but I prefer a more general way. In my situation, a new value is created by a logic, not from a basic constructor. I believe my requirement is general for everyone.

I tried to create a new function type: typedef std::function<mapped_type()> getter_type; And to define upsert_fn,


    template <typename Updater, typename K, typename Getter>
    typename std::enable_if<
        std::is_convertible<Updater, updater_type>::value,
        void>::type upsert_fn(K&& key, Updater fn, Getter gt) {
        size_t hv = hashed_key(key);
        cuckoo_status st;
        do {
            auto b = snapshot_and_lock_two(hv);
            size_t hp = get_hashpower();
            st = cuckoo_update_fn(key, fn, hv, b.i[0], b.i[1]);
            if (st == ok) {
                break;
            }

            // We run an insert, since the update failed. Since we already have
            // the locks, we don't run cuckoo_insert_loop immediately, to avoid
            // releasing and re-grabbing the locks. Recall, that the locks will
            // be released at the end of this call to cuckoo_insert.
            st = cuckoo_insert(hv, std::move(b), std::forward<K>(key),
                gt());
            if (st == failure_table_full) {
                cuckoo_fast_double(hp);
                // Retry until the insert doesn't fail due to expansion.
                if (cuckoo_insert_loop(hv, std::forward<K>(key),gt())) {
                    break;
                }
                // The only valid reason for failure is a duplicate key. In this
                // case, we retry the entire upsert_fn operation.
            }
        } while (st != ok);
    }

but I met some pure tech problem to deal with Args&&... val. Even the compiling failed.

I hope someone can help.

Thank you in advance, Ying

manugoyal commented 7 years ago

Thanks for the feedback. I think this is still best solved by adding a constructor to the value type, or wrapping the value type in a new class with your own default constructor. Having a special getter method is a bit more complicated of an interface, and also would require an extra call to the copy/move constructor.