Any reason to call a member function of the object in QSBR?

[abhinav04sharma]

https://github.com/preshing/junction/blob/5e0de1150b2aff1cf4a5b5c47a7cc9b06d50663c/junction/QSBR.h#L74

Since QSBR accepts a member function of the object here it's not clear how we can use this for object destruction. We cannot pass it the destructor of the object since that is forbidden in C++. It would be nice if we could pass arbitrary function here which takes the pointer (i.e. map element's value) as a parameter like this:

void enqueue(void (T::*func)(T*), T* target) {
       struct Closure {
            void (T::*func)(T*);
            T* target;
            static void thunk(void* param) {
                Closure* self = (Closure*) param;
                self->(*func)(target);
            }
        };
        Closure closure = {func, target};
        turf::LockGuard<turf::Mutex> guard(m_mutex);
        TURF_RACE_DETECT_GUARD(m_flushRaceDetector);
        m_deferredActions.push_back(Action(Closure::thunk, &closure, sizeof(closure)));
}

Then we could just call delete value in the callback function to free the object. For example:

struct Foo {
  static void destroy(Foo *ptr) {
    delete ptr;
  }
}

auto value= map.erase(key);
junction::DefaultQSBR.enqueue(&Foo::destroy, value);

I was wondering if there is any reason to enforce a member function in enqueue().

[abhinav04sharma]

@preshing I'll be happy to send a pull request for this if you think it makes sense.

[preshing]

I don't think your example will compile because you made Foo::destroy a static method, but your enqueue function still expects a pointer to member.

Why not just make a member function that deletes itself? That seems to accomplish what you want.

struct Foo {
  void destroy() {
    delete this;
  }
};

auto value= map.erase(key);
junction::DefaultQSBR.enqueue(&Foo::destroy, value);

[abhinav04sharma]

That's is exactly what I'm doing currently, but object suicide is just doesn't look idiomatic :) My example might be a little crude but I'm sure we can make this work with any arbitrary function. I opened this to find out if there was any specific reason to restrict the function to be a member of the object.

[preshing]

delete this is legal: https://isocpp.org/wiki/faq/freestore-mgmt#delete-this

To answer your question, the callback is a member of an object:

• to avoid the need for a void * cast inside the callback
• to avoid needing a virtual function
• because QSBR-managed classes are usually designed specifically for QSBR
• because it's easy to step into in Debug mode (compared to std::function)

Do you have a specific example where a different calling convention would offer a practical benefit over the current approach?

[brucejee]

@preshing I can use enqueue to destroy dynamic allocated object now? Can junction concurrent map support shared_ptr<V> as value type? I appreciate your help, thank you very much!

Does this wrapper code is correct？I want know the Insert and Clear member function will work properly？

// K must integer type
template <typename K, typename V>
class LeapfrogMap {
   public:
    using ConcurrentMap = typename junction::ConcurrentMap_Leapfrog<K, V*>;
    using MapIterator = typename ConcurrentMap::Iterator;
    using MapMutator = typename ConcurrentMap::Mutator;

    LeapfrogMap() {}
    ~LeapfrogMap() {}

    // insert value only when not exist
    template <typename... Args>
    inline bool InsertOrFind(K key, V*& value, Args && ... args) {
        value = m_hashMap.get(key);
        if (!value) {
            turf::LockGuard<turf::Mutex> lock(m_mutex);
            MapMutator mutator = m_hashMap.insertOrFind(key);
            value = mutator.getValue();
            if (!value) {
                value = new V(std::forward<Args>(args)...);
                mutator.assignValue(value);
                return true;
            }
        }
        return false;
    }

    // upsert function
    inline void Insert(K key, V* value) {
        static_assert(std::is_integral<K>::value, "Key type must be integer.");

        MapMutator mutator = m_hashMap.insertOrFind(key);
        V* oldValue = mutator.exchangeValue(value);
        if (oldValue != nullptr && oldValue != value) {
            junction::DefaultQSBR.enqueue(&V::Destroy, oldValue);
        }
    }

    inline bool Get(K key, V*& value) {
        static_assert(std::is_integral<K>::value, "Key type must be integer.");

        value = m_hashMap.get(key);
        return value != nullptr;
    }

    inline void Delete(K key) {
        static_assert(std::is_integral<K>::value, "Key type must be integer.");

        V* oldValue = m_hashMap.erase(key);
        if (oldValue != nullptr) {
            junction::DefaultQSBR.enqueue(&V::Destroy, oldValue);
        }
    }

    // 遍历比较耗时
    inline void Clear() {
        static_assert(std::is_integral<K>::value, "Key type must be integer.");

        MapIterator mapIter(m_hashMap);
        while (mapIter.isValid()) {
            junction::DefaultQSBR.enqueue(&V::Destroy, mapIter.getValue());
            m_hashMap.erase(mapIter.getKey());
            mapIter.next();
        }
    }

    // 遍历比较耗时
    inline void DoFunc(std::function<void(K key, V* value)> func) {
        if (func == nullptr) {
            return;
        }

        static_assert(std::is_integral<K>::value, "Key type must be integer.");

        MapIterator mapIter(m_hashMap);
        while (mapIter.isValid()) {
            func(mapIter.getKey(), mapIter.getValue());
            mapIter.next();
        }
    }

    // 对指定Key执行一段逻辑，如果满足条件就删除
    inline void DoFuncThenDelete(const K& key, std::function<bool(V* value)> func) {
        if (func == nullptr) {
            return;
        }

        static_assert(std::is_integral<K>::value, "Key type must be integer.");

        V* value = m_hashMap.get(key);
        if (value != nullptr && func(value)) {
            V* oldValue = m_hashMap.erase(key);
            if (oldValue != nullptr) {
                junction::DefaultQSBR.enqueue(&V::Destroy, oldValue);
            }
        }
    }

   private:
    ConcurrentMap m_hashMap;
    turf::Mutex m_mutex;
};

[preshing]

Can junction concurrent map support shared_ptr<V> as value type?

The answer to this question is still no at this time. Sorry!

[brucejee]

Thanks for your quick reply. Can you help me to check the wrapper code above, its Insert and Clear can work properly？

[preshing]

Sure, I took a quick look. That code looks pretty sensible, however, it appears that you're using junction's QSBR to defer the destruction of each item in your map. To be honest, I didn't expect that kind of use for junction's QSBR. It's entirely possible that it works, but it might not be very scalable, because every call to QSBR::enqueue involves a lock. But if you just want to gain more confidence that the code works as intended, it would help to write some multithreaded tests (similar to the tests already in junction). Ideally those tests would verify that all items were safely destroyed and their memory freed.

[brucejee]

Thank you very much，I have no idea how to safely destroy dynamic allocated object in the map，at this time i only can use the enqueue method.