From 0b5108a8a0d9b7a4a63bd6aae0271b71a887beea Mon Sep 17 00:00:00 2001 From: Benoit Germain Date: Thu, 25 Apr 2024 13:36:47 +0200 Subject: C++ integration: cleanup in Linda/Keeper interaction --- src/keeper.cpp | 47 ++++---------- src/keeper.h | 7 +-- src/linda.cpp | 190 ++++++++++++++++++++++++--------------------------------- src/linda.h | 70 +++++++++++++++++++++ 4 files changed, 164 insertions(+), 150 deletions(-) create mode 100644 src/linda.h diff --git a/src/keeper.cpp b/src/keeper.cpp index c5bbb9d..c306d58 100644 --- a/src/keeper.cpp +++ b/src/keeper.cpp @@ -40,6 +40,7 @@ #include "keeper.h" #include "compat.h" +#include "linda.h" #include "state.h" #include "tools.h" #include "uniquekey.h" @@ -207,16 +208,17 @@ static void push_table(lua_State* L, int idx_) // ################################################################################################# -int keeper_push_linda_storage(Universe* U, DestState L, void* ptr_, uintptr_t magic_) +// only used by linda:dump() and linda:__towatch() for debugging purposes +int keeper_push_linda_storage(Linda& linda_, DestState L) { - Keeper* const K{ which_keeper(U->keepers, magic_) }; + Keeper* const K{ linda_.whichKeeper() }; SourceState const KL{ K ? K->L : nullptr }; if (KL == nullptr) return 0; STACK_GROW(KL, 4); // KEEPER MAIN STACK_CHECK_START_REL(KL, 0); FIFOS_KEY.pushValue(KL); // fifos - lua_pushlightuserdata(KL, ptr_); // fifos ud + lua_pushlightuserdata(KL, &linda_); // fifos ud lua_rawget(KL, -2); // fifos storage lua_remove(KL, -2); // storage if (!lua_istable(KL, -1)) @@ -229,7 +231,7 @@ int keeper_push_linda_storage(Universe* U, DestState L, void* ptr_, uintptr_t ma STACK_GROW(L, 5); STACK_CHECK_START_REL(L, 0); lua_newtable(L); // out - InterCopyContext c{ U, L, KL, {}, {}, {}, LookupMode::FromKeeper, {} }; + InterCopyContext c{ linda_.U, L, KL, {}, {}, {}, LookupMode::FromKeeper, {} }; lua_pushnil(KL); // storage nil while (lua_next(KL, -2)) // storage key fifo { @@ -748,37 +750,14 @@ void init_keepers(Universe* U, lua_State* L) // ################################################################################################# -// should be called only when inside a keeper_acquire/keeper_release pair (see Linda::ProtectedCall) -Keeper* which_keeper(Keepers* keepers_, uintptr_t magic_) +Keeper* Linda::acquireKeeper() const { - int const nbKeepers{ keepers_->nb_keepers }; - if (nbKeepers) - { - unsigned int i = (unsigned int) ((magic_ >> KEEPER_MAGIC_SHIFT) % nbKeepers); - return &keepers_->keeper_array[i]; - } - return nullptr; -} - -// ################################################################################################# - -Keeper* keeper_acquire(Keepers* keepers_, uintptr_t magic_) -{ - int const nbKeepers{ keepers_->nb_keepers }; + int const nbKeepers{ U->keepers->nb_keepers }; // can be 0 if this happens during main state shutdown (lanes is being GC'ed -> no keepers) if (nbKeepers) { - /* - * Any hashing will do that maps pointers to 0..GNbKeepers-1 - * consistently. - * - * Pointers are often aligned by 8 or so - ignore the low order bits - * have to cast to unsigned long to avoid compilation warnings about loss of data when converting pointer-to-integer - */ - unsigned int i = (unsigned int)((magic_ >> KEEPER_MAGIC_SHIFT) % nbKeepers); - Keeper* K = &keepers_->keeper_array[i]; + Keeper* const K{ &U->keepers->keeper_array[m_keeper_index] }; K->m_mutex.lock(); - //++ K->count; return K; } return nullptr; @@ -786,12 +765,12 @@ Keeper* keeper_acquire(Keepers* keepers_, uintptr_t magic_) // ################################################################################################# -void keeper_release(Keeper* K) +void Linda::releaseKeeper(Keeper* K_) const { - //-- K->count; - if (K) + if (K_) // can be nullptr if we tried to acquire during shutdown { - K->m_mutex.unlock(); + assert(K_ == &U->keepers->keeper_array[m_keeper_index]); + K_->m_mutex.unlock(); } } diff --git a/src/keeper.h b/src/keeper.h index ef86f94..4c79fd7 100644 --- a/src/keeper.h +++ b/src/keeper.h @@ -15,6 +15,7 @@ extern "C" { #include // forwards +class Linda; enum class LookupMode; class Universe; @@ -34,18 +35,14 @@ struct Keepers Keeper keeper_array[1]; }; -static constexpr uintptr_t KEEPER_MAGIC_SHIFT{ 3 }; // crc64/we of string "NIL_SENTINEL" generated at http://www.nitrxgen.net/hashgen/ static constexpr UniqueKey NIL_SENTINEL{ 0x7EAAFA003A1D11A1ull, "linda.null" }; void init_keepers(Universe* U, lua_State* L); void close_keepers(Universe* U); -[[nodiscard]] Keeper* which_keeper(Keepers* keepers_, uintptr_t magic_); -[[nodiscard]] Keeper* keeper_acquire(Keepers* keepers_, uintptr_t magic_); -void keeper_release(Keeper* K_); void keeper_toggle_nil_sentinels(lua_State* L, int val_i_, LookupMode const mode_); -[[nodiscard]] int keeper_push_linda_storage(Universe* U, DestState L, void* ptr_, uintptr_t magic_); +[[nodiscard]] int keeper_push_linda_storage(Linda& linda_, DestState L); using keeper_api_t = lua_CFunction; #define KEEPER_API(_op) keepercall_##_op diff --git a/src/linda.cpp b/src/linda.cpp index a19b126..0a1ca3f 100644 --- a/src/linda.cpp +++ b/src/linda.cpp @@ -30,21 +30,22 @@ THE SOFTWARE. =============================================================================== */ +#include "linda.h" + #include "compat.h" -#include "deep.h" #include "keeper.h" #include "lanes_private.h" #include "threading.h" #include "tools.h" #include "universe.h" -#include #include -#include // xxh64 of string "CANCEL_ERROR" generated at https://www.pelock.com/products/hash-calculator static constexpr UniqueKey BATCH_SENTINEL{ 0x2DDFEE0968C62AA7ull, "linda.batched" }; +// ################################################################################################# + class LindaFactory : public DeepFactory { private: @@ -57,118 +58,85 @@ class LindaFactory : public DeepFactory // I'm not totally happy with having a global variable. But since it's stateless, it will do for the time being. static LindaFactory g_LindaFactory; +// ################################################################################################# // ################################################################################################# -/* -* Actual data is kept within a keeper state, which is hashed by the 'Linda' -* pointer (which is same to all userdatas pointing to it). -*/ -class Linda : public DeepPrelude // Deep userdata MUST start with this header +// Any hashing will do that maps pointers to [0..Universe::nb_keepers[ consistently. +// Pointers are often aligned by 8 or so - ignore the low order bits +// have to cast to unsigned long to avoid compilation warnings about loss of data when converting pointer-to-integer +static constexpr uintptr_t KEEPER_MAGIC_SHIFT{ 3 }; + +Linda::Linda(Universe* U_, LindaGroup group_, char const* name_, size_t len_) +: DeepPrelude{ g_LindaFactory } +, U{ U_ } +, m_keeper_index{ (group_ ? group_ : static_cast(std::bit_cast(this) >> KEEPER_MAGIC_SHIFT)) % U_->keepers->nb_keepers } { - private: + setName(name_, len_); +} - static constexpr size_t kEmbeddedNameLength = 24; - using EmbeddedName = std::array; - struct AllocatedName - { - size_t len{ 0 }; - char* name{ nullptr }; - }; - // depending on the name length, it is either embedded inside the Linda, or allocated separately - std::variant m_name; - - public: - - std::condition_variable m_read_happened; - std::condition_variable m_write_happened; - Universe* const U; // the universe this linda belongs to - uintptr_t const group; // a group to control keeper allocation between lindas - CancelRequest simulate_cancel{ CancelRequest::None }; - - public: - - // a fifo full userdata has one uservalue, the table that holds the actual fifo contents - [[nodiscard]] static void* operator new(size_t size_, Universe* U_) noexcept { return U_->internal_allocator.alloc(size_); } - // always embedded somewhere else or "in-place constructed" as a full userdata - // can't actually delete the operator because the compiler generates stack unwinding code that could call it in case of exception - static void operator delete(void* p_, Universe* U_) { U_->internal_allocator.free(p_, sizeof(Linda)); } - // this one is for us, to make sure memory is freed by the correct allocator - static void operator delete(void* p_) { static_cast(p_)->U->internal_allocator.free(p_, sizeof(Linda)); } - - Linda(Universe* U_, uintptr_t group_, char const* name_, size_t len_) - : DeepPrelude{ g_LindaFactory } - , U{ U_ } - , group{ group_ << KEEPER_MAGIC_SHIFT } - { - setName(name_, len_); - } +// ################################################################################################# - ~Linda() +Linda::~Linda() +{ + if (std::holds_alternative(m_name)) { - if (std::holds_alternative(m_name)) - { - AllocatedName& name = std::get(m_name); - U->internal_allocator.free(name.name, name.len); - } + AllocatedName& name = std::get(m_name); + U->internal_allocator.free(name.name, name.len); } +} - static int ProtectedCall(lua_State* L, lua_CFunction f_); - - private : +// ################################################################################################# - void setName(char const* name_, size_t len_) +void Linda::setName(char const* name_, size_t len_) +{ + // keep default + if (!name_ || len_ == 0) { - // keep default - if (!name_ || len_ == 0) - { - return; - } - ++len_; // don't forget terminating 0 - if (len_ < kEmbeddedNameLength) - { - m_name.emplace(); - char* const name{ std::get(m_name).data() }; - memcpy(name, name_, len_); - } - else - { - AllocatedName& name = std::get(m_name); - name.name = static_cast(U->internal_allocator.alloc(len_)); - name.len = len_; - memcpy(name.name, name_, len_); - } + return; } + ++len_; // don't forget terminating 0 + if (len_ < kEmbeddedNameLength) + { + m_name.emplace(); + char* const name{ std::get(m_name).data() }; + memcpy(name, name_, len_); + } + else + { + AllocatedName& name = std::get(m_name); + name.name = static_cast(U->internal_allocator.alloc(len_)); + name.len = len_; + memcpy(name.name, name_, len_); + } +} - public: - - uintptr_t hashSeed() const { return group ? group : std::bit_cast(this); } +// ################################################################################################# - char const* getName() const +char const* Linda::getName() const +{ + if (std::holds_alternative(m_name)) { - if (std::holds_alternative(m_name)) - { - AllocatedName const& name = std::get(m_name); - return name.name; - } - if (std::holds_alternative(m_name)) - { - char const* const name{ std::get(m_name).data() }; - return name; - } - return nullptr; + AllocatedName const& name = std::get(m_name); + return name.name; } -}; + if (std::holds_alternative(m_name)) + { + char const* const name{ std::get(m_name).data() }; + return name; + } + return nullptr; +} // ################################################################################################# template -[[nodiscard]] static inline Linda* ToLinda(lua_State* L, int idx_) +[[nodiscard]] static inline Linda* ToLinda(lua_State* L_, int idx_) { - Linda* const linda{ static_cast(g_LindaFactory.toDeep(L, idx_)) }; + Linda* const linda{ static_cast(g_LindaFactory.toDeep(L_, idx_)) }; if constexpr (!OPT) { - luaL_argcheck(L, linda != nullptr, idx_, "expecting a linda object"); - LUA_ASSERT(L, linda->U == universe_get(L)); + luaL_argcheck(L_, linda != nullptr, idx_, "expecting a linda object"); // doesn't return if linda is nullptr + LUA_ASSERT(L_, linda->U == universe_get(L_)); } return linda; } @@ -213,7 +181,7 @@ int Linda::ProtectedCall(lua_State* L, lua_CFunction f_) Linda* const linda{ ToLinda(L, 1) }; // acquire the keeper - Keeper* const K{ keeper_acquire(linda->U->keepers, linda->hashSeed()) }; + Keeper* const K{ linda->acquireKeeper() }; lua_State* const KL{ K ? K->L : nullptr }; if (KL == nullptr) return 0; @@ -229,7 +197,7 @@ int Linda::ProtectedCall(lua_State* L, lua_CFunction f_) lua_settop(KL, 0); // release the keeper - keeper_release(K); + linda->releaseKeeper(K); // if there was an error, forward it if (rc != LUA_OK) @@ -306,7 +274,7 @@ LUAG_FUNC(linda_send) KeeperCallResult pushed; { Lane* const lane{ LANE_POINTER_REGKEY.readLightUserDataValue(L) }; - Keeper* const K{ which_keeper(linda->U->keepers, linda->hashSeed()) }; + Keeper* const K{ linda->whichKeeper() }; KeeperState const KL{ K ? K->L : nullptr }; if (KL == nullptr) return 0; @@ -472,7 +440,7 @@ LUAG_FUNC(linda_receive) } Lane* const lane{ LANE_POINTER_REGKEY.readLightUserDataValue(L) }; - Keeper* const K{ which_keeper(linda->U->keepers, linda->hashSeed()) }; + Keeper* const K{ linda->whichKeeper() }; KeeperState const KL{ K ? K->L : nullptr }; if (KL == nullptr) return 0; @@ -584,7 +552,7 @@ LUAG_FUNC(linda_set) // make sure the key is of a valid type (throws an error if not the case) check_key_types(L, 2, 2); - Keeper* const K{ which_keeper(linda->U->keepers, linda->hashSeed()) }; + Keeper* const K{ linda->whichKeeper() }; KeeperCallResult pushed; if (linda->simulate_cancel == CancelRequest::None) { @@ -639,7 +607,7 @@ LUAG_FUNC(linda_count) // make sure the keys are of a valid type check_key_types(L, 2, lua_gettop(L)); - Keeper* const K{ which_keeper(linda->U->keepers, linda->hashSeed()) }; + Keeper* const K{ linda->whichKeeper() }; KeeperCallResult const pushed{ keeper_call(linda->U, K->L, KEEPER_API(count), L, linda, 2) }; return OptionalValue(pushed, L, "tried to count an invalid key"); }; @@ -667,7 +635,7 @@ LUAG_FUNC(linda_get) KeeperCallResult pushed; if (linda->simulate_cancel == CancelRequest::None) { - Keeper* const K{ which_keeper(linda->U->keepers, linda->hashSeed()) }; + Keeper* const K{ linda->whichKeeper() }; pushed = keeper_call(linda->U, K->L, KEEPER_API(get), L, linda, 2); if (pushed.value_or(0) > 0) { @@ -709,7 +677,7 @@ LUAG_FUNC(linda_limit) KeeperCallResult pushed; if (linda->simulate_cancel == CancelRequest::None) { - Keeper* const K{ which_keeper(linda->U->keepers, linda->hashSeed()) }; + Keeper* const K{ linda->whichKeeper() }; pushed = keeper_call(linda->U, K->L, KEEPER_API(limit), L, linda, 2); LUA_ASSERT(L, pushed.has_value() && (pushed.value() == 0 || pushed.value() == 1)); // no error, optional boolean value saying if we should wake blocked writer threads if (pushed.value() == 1) @@ -863,7 +831,7 @@ LUAG_FUNC(linda_dump) auto dump = [](lua_State* L) { Linda* const linda{ ToLinda(L, 1) }; - return keeper_push_linda_storage(linda->U, DestState{ L }, linda, linda->hashSeed()); + return keeper_push_linda_storage(*linda, DestState{ L }); }; return Linda::ProtectedCall(L, dump); } @@ -871,13 +839,13 @@ LUAG_FUNC(linda_dump) // ################################################################################################# /* - * table = linda:dump() - * return a table listing all pending data inside the linda + * table/string = linda:__towatch() + * return a table listing all pending data inside the linda, or the stringified linda if empty */ LUAG_FUNC(linda_towatch) { Linda* const linda{ ToLinda(L, 1) }; - int pushed{ keeper_push_linda_storage(linda->U, DestState{ L }, linda, linda->hashSeed()) }; + int pushed{ keeper_push_linda_storage(*linda, DestState{ L }) }; if (pushed == 0) { // if the linda is empty, don't return nil @@ -892,7 +860,7 @@ DeepPrelude* LindaFactory::newDeepObjectInternal(lua_State* L) const { size_t name_len = 0; char const* linda_name = nullptr; - unsigned long linda_group = 0; + LindaGroup linda_group{ 0 }; // should have a string and/or a number of the stack as parameters (name and group) switch (lua_gettop(L)) { @@ -906,13 +874,13 @@ DeepPrelude* LindaFactory::newDeepObjectInternal(lua_State* L) const } else { - linda_group = (unsigned long) lua_tointeger(L, -1); + linda_group = LindaGroup{ static_cast(lua_tointeger(L, -1)) }; } break; case 2: // 2 parameters, a name and group, in that order linda_name = lua_tolstring(L, -2, &name_len); - linda_group = (unsigned long) lua_tointeger(L, -1); + linda_group = LindaGroup{ static_cast(lua_tointeger(L, -1)) }; break; } @@ -932,7 +900,7 @@ void LindaFactory::deleteDeepObjectInternal(lua_State* L, DeepPrelude* o_) const { Linda* const linda{ static_cast(o_) }; LUA_ASSERT(L, linda); - Keeper* const myK{ which_keeper(linda->U->keepers, linda->hashSeed()) }; + Keeper* const myK{ linda->whichKeeper() }; // if collected after the universe, keepers are already destroyed, and there is nothing to clear if (myK) { @@ -940,13 +908,13 @@ void LindaFactory::deleteDeepObjectInternal(lua_State* L, DeepPrelude* o_) const // because we are already inside a protected area, and trying to do so would deadlock! bool const need_acquire_release{ myK->L != L }; // Clean associated structures in the keeper state. - Keeper* const K{ need_acquire_release ? keeper_acquire(linda->U->keepers, linda->hashSeed()) : myK }; + Keeper* const K{ need_acquire_release ? linda->acquireKeeper() : myK }; // hopefully this won't ever raise an error as we would jump to the closest pcall site while forgetting to release the keeper mutex... [[maybe_unused]] KeeperCallResult const result{ keeper_call(linda->U, K->L, KEEPER_API(clear), L, linda, 0) }; LUA_ASSERT(L, result.has_value() && result.value() == 0); if (need_acquire_release) { - keeper_release(K); + linda->releaseKeeper(K); } } diff --git a/src/linda.h b/src/linda.h new file mode 100644 index 0000000..559ef24 --- /dev/null +++ b/src/linda.h @@ -0,0 +1,70 @@ +#pragma once + +#include "cancel.h" +#include "deep.h" +#include "keeper.h" +#include "universe.h" + +#include +#include +#include + +struct Keeper; + +// ################################################################################################# + +using LindaGroup = Unique; + +class Linda : public DeepPrelude // Deep userdata MUST start with this header +{ + private: + + static constexpr size_t kEmbeddedNameLength = 24; + using EmbeddedName = std::array; + struct AllocatedName + { + size_t len{ 0 }; + char* name{ nullptr }; + }; + // depending on the name length, it is either embedded inside the Linda, or allocated separately + std::variant m_name; + + public: + + std::condition_variable m_read_happened; + std::condition_variable m_write_happened; + Universe* const U{ nullptr }; // the universe this linda belongs to + int const m_keeper_index{ -1 }; // the keeper associated to this linda + CancelRequest simulate_cancel{ CancelRequest::None }; + + public: + + // a fifo full userdata has one uservalue, the table that holds the actual fifo contents + [[nodiscard]] static void* operator new(size_t size_, Universe* U_) noexcept { return U_->internal_allocator.alloc(size_); } + // always embedded somewhere else or "in-place constructed" as a full userdata + // can't actually delete the operator because the compiler generates stack unwinding code that could call it in case of exception + static void operator delete(void* p_, Universe* U_) { U_->internal_allocator.free(p_, sizeof(Linda)); } + // this one is for us, to make sure memory is freed by the correct allocator + static void operator delete(void* p_) { static_cast(p_)->U->internal_allocator.free(p_, sizeof(Linda)); } + + ~Linda(); + Linda(Universe* U_, LindaGroup group_, char const* name_, size_t len_); + Linda() = delete; + Linda(Linda const&) = delete; + Linda(Linda const&&) = delete; + Linda& operator=(Linda const&) = delete; + Linda& operator=(Linda const&&) = delete; + + static int ProtectedCall(lua_State* L, lua_CFunction f_); + + private : + + void setName(char const* name_, size_t len_); + + public: + + [[nodiscard]] char const* getName() const; + [[nodiscard]] Keeper* whichKeeper() const { return U->keepers->nb_keepers ? &U->keepers->keeper_array[m_keeper_index] : nullptr; } + [[nodiscard]] Keeper* acquireKeeper() const; + void releaseKeeper(Keeper* keeper_) const; +}; -- cgit v1.2.3-55-g6feb