From ca245ab64361053114b369c98e0ac57e951ced10 Mon Sep 17 00:00:00 2001 From: Ilia Udalov Date: Sat, 11 Feb 2017 02:58:54 +0300 Subject: [PATCH] Rewrite StroredObject logic --- src/cpp/garbage-collector.h | 2 + src/cpp/shared-table.cpp | 42 ++++++++++----------- src/cpp/shared-table.h | 10 ++++- src/cpp/stored-object.cpp | 75 ++++++++++++------------------------- src/cpp/stored-object.h | 61 ++++++++++++------------------ src/cpp/threading.cpp | 10 ++--- 6 files changed, 82 insertions(+), 118 deletions(-) diff --git a/src/cpp/garbage-collector.h b/src/cpp/garbage-collector.h index bde6df3..434cb96 100644 --- a/src/cpp/garbage-collector.h +++ b/src/cpp/garbage-collector.h @@ -11,6 +11,8 @@ namespace effil { // Unique handle for all objects spawned from one object. typedef void* GCObjectHandle; +static const GCObjectHandle GCNull = nullptr; + // All effil objects that owned in lua code have to inherit this class. // This type o object can persist in multiple threads and in multiple lua states. // Child has to care about storing data and concurrent access. diff --git a/src/cpp/shared-table.cpp b/src/cpp/shared-table.cpp index 9765a91..0d6ea2d 100644 --- a/src/cpp/shared-table.cpp +++ b/src/cpp/shared-table.cpp @@ -9,13 +9,11 @@ namespace effil { SharedTable::SharedTable() noexcept : GCObject(), - lock_(new SpinMutex()), - data_(new std::unordered_map()){ + data_(std::make_shared()) { } SharedTable::SharedTable(const SharedTable& init) noexcept : GCObject(init), - lock_(init.lock_), data_(init.data_) { } @@ -31,50 +29,50 @@ sol::object SharedTable::getUserType(sol::state_view &lua) noexcept { } void SharedTable::set(StoredObject&& key, StoredObject&& value) noexcept { - std::lock_guard g(*lock_); + std::lock_guard g(data_->lock); - if (key.isGCObject()) refs_->insert(key.gcHandle()); - if (value.isGCObject()) refs_->insert(value.gcHandle()); + if (key->gcHandle()) refs_->insert(key->gcHandle()); + if (value->gcHandle()) refs_->insert(value->gcHandle()); - (*data_)[std::move(key)] = std::move(value); + data_->entries[std::move(key)] = std::move(value); } void SharedTable::luaSet(const sol::stack_object& luaKey, const sol::stack_object& luaValue) { ASSERT(luaKey.valid()) << "Invalid table index"; - StoredObject key(luaKey); + StoredObject key = createStoredObject(luaKey); if (luaValue.get_type() == sol::type::nil) { - std::lock_guard g(*lock_); + std::lock_guard g(data_->lock); // in this case object is not obligatory to own data - auto it = (*data_).find(key); - if (it != (*data_).end()) { - if (it->first.isGCObject()) refs_->erase(it->first.gcHandle()); - if (it->second.isGCObject()) refs_->erase(it->second.gcHandle()); - (*data_).erase(it); + auto it = data_->entries.find(key); + if (it != data_->entries.end()) { + if (it->first->gcHandle()) refs_->erase(it->first->gcHandle()); + if (it->second->gcHandle()) refs_->erase(it->second->gcHandle()); + data_->entries.erase(it); } } else { - set(std::move(key), StoredObject(luaValue)); + set(std::move(key), createStoredObject(luaValue)); } } sol::object SharedTable::luaGet(const sol::stack_object& key, const sol::this_state& state) const { ASSERT(key.valid()); - StoredObject cppKey(key); - std::lock_guard g(*lock_); - auto val = (*data_).find(cppKey); - if (val == (*data_).end()) { + StoredObject cppKey = createStoredObject(key); + std::lock_guard g(data_->lock); + auto val = data_->entries.find(cppKey); + if (val == data_->entries.end()) { return sol::nil; } else { - return val->second.unpack(state); + return val->second->unpack(state); } } size_t SharedTable::size() const noexcept { - std::lock_guard g(*lock_); - return data_->size(); + std::lock_guard g(data_->lock); + return data_->entries.size(); } } // effil diff --git a/src/cpp/shared-table.h b/src/cpp/shared-table.h index d674ff1..e03e5d9 100644 --- a/src/cpp/shared-table.h +++ b/src/cpp/shared-table.h @@ -27,8 +27,14 @@ public: size_t size() const noexcept; private: - mutable std::shared_ptr lock_; - std::shared_ptr> data_; + typedef std::unique_ptr StoredObject; + struct SharedData { + SpinMutex lock; + std::unordered_map entries; + }; + +private: + std::shared_ptr data_; }; } // effil diff --git a/src/cpp/stored-object.cpp b/src/cpp/stored-object.cpp index d3d7341..f6c6f94 100644 --- a/src/cpp/stored-object.cpp +++ b/src/cpp/stored-object.cpp @@ -25,8 +25,8 @@ public: PrimitiveHolder(const StoredType& init) noexcept : data_(init) {} - bool compare(const BaseHolder* other) const noexcept final { - return BaseHolder::compare(other) && static_cast*>(other)->data_ == data_; + bool rawCompare(const BaseHolder* other) const noexcept final { + return static_cast*>(other)->data_ == data_; } std::size_t hash() const noexcept final { @@ -51,8 +51,8 @@ public: function_ = dumper(luaObject); } - bool compare(const BaseHolder* other) const noexcept final { - return BaseHolder::compare(other) && static_cast(other)->function_ == function_; + bool rawCompare(const BaseHolder* other) const noexcept final { + return static_cast(other)->function_ == function_; } std::size_t hash() const noexcept final { @@ -84,8 +84,8 @@ public: TableHolder(GCObjectHandle handle) noexcept : handle_(handle) {} - bool compare(const BaseHolder *other) const noexcept final { - return BaseHolder::compare(other) && static_cast(other)->handle_ == handle_; + bool rawCompare(const BaseHolder *other) const noexcept final { + return static_cast(other)->handle_ == handle_; } std::size_t hash() const noexcept final { @@ -96,7 +96,7 @@ public: return sol::make_object(state, *static_cast(getGC().get(handle_))); } - GCObjectHandle handle() const noexcept { return handle_; } + GCObjectHandle gcHandle() const noexcept override { return handle_; } private: GCObjectHandle handle_; }; @@ -120,12 +120,12 @@ StoredObject makeStoredObject(sol::object luaObject, SolTableToShared& visited) SharedTable table = getGC().create(); visited.emplace_back(std::make_pair(luaTable, table.handle())); dumpTable(&table, luaTable, visited); - return StoredObject(table.handle()); + return createStoredObject(table.handle()); } else { - return StoredObject(st->second); + return createStoredObject(st->second); } } else { - return StoredObject(luaObject); + return createStoredObject(luaObject); } } @@ -136,7 +136,7 @@ void dumpTable(SharedTable* target, sol::table luaTable, SolTableToShared& visit } template -std::unique_ptr fromSolObject(const SolObject& luaObject) { +StoredObject fromSolObject(const SolObject& luaObject) { switch(luaObject.get_type()) { case sol::type::nil: break; @@ -172,57 +172,28 @@ std::unique_ptr fromSolObject(const SolObject& luaObject) { } // namespace -StoredObject::StoredObject(StoredObject&& init) noexcept - : data_(std::move(init.data_)) {} - -StoredObject::StoredObject(GCObjectHandle handle) noexcept - : data_(new TableHolder(handle)) { +StoredObject createStoredObject(bool value) { + return std::make_unique>(value); } -StoredObject::StoredObject(const sol::object& object) - : data_(fromSolObject(object)) { +StoredObject createStoredObject(double value) { + return std::make_unique>(value); } -StoredObject::StoredObject(const sol::stack_object& object) - : data_(fromSolObject(object)) { +StoredObject createStoredObject(const std::string& value) { + return std::make_unique>(value); } -StoredObject::operator bool() const noexcept { - return (bool)data_; +StoredObject createStoredObject(const sol::object &object) { + return fromSolObject(object); } -std::size_t StoredObject::hash() const noexcept { - if (data_) - return data_->hash(); - else - return 0; +StoredObject createStoredObject(const sol::stack_object &object) { + return fromSolObject(object); } -sol::object StoredObject::unpack(sol::this_state state) const { - if (data_) - return data_->unpack(state); - else - return sol::nil; -} - -bool StoredObject::isGCObject() const noexcept { - return data_->type() == typeid(TableHolder); -} - -GCObjectHandle StoredObject::gcHandle() const noexcept { - return ((TableHolder*)data_.get())->handle(); -} - -StoredObject& StoredObject::operator=(StoredObject&& o) noexcept { - data_ = std::move(o.data_); - return *this; -} - -bool StoredObject::operator==(const StoredObject& o) const noexcept { - if (data_ && o.data_) - return data_->compare(o.data_.get()); - else - return data_.get() == o.data_.get(); +StoredObject createStoredObject(GCObjectHandle handle) { + return std::make_unique(handle); } } // effil diff --git a/src/cpp/stored-object.h b/src/cpp/stored-object.h index ecd0c32..a58bd89 100644 --- a/src/cpp/stored-object.h +++ b/src/cpp/stored-object.h @@ -11,54 +11,41 @@ public: BaseHolder() = default; virtual ~BaseHolder() = default; - virtual bool compare(const BaseHolder* other) const noexcept { return typeid(*this) == typeid(*other); } + bool compare(const BaseHolder* other) const noexcept { + return typeid(*this) == typeid(*other) && rawCompare(other); + } + virtual bool rawCompare(const BaseHolder* other) const = 0; virtual const std::type_info& type() { return typeid(*this); } virtual std::size_t hash() const noexcept = 0; virtual sol::object unpack(sol::this_state state) const = 0; + virtual GCObjectHandle gcHandle() const noexcept { return GCNull; } + private: BaseHolder(const BaseHolder&) = delete; BaseHolder(BaseHolder&) = delete; }; -class StoredObject { -public: - StoredObject() = default; - StoredObject(StoredObject&& init) noexcept; - StoredObject(GCObjectHandle) noexcept; - StoredObject(const sol::object&); - StoredObject(const sol::stack_object&); +typedef std::unique_ptr StoredObject; - - operator bool() const noexcept; - std::size_t hash() const noexcept; - sol::object unpack(sol::this_state state) const; - - bool isGCObject() const noexcept; - GCObjectHandle gcHandle() const noexcept; - - StoredObject& operator=(StoredObject&& o) noexcept; - bool operator==(const StoredObject& o) const noexcept; - -private: - std::unique_ptr data_; - -private: - StoredObject(const StoredObject&) = delete; - StoredObject& operator=(const StoredObject&) = delete; -}; - -} // effil - -namespace std { - -// For storing as key in std::unordered_map -template<> -struct hash { - std::size_t operator()(const effil::StoredObject &object) const noexcept { - return object.hash(); +struct StoredObjectHash { + size_t operator()(const StoredObject& o) const { + return o->hash(); } }; -} // std +struct StoredObjectEqual { + bool operator()(const StoredObject& lhs, const StoredObject& rhs) const { + return lhs->compare(rhs.get()); + } +}; + +StoredObject createStoredObject(bool); +StoredObject createStoredObject(double); +StoredObject createStoredObject(const std::string&); +StoredObject createStoredObject(GCObjectHandle); +StoredObject createStoredObject(const sol::object &); +StoredObject createStoredObject(const sol::stack_object &); + +} // effil diff --git a/src/cpp/threading.cpp b/src/cpp/threading.cpp index 4b207f7..cd2f398 100644 --- a/src/cpp/threading.cpp +++ b/src/cpp/threading.cpp @@ -44,8 +44,8 @@ LuaThread::LuaThread(std::shared_ptr threadData, const std::string& std::vector arguments; for(const auto& iter: args) { - StoredObject store(iter.get()); - arguments.push_back(store.unpack(sol::this_state{pThreadData_->luaState})); + StoredObject store = createStoredObject(iter.get()); + arguments.push_back(store->unpack(sol::this_state{pThreadData_->luaState})); } pThread_.reset(new std::thread(&LuaThread::work, pThreadData_, function, std::move(arguments))); @@ -95,7 +95,7 @@ void LuaThread::work(std::shared_ptr threadData, const std::string s (void)results; // TODO: try to avoid use of useless sol::function_result here sol::variadic_args args(threadData->luaState, -lua_gettop(threadData->luaState)); for(const auto& iter: args) { - StoredObject store(iter.get()); + StoredObject store = createStoredObject(iter.get()); threadData->results.emplace_back(std::move(store)); } threadData->status = ThreadStatus::Completed; @@ -106,7 +106,7 @@ void LuaThread::work(std::shared_ptr threadData, const std::string s catch (const sol::error& err) { threadData->status = ThreadStatus::Failed; sol::stack::push(threadData->luaState, err.what()); - StoredObject store(sol::stack::pop(threadData->luaState)); + StoredObject store = createStoredObject(sol::stack::pop(threadData->luaState)); threadData->results.emplace_back(std::move(store)); } } @@ -139,7 +139,7 @@ std::tuple LuaThread::wait(sol::this_state state) const { for (const StoredObject& obj: pThreadData_->results) { - returns.add(obj.unpack(state)); + returns.add(obj->unpack(state)); } } return std::make_tuple(sol::make_object(state, threadStatusToString(stat)), std::move(returns));