From 64628c17572f29248cdb7bb2069e84c2b3be1b22 Mon Sep 17 00:00:00 2001 From: Ilia Date: Sun, 16 Apr 2017 14:08:24 +0300 Subject: [PATCH] GC API for lua (#43) Almost old gc api, but: rebased on new master GC::State -> enabled_ flag release lua state as soon as possible --- src/cpp/garbage-collector.cpp | 56 +++++++++++++---------- src/cpp/garbage-collector.h | 46 ++++++++++++------- src/cpp/lua-module.cpp | 10 +++-- src/cpp/shared-table.cpp | 9 ++-- src/cpp/stored-object.cpp | 8 ++-- src/cpp/threading.cpp | 39 +++++++++------- src/lua/effil.lua | 1 + tests/cpp/garbage-collector.cpp | 80 ++++++++++++++++----------------- tests/cpp/shared-table.cpp | 6 +-- tests/lua/gc.lua | 40 +++++++++++++++++ tests/lua/run_tests.lua | 1 + tests/lua/test_utils.lua | 4 ++ 12 files changed, 188 insertions(+), 112 deletions(-) create mode 100644 tests/lua/gc.lua diff --git a/src/cpp/garbage-collector.cpp b/src/cpp/garbage-collector.cpp index 67981b0..731d4db 100644 --- a/src/cpp/garbage-collector.cpp +++ b/src/cpp/garbage-collector.cpp @@ -7,13 +7,12 @@ namespace effil { -GarbageCollector::GarbageCollector() - : state_(GCState::Idle) +GC::GC() + : enabled_(true) , lastCleanup_(0) , step_(200) {} -GCObject* GarbageCollector::get(GCObjectHandle handle) { - std::lock_guard g(lock_); +GCObject* GC::findObject(GCObjectHandle handle) { auto it = objects_.find(handle); if (it == objects_.end()) { DEBUG << "Null handle " << handle << std::endl; @@ -22,21 +21,16 @@ GCObject* GarbageCollector::get(GCObjectHandle handle) { return it->second.get(); } -bool GarbageCollector::has(GCObjectHandle handle) const { +bool GC::has(GCObjectHandle handle) const { std::lock_guard g(lock_); return objects_.find(handle) != objects_.end(); } // Here is the naive tri-color marking // garbage collecting algorithm implementation. -void GarbageCollector::cleanup() { +void GC::collect() { std::lock_guard g(lock_); - if (state_ == GCState::Stopped) - return; - assert(state_ != GCState::Running); - state_ = GCState::Running; - std::vector grey; std::map> black; @@ -59,30 +53,44 @@ void GarbageCollector::cleanup() { // Sweep phase objects_ = std::move(black); - state_ = GCState::Idle; lastCleanup_.store(0); } -size_t GarbageCollector::size() const { +size_t GC::size() const { std::lock_guard g(lock_); return objects_.size(); } -void GarbageCollector::stop() { +size_t GC::count() { std::lock_guard g(lock_); - assert(state_ == GCState::Idle || state_ == GCState::Stopped); - state_ = GCState::Stopped; + return objects_.size(); } -void GarbageCollector::resume() { - std::lock_guard g(lock_); - assert(state_ == GCState::Idle || state_ == GCState::Stopped); - state_ = GCState::Idle; -} - -GarbageCollector& getGC() { - static GarbageCollector pool; +GC& GC::instance() { + static GC pool; return pool; } +sol::table GC::getLuaApi(sol::state_view& lua) { + sol::table api = lua.create_table_with(); + api["collect"] = [=] { + instance().collect(); + }; + api["pause"] = [] { instance().pause(); }; + api["resume"] = [] { instance().resume(); }; + api["enabled"] = [] { return instance().enabled(); }; + api["step"] = [](sol::optional newStep){ + auto previous = instance().step(); + if (newStep) { + REQUIRE(*newStep <= 0) << "gc.step have to be > 0"; + instance().step(static_cast(*newStep)); + } + return previous; + }; + api["count"] = [] { + return instance().count(); + }; + return api; +} + } // effil \ No newline at end of file diff --git a/src/cpp/garbage-collector.h b/src/cpp/garbage-collector.h index 5fcb04a..1596b56 100644 --- a/src/cpp/garbage-collector.h +++ b/src/cpp/garbage-collector.h @@ -2,6 +2,8 @@ #include "spin-mutex.h" +#include + #include #include #include @@ -32,18 +34,17 @@ protected: std::shared_ptr> refs_; }; -enum class GCState { Idle, Running, Stopped }; - -class GarbageCollector { +class GC { public: - GarbageCollector(); - ~GarbageCollector() = default; + GC(); + ~GC() = default; // This method is used to create all managed objects. template ObjectType create(Args&&... args) { - if (lastCleanup_.fetch_add(1) == step_) - cleanup(); + if (enabled_ && lastCleanup_.fetch_add(1) == step_) + collect(); + auto object = std::make_shared(std::forward(args)...); std::lock_guard g(lock_); @@ -51,28 +52,39 @@ public: return *object; } - GCObject* get(GCObjectHandle handle); + template + ObjectType get(GCObjectHandle handle) { + std::lock_guard g(lock_); + // TODO: add dynamic cast to check? + return *static_cast(findObject(handle)); + } bool has(GCObjectHandle handle) const; - void cleanup(); + + void collect(); size_t size() const; - void stop(); - void resume(); + void pause() { enabled_ = false; }; + void resume() { enabled_ = true; }; size_t step() const { return step_; } void step(size_t newStep) { step_ = newStep; } - GCState state() const { return state_; } + bool enabled() { return enabled_; }; + size_t count(); + + static GC& instance(); + static sol::table getLuaApi(sol::state_view& lua); private: mutable std::mutex lock_; - GCState state_; + bool enabled_; std::atomic lastCleanup_; size_t step_; std::map> objects_; private: - GarbageCollector(GarbageCollector&&) = delete; - GarbageCollector(const GarbageCollector&) = delete; + GCObject* findObject(GCObjectHandle handle); + +private: + GC(GC&&) = delete; + GC(const GC&) = delete; }; -GarbageCollector& getGC(); - } // effil \ No newline at end of file diff --git a/src/cpp/lua-module.cpp b/src/cpp/lua-module.cpp index e9aa3e0..3b951bd 100644 --- a/src/cpp/lua-module.cpp +++ b/src/cpp/lua-module.cpp @@ -18,9 +18,11 @@ sol::object createThread(const sol::this_state& lua, return sol::make_object(lua, std::make_shared(path, cpath, stepwise, step, function, args)); } -sol::object createTable(sol::this_state lua) { return sol::make_object(lua, getGC().create()); } +sol::object createTable(sol::this_state lua) { + return sol::make_object(lua, GC::instance().create()); +} -SharedTable globalTable = getGC().create(); +SharedTable globalTable = GC::instance().create(); } // namespace @@ -39,7 +41,9 @@ extern "C" int luaopen_libeffil(lua_State* L) { "size", SharedTable::luaSize, "setmetatable", SharedTable::luaSetMetatable, "getmetatable", SharedTable::luaGetMetatable, - "G", sol::make_object(lua, globalTable) + "G", sol::make_object(lua, globalTable), + "getmetatable", SharedTable::luaGetMetatable, + "gc", GC::getLuaApi(lua) ); sol::stack::push(lua, publicApi); return 1; diff --git a/src/cpp/shared-table.cpp b/src/cpp/shared-table.cpp index 30daa8e..d752c57 100644 --- a/src/cpp/shared-table.cpp +++ b/src/cpp/shared-table.cpp @@ -104,7 +104,7 @@ sol::object SharedTable::rawGet(const sol::stack_object& luaKey, sol::this_state { \ std::unique_lock lock(data_->lock); \ if (data_->metatable != GCNull) { \ - SharedTable tableHolder = *static_cast(getGC().get(data_->metatable)); \ + auto tableHolder = GC::instance().get(data_->metatable); \ lock.unlock(); \ sol::function handler = tableHolder.get(createStoredObject(methodName), state); \ if (handler.valid()) { \ @@ -160,7 +160,7 @@ void SharedTable::luaNewIndex(const sol::stack_object& luaKey, const sol::stack_ { std::unique_lock lock(data_->lock); if (data_->metatable != GCNull) { - SharedTable tableHolder = *static_cast(getGC().get(data_->metatable)); + auto tableHolder = GC::instance().get(data_->metatable); lock.unlock(); sol::function handler = tableHolder.get(createStoredObject("__newindex"), state); if (handler.valid()) { @@ -180,7 +180,8 @@ sol::object SharedTable::luaIndex(const sol::stack_object& luaKey, sol::this_sta StoredArray SharedTable::luaCall(sol::this_state state, const sol::variadic_args& args) { std::unique_lock lock(data_->lock); if (data_->metatable != GCNull) { - sol::function handler = static_cast(getGC().get(data_->metatable))->get(createStoredObject(std::string("__call")), state); + auto metatable = GC::instance().get(data_->metatable); + sol::function handler = metatable.get(createStoredObject(std::string("__call")), state); lock.unlock(); if (handler.valid()) { StoredArray storedResults; @@ -280,7 +281,7 @@ SharedTable SharedTable::luaSetMetatable(SharedTable& stable, const sol::stack_o sol::object SharedTable::luaGetMetatable(const SharedTable& stable, sol::this_state state) { std::lock_guard lock(stable.data_->lock); return stable.data_->metatable == GCNull ? sol::nil : - sol::make_object(state, *static_cast(getGC().get(stable.data_->metatable))); + sol::make_object(state, GC::instance().get(stable.data_->metatable)); } sol::object SharedTable::luaRawGet(const SharedTable& stable, const sol::stack_object& key, sol::this_state state) { diff --git a/src/cpp/stored-object.cpp b/src/cpp/stored-object.cpp index 11be2dd..9a3dfc6 100644 --- a/src/cpp/stored-object.cpp +++ b/src/cpp/stored-object.cpp @@ -79,7 +79,7 @@ public: TableHolder(const SolType& luaObject) { assert(luaObject.template is()); handle_ = luaObject.template as().handle(); - assert(getGC().has(handle_)); + assert(GC::instance().has(handle_)); } TableHolder(GCObjectHandle handle) @@ -90,7 +90,7 @@ public: } sol::object unpack(sol::this_state state) const final { - return sol::make_object(state, *static_cast(getGC().get(handle_))); + return sol::make_object(state, GC::instance().get(handle_)); } GCObjectHandle gcHandle() const override { return handle_; } @@ -115,7 +115,7 @@ StoredObject makeStoredObject(sol::object luaObject, SolTableToShared& visited) auto st = std::find_if(visited.begin(), visited.end(), comparator); if (st == std::end(visited)) { - SharedTable table = getGC().create(); + SharedTable table = GC::instance().create(); visited.emplace_back(std::make_pair(luaTable, table.handle())); dumpTable(&table, luaTable, visited); return createStoredObject(table.handle()); @@ -160,7 +160,7 @@ StoredObject fromSolObject(const SolObject& luaObject) { sol::table luaTable = luaObject; // Tables pool is used to store tables. // Right now not defiantly clear how ownership between states works. - SharedTable table = getGC().create(); + SharedTable table = GC::instance().create(); SolTableToShared visited{{luaTable, table.handle()}}; // Let's dump table and all subtables diff --git a/src/cpp/threading.cpp b/src/cpp/threading.cpp index 78ee97c..3cfa80d 100644 --- a/src/cpp/threading.cpp +++ b/src/cpp/threading.cpp @@ -54,7 +54,6 @@ class ThreadHandle { public: const bool managed; - sol::state lua; Status status; StoredArray result; @@ -68,8 +67,9 @@ public: ThreadHandle(bool isManaged) : managed(isManaged) , status(Status::Running) - , command_(Command::Run) { - luaL_openlibs(lua); + , command_(Command::Run) + , lua_(std::make_unique()) { + luaL_openlibs(*lua_); } Command command() const { return command_; } @@ -81,9 +81,18 @@ public: command_ = cmd; } + sol::state& lua() { + assert(lua_); + return *lua_; + } + + void destroyLua() { lua_.reset(); } + private: SpinMutex commandMutex_; Command command_; + + std::unique_ptr lua_; }; namespace { @@ -112,13 +121,8 @@ void luaHook(lua_State*, lua_Debug*) { class ScopeGuard { public: - ScopeGuard(const std::function& f) - : f_(f) { - } - - ~ScopeGuard() { - f_(); - } + ScopeGuard(const std::function& f) : f_(f) {} + ~ScopeGuard() { f_(); } private: std::function f_; @@ -130,6 +134,9 @@ void runThread(std::shared_ptr handle, ScopeGuard reportComplete([=](){ DEBUG << "Finished " << std::endl; + // Let's destroy accociated state + // to release all resources as soon as possible + handle->destroyLua(); handle->completion.notify(); }); @@ -137,10 +144,10 @@ void runThread(std::shared_ptr handle, thisThreadHandle = handle.get(); try { - sol::function userFuncObj = loadString(handle->lua, strFunction); + sol::function userFuncObj = loadString(handle->lua(), strFunction); sol::function_result results = userFuncObj(std::move(arguments)); (void)results; // just leave all returns on the stack - sol::variadic_args args(handle->lua, -lua_gettop(handle->lua)); + sol::variadic_args args(handle->lua(), -lua_gettop(handle->lua())); for (const auto& iter : args) { StoredObject store = createStoredObject(iter.get()); handle->result.emplace_back(std::move(store)); @@ -193,12 +200,12 @@ Thread::Thread(const std::string& path, const sol::variadic_args& variadicArgs) : handle_(std::make_shared(managed)) { - handle_->lua["package"]["path"] = path; - handle_->lua["package"]["cpath"] = cpath; - handle_->lua.script("require 'effil'"); + handle_->lua()["package"]["path"] = path; + handle_->lua()["package"]["cpath"] = cpath; + handle_->lua().script("require 'effil'"); if (managed) - lua_sethook(handle_->lua, luaHook, LUA_MASKCOUNT, step); + lua_sethook(handle_->lua(), luaHook, LUA_MASKCOUNT, step); std::string strFunction = dumpFunction(function); diff --git a/src/lua/effil.lua b/src/lua/effil.lua index 9326736..94d2934 100644 --- a/src/lua/effil.lua +++ b/src/lua/effil.lua @@ -23,6 +23,7 @@ local api = { setmetatable = capi.setmetatable, getmetatable = capi.getmetatable, G = capi.G, + gc = capi.gc } local function run_thread(config, f, ...) diff --git a/tests/cpp/garbage-collector.cpp b/tests/cpp/garbage-collector.cpp index 4c897f6..c3a3902 100644 --- a/tests/cpp/garbage-collector.cpp +++ b/tests/cpp/garbage-collector.cpp @@ -12,10 +12,10 @@ TEST(gc, GCObject) { GCObject o1; EXPECT_EQ(o1.instances(), (size_t)1); - GCObject o2 = getGC().create(); + GCObject o2 = GC::instance().create(); EXPECT_EQ(o2.instances(), (size_t)2); - GCObject o3 = getGC().create(); + GCObject o3 = GC::instance().create(); GCObject o4(o3); GCObject o5(o4); EXPECT_EQ(o5.instances(), o3.instances()); @@ -25,18 +25,15 @@ TEST(gc, GCObject) { } TEST(gc, collect) { - getGC().cleanup(); - ASSERT_EQ(getGC().size(), (size_t)1); - + GC::instance().collect(); + ASSERT_EQ(GC::instance().size(), (size_t)1); { - GCObject o1 = getGC().create(); - ; - GCObject o2 = getGC().create(); - ; + GCObject o1 = GC::instance().create(); + GCObject o2 = GC::instance().create(); } - EXPECT_EQ(getGC().size(), (size_t)3); - getGC().cleanup(); - EXPECT_EQ(getGC().size(), (size_t)1); + EXPECT_EQ(GC::instance().size(), (size_t)3); + GC::instance().collect(); + EXPECT_EQ(GC::instance().size(), (size_t)1); } namespace { @@ -44,26 +41,27 @@ namespace { struct Dummy : public GCObject { void add(GCObjectHandle ref) { refs_->insert(ref); } }; + } TEST(gc, withRefs) { - getGC().cleanup(); + GC::instance().collect(); { - Dummy root = getGC().create(); + Dummy root = GC::instance().create(); { - Dummy orphan = getGC().create(); + Dummy orphan = GC::instance().create(); for (size_t i = 0; i < 3; i++) { - Dummy child = getGC().create(); + Dummy child = GC::instance().create(); root.add(child.handle()); } } - EXPECT_EQ(getGC().size(), (size_t)6); - getGC().cleanup(); - EXPECT_EQ(getGC().size(), (size_t)5); + EXPECT_EQ(GC::instance().size(), (size_t)6); + GC::instance().collect(); + EXPECT_EQ(GC::instance().size(), (size_t)5); } - getGC().cleanup(); - EXPECT_EQ(getGC().size(), (size_t)1); + GC::instance().collect(); + EXPECT_EQ(GC::instance().size(), (size_t)1); } TEST(gc, autoCleanup) { @@ -73,59 +71,59 @@ TEST(gc, autoCleanup) { for (size_t i = 0; i < 5; i++) threads.emplace_back([=] { for (size_t i = 0; i < objectsPerThread; i++) - getGC().create(); + GC::instance().create(); }); for (auto& thread : threads) thread.join(); - EXPECT_LT(getGC().size(), getGC().step()); + EXPECT_LT(GC::instance().size(), GC::instance().step()); } TEST(gc, gcInLuaState) { sol::state lua; bootstrapState(lua); - lua["st"] = getGC().create(); + lua["st"] = GC::instance().create(); lua.script(R"( for i=1,1000 do st[i] = {"Wow"} end )"); - EXPECT_EQ(getGC().size(), (size_t)1002); + EXPECT_EQ(GC::instance().size(), (size_t)1002); lua.script(R"( for i=1,1000 do st[i] = nil end )"); - getGC().cleanup(); - EXPECT_EQ(getGC().size(), (size_t)2); + GC::instance().collect(); + EXPECT_EQ(GC::instance().size(), (size_t)2); } TEST(gc, cycles) { { sol::state lua; bootstrapState(lua); - getGC().cleanup(); + GC::instance().collect(); - lua["st"] = getGC().create(); + lua["st"] = GC::instance().create(); lua.script(R"( st.parent = {} st.parent.child = { ref = st.parent } st[4] = { one = 1 } st[5] = { flag = true } )"); - EXPECT_EQ(getGC().size(), (size_t)6); + EXPECT_EQ(GC::instance().size(), (size_t)6); lua.script("st.parent = nil"); lua.collect_garbage(); - getGC().cleanup(); - EXPECT_EQ(getGC().size(), (size_t)4); + GC::instance().collect(); + EXPECT_EQ(GC::instance().size(), (size_t)4); } - getGC().cleanup(); - EXPECT_EQ(getGC().size(), (size_t)1); + GC::instance().collect(); + EXPECT_EQ(GC::instance().size(), (size_t)1); } TEST(gc, multipleStates) { @@ -135,12 +133,12 @@ TEST(gc, multipleStates) { bootstrapState(lua2); { - SharedTable st = getGC().create(); + SharedTable st = GC::instance().create(); lua1["st"] = st; lua2["st"] = st; } - getGC().cleanup(); - EXPECT_EQ(getGC().size(), (size_t)2); + GC::instance().collect(); + EXPECT_EQ(GC::instance().size(), (size_t)2); lua1.script(R"( st.men = { name = "John", age = 22 } @@ -152,13 +150,13 @@ st.men.car = st.car st.men.cat = st.cat st.men.fish = st.fish )"); - getGC().cleanup(); - EXPECT_EQ(getGC().size(), (size_t)6); + GC::instance().collect(); + EXPECT_EQ(GC::instance().size(), (size_t)6); lua2.script("copy = { st.men } st = nil"); lua1.script("st = nil"); lua1.collect_garbage(); lua2.collect_garbage(); - getGC().cleanup(); - EXPECT_EQ(getGC().size(), (size_t)5); + GC::instance().collect(); + EXPECT_EQ(GC::instance().size(), (size_t)5); } diff --git a/tests/cpp/shared-table.cpp b/tests/cpp/shared-table.cpp index a07fddf..82364a0 100644 --- a/tests/cpp/shared-table.cpp +++ b/tests/cpp/shared-table.cpp @@ -129,9 +129,9 @@ TEST(sharedTable, playingWithSharedTables) { sol::state lua; bootstrapState(lua); - lua["recursive"] = getGC().create(); - lua["st1"] = getGC().create(); - lua["st2"] = getGC().create(); + lua["recursive"] = GC::instance().create(); + lua["st1"] = GC::instance().create(); + lua["st2"] = GC::instance().create(); lua.script(R"( st1.proxy = st2 diff --git a/tests/lua/gc.lua b/tests/lua/gc.lua new file mode 100644 index 0000000..c149b03 --- /dev/null +++ b/tests/lua/gc.lua @@ -0,0 +1,40 @@ +local effil = require "effil" +local gc = effil.gc + +TestGC = { tearDown = tearDown } + +function TestGC:testCleanup() + collectgarbage() + gc.collect() + test.assertEquals(gc.count(), 1) + + for i = 0, 10000 do + local tmp = effil.table() + end + + collectgarbage() + gc.collect() + test.assertEquals(gc.count(), 1) +end + +function TestGC:testDisableGC() + local nobjects = 10000 + + collectgarbage() + gc.collect() + test.assertEquals(gc.count(), 1) + + gc.pause() + test.assertFalse(gc.enabled()) + + for i = 1, nobjects do + local tmp = effil.table() + end + + test.assertEquals(gc.count(), nobjects + 1) + collectgarbage() + gc.collect() + test.assertEquals(gc.count(), 1) + + gc.resume() +end \ No newline at end of file diff --git a/tests/lua/run_tests.lua b/tests/lua/run_tests.lua index ce915a7..b2292d7 100755 --- a/tests/lua/run_tests.lua +++ b/tests/lua/run_tests.lua @@ -31,6 +31,7 @@ effil = require 'effil' require 'test_utils' require 'thread' require 'shared_table' +require 'gc' -- Hack tests functions to print when test starts for suite_name, suite in pairs(_G) do diff --git a/tests/lua/test_utils.lua b/tests/lua/test_utils.lua index 0a8efe4..24b19f3 100644 --- a/tests/lua/test_utils.lua +++ b/tests/lua/test_utils.lua @@ -46,6 +46,10 @@ end function tearDown() collectgarbage() + effil.gc.collect() + -- effil.G is always present + -- thus, gc has one object + test.assertEquals(effil.gc.count(), 1) end function make_test_with_param(test_suite, test_case_pattern, ...)