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

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

Open
abhinav04sharma opened this issue Nov 18, 2017 · 9 comments
Open

Comments

@abhinav04sharma
Copy link

abhinav04sharma commented Nov 18, 2017

TURF_CALL_MEMBER (*self->target, self->pmf)();

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
Copy link
Author

abhinav04sharma commented Nov 18, 2017

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

@preshing
Copy link
Owner

preshing commented Nov 19, 2017

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
Copy link
Author

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
Copy link
Owner

preshing commented Nov 20, 2017

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
Copy link

brucejee commented Aug 10, 2022

@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
Copy link
Owner

Can junction concurrent map support shared_ptr as value type?

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

@brucejee
Copy link

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

@preshing
Copy link
Owner

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
Copy link

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.

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

3 participants