diff --git a/src/snmalloc/ds/combininglock.h b/src/snmalloc/ds/combininglock.h index f710dd5d4..6d0fab38e 100644 --- a/src/snmalloc/ds/combininglock.h +++ b/src/snmalloc/ds/combininglock.h @@ -9,7 +9,15 @@ namespace snmalloc { class CombineLockNode; - using CombiningLock = std::atomic; + + struct CombiningLock + { + // Fast path lock incase there is no contention. + std::atomic flag{false}; + + // MCS queue of work items + std::atomic head{nullptr}; + }; /** * @brief Combinations of MCS queue lock with Flat Combining @@ -52,13 +60,30 @@ namespace snmalloc // Stores the C++ lambda associated with this node in the queue. void (*f_raw)(CombineLockNode*); + void release(CombiningLock& lock) + { + lock.flag.store(false, std::memory_order_release); + } + public: constexpr CombineLockNode(void (*f)(CombineLockNode*)) : f_raw(f) {} void attach(CombiningLock& lock) { - // Add to the queue of pending work - auto prev = lock.exchange(this); + // Start by assuming no contention and attempt to acquire the lock. + if (lock.flag.exchange(true, std::memory_order_acq_rel) == false) + { + // We grabbed the lock. + f_raw(this); + + // Release the lock + release(lock); + return; + } + + // There is contention for the lock, we need to add our work to the + // queue of pending work + auto prev = lock.head.exchange(this); if (prev != nullptr) { @@ -73,10 +98,22 @@ namespace snmalloc if (status.load() == LockStatus::DONE) return; } - // We could add an else branch here that could set - // status = LockStatus::Ready - // However, the subsequent state assumes it is Ready, and - // nothing would read it. + else + { + // We are the head of the queue. Spin until we acquire the fast path lock. + while (lock.flag.exchange(true, std::memory_order_acquire)) + { + while (lock.flag.load(std::memory_order_relaxed)) + { + Aal::pause(); + } + } + + // We could set + // status = LockStatus::Ready + // However, the subsequent state assumes it is Ready, and + // nothing would read it. + } // We are the head of the queue, and responsible for // waking/performing our and subsequent work. @@ -100,11 +137,12 @@ namespace snmalloc // This could be the end of the queue, attempt to close the // queue. auto curr_c = curr; - if (lock.compare_exchange_strong(curr_c, nullptr)) + if (lock.head.compare_exchange_strong(curr_c, nullptr)) { // Queue was successfully closed. // Notify last element the work was completed. curr->status = LockStatus::DONE; + release(lock); return; } @@ -114,7 +152,7 @@ namespace snmalloc Aal::pause(); // As we had to wait, give the job to the next thread - // to carry on performing the work. + // to carry on performing the work, and release the flag. curr->next.load()->status = LockStatus::READY; // Notify the thread that we completed its work.