From 7daf9d0df46451302d44dbb56179f98b18ec2fdb Mon Sep 17 00:00:00 2001 From: mihacooper Date: Mon, 23 Jan 2017 23:53:42 +0300 Subject: [PATCH 1/2] optimize some data transferring and throw sol::error instead of writing to std::cerr --- src/shared-table.cpp | 13 +++++-------- src/shared-table.h | 6 +++--- src/stored-object.cpp | 23 ++++++++++------------- src/stored-object.h | 8 ++++---- src/threading.cpp | 20 +++++++++----------- src/threading.h | 2 -- src/utils.h | 30 +++++++++++++++++++++++++----- 7 files changed, 56 insertions(+), 46 deletions(-) diff --git a/src/shared-table.cpp b/src/shared-table.cpp index 188ef45..db3a6a3 100644 --- a/src/shared-table.cpp +++ b/src/shared-table.cpp @@ -16,14 +16,13 @@ sol::object SharedTable::getUserType(sol::state_view &lua) noexcept { return sol::stack::pop(lua); } -void SharedTable::set(StoredObject key, StoredObject value) noexcept { +void SharedTable::set(StoredObject&& key, StoredObject&& value) noexcept { std::lock_guard g(lock_); data_[std::move(key)] = std::move(value); } -void SharedTable::luaSet(sol::stack_object luaKey, sol::stack_object luaValue) { - if (!luaKey.valid()) - throw sol::error("Invalid table index"); +void SharedTable::luaSet(const sol::stack_object& luaKey, const sol::stack_object& luaValue) { + ASSERT(luaKey.valid()) << "Invalid table index"; StoredObject key(luaKey); if (luaValue.get_type() == sol::type::nil) { @@ -31,13 +30,11 @@ void SharedTable::luaSet(sol::stack_object luaKey, sol::stack_object luaValue) { // in this case object is not obligatory to own data data_.erase(key); } else { - StoredObject value(luaValue); - std::lock_guard g(lock_); - data_[std::move(key)] = std::move(value); + set(std::move(key), StoredObject(luaValue)); } } -sol::object SharedTable::luaGet(sol::stack_object key, sol::this_state state) const noexcept { +sol::object SharedTable::luaGet(const sol::stack_object& key, const sol::this_state& state) const noexcept { assert(key.valid()); StoredObject cppKey(key); diff --git a/src/shared-table.h b/src/shared-table.h index f1126be..5ab3cf9 100644 --- a/src/shared-table.h +++ b/src/shared-table.h @@ -17,12 +17,12 @@ public: virtual ~SharedTable() = default; static sol::object getUserType(sol::state_view &lua) noexcept; - void set(StoredObject, StoredObject) noexcept; + void set(StoredObject&&, StoredObject&&) noexcept; size_t size() const noexcept; public: // lua bindings - void luaSet(sol::stack_object luaKey, sol::stack_object luaValue); - sol::object luaGet(sol::stack_object key, sol::this_state state) const noexcept; + void luaSet(const sol::stack_object& luaKey, const sol::stack_object& luaValue); + sol::object luaGet(const sol::stack_object& key, const sol::this_state& state) const noexcept; protected: mutable SpinMutex lock_; diff --git a/src/stored-object.cpp b/src/stored-object.cpp index 4cf2702..93441d7 100644 --- a/src/stored-object.cpp +++ b/src/stored-object.cpp @@ -16,17 +16,17 @@ namespace { template class PrimitiveHolder : public BaseHolder { public: - PrimitiveHolder(sol::stack_object luaObject) noexcept + PrimitiveHolder(const sol::stack_object& luaObject) noexcept : BaseHolder(luaObject.get_type()), data_(luaObject.as()) {} - PrimitiveHolder(sol::object luaObject) noexcept + PrimitiveHolder(const sol::object& luaObject) noexcept : BaseHolder(luaObject.get_type()), data_(luaObject.as()) {} PrimitiveHolder(sol::type type, const StoredType& init) noexcept : BaseHolder(type), data_(init) {} bool rawCompare(const BaseHolder* other) const noexcept final { - assert(type_ == other->type()); + ASSERT(type_ == other->type()); return static_cast*>(other)->data_ == data_; } @@ -50,7 +50,7 @@ public: sol::state_view lua(luaObject.lua_state()); sol::function dumper = lua["string"]["dump"]; - assert(dumper.valid()); + ASSERT(dumper.valid()); function_ = dumper(luaObject); } @@ -65,14 +65,11 @@ public: sol::object unpack(sol::this_state state) const noexcept final { sol::state_view lua((lua_State*)state); sol::function loader = lua["loadstring"]; - assert(loader.valid()); + ASSERT(loader.valid()); sol::function result = loader(function_); - if (!result.valid()) { - ERROR << "Unable to restore function!" << std::endl; - ERROR << "Content:" << std::endl; - ERROR << function_ << std::endl; - } + ASSERT(result.valid()) << "Unable to restore function!" << std::endl + << "Content:" << std::endl << function_ << std::endl; return sol::make_object(state, result); } @@ -115,7 +112,7 @@ void dumpTable(SharedTable* target, sol::table luaTable, SolTableToShared& visit } template -std::unique_ptr fromSolObject(SolObject luaObject) { +std::unique_ptr fromSolObject(const SolObject& luaObject) { switch(luaObject.get_type()) { case sol::type::nil: break; @@ -158,11 +155,11 @@ StoredObject::StoredObject(SharedTable* table) noexcept : data_(new PrimitiveHolder(sol::type::userdata, table)) { } -StoredObject::StoredObject(sol::object object) noexcept +StoredObject::StoredObject(const sol::object& object) noexcept : data_(fromSolObject(object)) { } -StoredObject::StoredObject(sol::stack_object object) noexcept +StoredObject::StoredObject(const sol::stack_object& object) noexcept : data_(fromSolObject(object)) { } diff --git a/src/stored-object.h b/src/stored-object.h index 87f1d53..d5addc3 100644 --- a/src/stored-object.h +++ b/src/stored-object.h @@ -1,6 +1,6 @@ #pragma once -#include +#include #include #include @@ -17,7 +17,7 @@ public: } bool compare(const BaseHolder* other) const noexcept { - assert(other != nullptr); + ASSERT(other != nullptr); return type_ == other->type_ && rawCompare(other); } @@ -40,8 +40,8 @@ public: StoredObject() = default; StoredObject(StoredObject&& init) noexcept; StoredObject(SharedTable*) noexcept; - StoredObject(sol::object) noexcept; - StoredObject(sol::stack_object) noexcept; + StoredObject(const sol::object&) noexcept; + StoredObject(const sol::stack_object&) noexcept; operator bool() const noexcept; std::size_t hash() const noexcept; diff --git a/src/threading.cpp b/src/threading.cpp index b53e807..9e7af23 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -9,7 +9,7 @@ LuaThread::LuaThread(const sol::function& function, const sol::variadic_args& ar // 2. Create new state p_state_.reset(new sol::state); - assert(p_state_.get() != NULL); + ASSERT(p_state_.get() != NULL); p_state_->open_libraries( sol::lib::base, sol::lib::string, sol::lib::package, sol::lib::io, sol::lib::os @@ -22,7 +22,7 @@ LuaThread::LuaThread(const sol::function& function, const sol::variadic_args& ar // 4. Run thread p_thread_.reset(new std::thread(&LuaThread::work, this)); - assert(p_thread_.get() != NULL); + ASSERT(p_thread_.get() != NULL); } void LuaThread::storeArgs(const sol::variadic_args &args) noexcept { @@ -49,15 +49,13 @@ void LuaThread::detach() noexcept { } void LuaThread::work() noexcept { - if (p_state_.get() && p_arguments_.get()) { - std::string func_owner = std::move(str_function_); - std::shared_ptr state_owner = p_state_; - std::shared_ptr> arguments_owner = p_arguments_; - sol::function_result func = (*state_owner)["loadstring"](func_owner); - func.get()(sol::as_args(*arguments_owner)); - } else { - throw sol::error("Internal error: invalid thread Lua state"); - } + ASSERT(p_state_.get() && p_arguments_.get()) << "invalid thread Lua state" << std::endl; + + std::string func_owner = std::move(str_function_); + std::shared_ptr state_owner = p_state_; + std::shared_ptr> arguments_owner = p_arguments_; + sol::function_result func = (*state_owner)["loadstring"](func_owner); + func.get()(sol::as_args(*arguments_owner)); } std::string LuaThread::threadId() noexcept { diff --git a/src/threading.h b/src/threading.h index 6e5087a..f685e59 100644 --- a/src/threading.h +++ b/src/threading.h @@ -2,8 +2,6 @@ #include "shared-table.h" -#include - #include #include #include diff --git a/src/utils.h b/src/utils.h index ce16d3b..d9b435a 100644 --- a/src/utils.h +++ b/src/utils.h @@ -1,9 +1,29 @@ #pragma once #include +#include + +#include + +namespace utils { + +class ExceptionThrower +{ +public: + void operator =(const std::ostream& os) + { + throw sol::error(static_cast(os).str()); + } + + operator bool() + { + return true; + } + +}; + +} + +#define ERROR if (auto thr = utils::ExceptionThrower()) thr = std::stringstream() << __FILE__ << ":" << __LINE__ << std::endl +#define ASSERT(cond) if (!(cond)) ERROR << "In condition '" << #cond << "': " -#ifdef NDEBUG -# define ERROR if(false) std::cerr -#else -# define ERROR std::cerr -#endif \ No newline at end of file From 91a87084ffb0001a32c7a7a23b0e8cd464354005 Mon Sep 17 00:00:00 2001 From: mihacooper Date: Tue, 24 Jan 2017 19:27:33 +0300 Subject: [PATCH 2/2] fix review comments --- src/stored-object.cpp | 6 +++--- src/threading.cpp | 2 +- src/utils.h | 28 +++++++++++++++++----------- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/stored-object.cpp b/src/stored-object.cpp index 93441d7..a549d06 100644 --- a/src/stored-object.cpp +++ b/src/stored-object.cpp @@ -68,8 +68,8 @@ public: ASSERT(loader.valid()); sol::function result = loader(function_); - ASSERT(result.valid()) << "Unable to restore function!" << std::endl - << "Content:" << std::endl << function_ << std::endl; + ASSERT(result.valid()) << "Unable to restore function!\n" + << "Content:\n" << function_; return sol::make_object(state, result); } @@ -141,7 +141,7 @@ std::unique_ptr fromSolObject(const SolObject& luaObject) { return std::make_unique>(sol::type::userdata, table); } default: - ERROR << "Unable to store object of that type: " << (int)luaObject.get_type() << std::endl; + ERROR << "Unable to store object of that type: " << (int)luaObject.get_type() << "\n"; } return nullptr; } diff --git a/src/threading.cpp b/src/threading.cpp index 9e7af23..d54d2a1 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -49,7 +49,7 @@ void LuaThread::detach() noexcept { } void LuaThread::work() noexcept { - ASSERT(p_state_.get() && p_arguments_.get()) << "invalid thread Lua state" << std::endl; + ASSERT(p_state_.get() && p_arguments_.get()) << "invalid thread Lua state\n"; std::string func_owner = std::move(str_function_); std::shared_ptr state_owner = p_state_; diff --git a/src/utils.h b/src/utils.h index d9b435a..07cd984 100644 --- a/src/utils.h +++ b/src/utils.h @@ -5,25 +5,31 @@ #include +namespace effil { namespace utils { -class ExceptionThrower -{ +class Exception : public sol::error { public: - void operator =(const std::ostream& os) - { - throw sol::error(static_cast(os).str()); + Exception() noexcept : sol::error("") {} + + template + Exception& operator<<(const T& value) { + std::stringstream ss; + ss << value; + message_ += ss.str(); + return *this; } - operator bool() - { - return true; + virtual const char* what() const noexcept override { + return message_.c_str(); } +private: + std::string message_; }; -} +} // utils +} // effil -#define ERROR if (auto thr = utils::ExceptionThrower()) thr = std::stringstream() << __FILE__ << ":" << __LINE__ << std::endl +#define ERROR throw effil::utils::Exception() << __FILE__ << ":" << __LINE__ #define ASSERT(cond) if (!(cond)) ERROR << "In condition '" << #cond << "': " -