Rewrite StroredObject logic

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.

src/cpp/shared-table.cpp

@@ -9,13 +9,11 @@ namespace effil {
 
 SharedTable::SharedTable() noexcept
         : GCObject(),
-          lock_(new SpinMutex()),
+          data_(std::make_shared<SharedData>()) {
-          data_(new std::unordered_map<StoredObject, StoredObject>()){
 }
 
 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<SpinMutex> g(*lock_);
+    std::lock_guard<SpinMutex> g(data_->lock);
 
-    if (key.isGCObject()) refs_->insert(key.gcHandle());
+    if (key->gcHandle()) refs_->insert(key->gcHandle());
-    if (value.isGCObject()) refs_->insert(value.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<SpinMutex> g(*lock_);
+        std::lock_guard<SpinMutex> g(data_->lock);
 
         // in this case object is not obligatory to own data
-        auto it = (*data_).find(key);
+        auto it = data_->entries.find(key);
-        if (it != (*data_).end()) {
+        if (it != data_->entries.end()) {
-            if (it->first.isGCObject()) refs_->erase(it->first.gcHandle());
+            if (it->first->gcHandle()) refs_->erase(it->first->gcHandle());
-            if (it->second.isGCObject()) refs_->erase(it->second.gcHandle());
+            if (it->second->gcHandle()) refs_->erase(it->second->gcHandle());
-            (*data_).erase(it);
+            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);
+    StoredObject cppKey = createStoredObject(key);
-    std::lock_guard<SpinMutex> g(*lock_);
+    std::lock_guard<SpinMutex> g(data_->lock);
-    auto val = (*data_).find(cppKey);
+    auto val = data_->entries.find(cppKey);
-    if (val == (*data_).end()) {
+    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<SpinMutex> g(*lock_);
+    std::lock_guard<SpinMutex> g(data_->lock);
-    return data_->size();
+    return data_->entries.size();
 }
 
 } // effil

src/cpp/shared-table.h

@@ -27,8 +27,14 @@ public:
     size_t size() const noexcept;
 
 private:
-    mutable std::shared_ptr<SpinMutex> lock_;
+    typedef std::unique_ptr<BaseHolder> StoredObject;
-    std::shared_ptr<std::unordered_map<StoredObject, StoredObject>> data_;
+    struct SharedData {
+        SpinMutex lock;
+        std::unordered_map<StoredObject, StoredObject, StoredObjectHash, StoredObjectEqual> entries;
+    };
+
+private:
+    std::shared_ptr<SharedData> data_;
 };
 
 } // effil

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 {
+    bool rawCompare(const BaseHolder* other) const noexcept final {
-        return BaseHolder::compare(other) && static_cast<const PrimitiveHolder<StoredType>*>(other)->data_ == data_;
+        return static_cast<const PrimitiveHolder<StoredType>*>(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 {
+    bool rawCompare(const BaseHolder* other) const noexcept final {
-        return BaseHolder::compare(other) && static_cast<const FunctionHolder*>(other)->function_ == function_;
+        return static_cast<const FunctionHolder*>(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 {
+    bool rawCompare(const BaseHolder *other) const noexcept final {
-        return BaseHolder::compare(other) && static_cast<const TableHolder*>(other)->handle_ == handle_;
+        return static_cast<const TableHolder*>(other)->handle_ == handle_;
     }
 
     std::size_t hash() const noexcept final {
@@ -96,7 +96,7 @@ public:
         return sol::make_object(state, *static_cast<SharedTable*>(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<SharedTable>();
             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<typename SolObject>
-std::unique_ptr<BaseHolder> 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<BaseHolder> fromSolObject(const SolObject& luaObject) {
 
 } // namespace
 
-StoredObject::StoredObject(StoredObject&& init) noexcept
+StoredObject createStoredObject(bool value) {
-        : data_(std::move(init.data_)) {}
+    return std::make_unique<PrimitiveHolder<bool>>(value);
-
-StoredObject::StoredObject(GCObjectHandle handle) noexcept
-        : data_(new TableHolder(handle)) {
 }
 
-StoredObject::StoredObject(const sol::object& object)
+StoredObject createStoredObject(double value) {
-        : data_(fromSolObject(object)) {
+    return std::make_unique<PrimitiveHolder<double>>(value);
 }
 
-StoredObject::StoredObject(const sol::stack_object& object)
+StoredObject createStoredObject(const std::string& value) {
-        : data_(fromSolObject(object)) {
+    return std::make_unique<PrimitiveHolder<std::string>>(value);
 }
 
-StoredObject::operator bool() const noexcept {
+StoredObject createStoredObject(const sol::object &object) {
-    return (bool)data_;
+    return fromSolObject(object);
 }
 
-std::size_t StoredObject::hash() const noexcept {
+StoredObject createStoredObject(const sol::stack_object &object) {
-    if (data_)
+    return fromSolObject(object);
-        return data_->hash();
-    else
-        return 0;
 }
 
-sol::object StoredObject::unpack(sol::this_state state) const {
+StoredObject createStoredObject(GCObjectHandle handle) {
-    if (data_)
+    return std::make_unique<TableHolder>(handle);
-        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();
 }
 
 } // effil

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 {
+typedef std::unique_ptr<BaseHolder> StoredObject;
-public:
-    StoredObject() = default;
-    StoredObject(StoredObject&& init) noexcept;
-    StoredObject(GCObjectHandle) noexcept;
-    StoredObject(const sol::object&);
-    StoredObject(const sol::stack_object&);
 
-
+struct StoredObjectHash {
-    operator bool() const noexcept;
+    size_t operator()(const StoredObject& o) const {
-    std::size_t hash() const noexcept;
+        return o->hash();
-    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<BaseHolder> 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<effil::StoredObject> {
-    std::size_t operator()(const effil::StoredObject &object) const noexcept {
-        return object.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

src/cpp/threading.cpp

@@ -44,8 +44,8 @@ LuaThread::LuaThread(std::shared_ptr<ThreadData> threadData, const std::string&
 
     std::vector<sol::object> arguments;
     for(const auto& iter: args) {
-        StoredObject store(iter.get<sol::object>());
+        StoredObject store = createStoredObject(iter.get<sol::object>());
-        arguments.push_back(store.unpack(sol::this_state{pThreadData_->luaState}));
+        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> 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<sol::object>());
+            StoredObject store = createStoredObject(iter.get<sol::object>());
             threadData->results.emplace_back(std::move(store));
         }
         threadData->status = ThreadStatus::Completed;
@@ -106,7 +106,7 @@ void LuaThread::work(std::shared_ptr<ThreadData> 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<sol::object>(threadData->luaState));
+        StoredObject store = createStoredObject(sol::stack::pop<sol::object>(threadData->luaState));
         threadData->results.emplace_back(std::move(store));
     }
 }
@@ -139,7 +139,7 @@ std::tuple<sol::object, sol::table> 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));