Enable manual control of GC inside keeper states

8 files changed, 119 insertions, 73 deletions

diff --git a/Makefile b/Makefile
index 868c481..0750401 100644
--- a/Makefile
+++ b/Makefile
@@ -72,25 +72,25 @@ rock:
 #--- Testing ---
 #
 test:
-        $(MAKE) errhangtest
-        $(MAKE) irayo_recursive
-        $(MAKE) irayo_closure
+        $(MAKE) atexit
+        $(MAKE) atomic
         $(MAKE) basic
         $(MAKE) cancel
-        $(MAKE) fifo
-        $(MAKE) keeper
-        $(MAKE) timer
-        $(MAKE) atomic
         $(MAKE) cyclic
-        $(MAKE) objects
+        $(MAKE) errhangtest
         $(MAKE) fibonacci
-        $(MAKE) recursive
+        $(MAKE) fifo
         $(MAKE) func_is_string
-        $(MAKE) atexit
+        $(MAKE) irayo_recursive
+        $(MAKE) irayo_closure
+        $(MAKE) keeper
         $(MAKE) linda_perf
-        $(MAKE) rupval
+        $(MAKE) objects
         $(MAKE) package
         $(MAKE) pingpong
+        $(MAKE) recursive
+        $(MAKE) rupval
+        $(MAKE) timer
 
 basic: tests/basic.lua $(_TARGET_SO)
         $(_PREFIX) $(LUA) $<
diff --git a/docs/index.html b/docs/index.html
index 24fa4ef..ee5acfa 100644
--- a/docs/index.html
+++ b/docs/index.html
@@ -291,6 +291,18 @@
                 </tr>
 
                 <tr valign=top>
+                        <td id="keepers_gc_threshold">
+                                <code>.keepers_gc_threshold</code>
+                        </td>
+                        <td>integer</td>
+                        <td>
+                                If &lt;0, GC runs automatically. This is the default.<br/>
+                                If 0, GC runs after *every* keeper operation.<br/>
+                                If &gt;0, Keepers run GC manually with <tt>lua_gc(LUA_GCCOLLECT)</tt> whenever memory usage reported by <tt>lua_gc(LUA_GCCOUNT)</tt> reaches this threshold. Check is made after every keeper operation (see <a href="#lindas">below</a>). If memory usage remains above threshold after the GC cycle, an error is raised.
+                        </td>
+                </tr>
+
+                <tr valign=top>
                         <td id="with_timers">
                                 <code>.with_timers</code>
                         </td>
@@ -1784,4 +1796,4 @@ int luaD_new_clonable( lua_State* L)
 </p>
 
 </body>
-</html></pre>
\ No newline at end of file
+</html>
diff --git a/src/keeper.cpp b/src/keeper.cpp
index 244cb6a..937d190 100644
--- a/src/keeper.cpp
+++ b/src/keeper.cpp
@@ -652,12 +652,18 @@ void init_keepers(Universe* U, lua_State* L)
 {
     STACK_CHECK_START_REL(L, 0);                                   // L                            K
     lua_getfield(L, 1, "nb_keepers");                              // nb_keepers
-    int nb_keepers{ static_cast<int>(lua_tointeger(L, -1)) };
+    int const nb_keepers{ static_cast<int>(lua_tointeger(L, -1)) };
     lua_pop(L, 1);                                                 //
     if (nb_keepers < 1)
     {
         std::ignore = luaL_error(L, "Bad number of keepers (%d)", nb_keepers);
     }
+    STACK_CHECK(L, 0);
+
+    lua_getfield(L, 1, "keepers_gc_threshold");                    // keepers_gc_threshold
+    int const keepers_gc_threshold{ static_cast<int>(lua_tointeger(L, -1)) };
+    lua_pop(L, 1);                                                 //
+    STACK_CHECK(L, 0);
 
     // Keepers contains an array of 1 Keeper, adjust for the actual number of keeper states
     {
@@ -668,6 +674,7 @@ void init_keepers(Universe* U, lua_State* L)
             std::ignore = luaL_error(L, "init_keepers() failed while creating keeper array; out of memory");
         }
         memset(U->keepers, 0, bytes);
+        U->keepers->gc_threshold = keepers_gc_threshold;
         U->keepers->nb_keepers = nb_keepers;
     }
     for (int i = 0; i < nb_keepers; ++i)                           // keepersUD
@@ -685,6 +692,11 @@ void init_keepers(Universe* U, lua_State* L)
         // therefore, we need a recursive mutex.
         MUTEX_RECURSIVE_INIT(&U->keepers->keeper_array[i].keeper_cs);
 
+        if (U->keepers->gc_threshold >= 0)
+        {
+            lua_gc(K, LUA_GCSTOP, 0);
+        }
+
         STACK_CHECK_START_ABS(K, 0);
 
         // copy the universe pointer in the keeper itself
@@ -735,8 +747,12 @@ void init_keepers(Universe* U, lua_State* L)
 Keeper* which_keeper(Keepers* keepers_, uintptr_t magic_)
 {
     int const nbKeepers{ keepers_->nb_keepers };
-    unsigned int i = (unsigned int)((magic_ >> KEEPER_MAGIC_SHIFT) % nbKeepers);
-    return &keepers_->keeper_array[i];
+    if (nbKeepers)
+    {
+        unsigned int i = (unsigned int) ((magic_ >> KEEPER_MAGIC_SHIFT) % nbKeepers);
+        return &keepers_->keeper_array[i];
+    }
+    return nullptr;
 }
 
 // ##################################################################################################
@@ -745,11 +761,7 @@ Keeper* keeper_acquire(Keepers* keepers_, uintptr_t magic_)
 {
     int const nbKeepers{ keepers_->nb_keepers };
     // can be 0 if this happens during main state shutdown (lanes is being GC'ed -> no keepers)
-    if( nbKeepers == 0)
-    {
-        return nullptr;
-    }
-    else
+    if (nbKeepers)
     {
         /*
         * Any hashing will do that maps pointers to 0..GNbKeepers-1 
@@ -765,6 +777,7 @@ Keeper* keeper_acquire(Keepers* keepers_, uintptr_t magic_)
         //++ K->count;
         return K;
     }
+    return nullptr;
 }
 
 // ##################################################################################################
@@ -843,5 +856,30 @@ int keeper_call(Universe* U, lua_State* K, keeper_api_t func_, lua_State* L, voi
     }
     // whatever happens, restore the stack to where it was at the origin
     lua_settop(K, Ktos);
+
+    // don't do this for this particular function, as it is only called during Linda destruction, and we don't want to raise an error, ever
+    if (func_ != KEEPER_API(clear)) [[unlikely]]
+    {
+        // since keeper state GC is stopped, let's run a step once in a while if required
+        int const gc_threshold{ U->keepers->gc_threshold };
+        if (gc_threshold == 0) [[unlikely]]
+        {
+            lua_gc(K, LUA_GCSTEP, 0);
+        }
+        else if (gc_threshold > 0) [[likely]]
+        {
+            int const gc_usage{ lua_gc(K, LUA_GCCOUNT, 0) };
+            if (gc_usage >= gc_threshold)
+            {
+                lua_gc(K, LUA_GCCOLLECT, 0);
+                int const gc_usage_after{ lua_gc(K, LUA_GCCOUNT, 0) };
+                if (gc_usage_after > gc_threshold) [[unlikely]]
+                {
+                    luaL_error(L, "Keeper GC threshold is too low, need at least %d", gc_usage_after);
+                }
+            }
+        }
+    }
+
     return retvals;
 }
diff --git a/src/keeper.h b/src/keeper.h
index e081bea..f7e3951 100644
--- a/src/keeper.h
+++ b/src/keeper.h
@@ -24,6 +24,7 @@ struct Keeper
 
 struct Keepers
 {
+    int gc_threshold{ 0 };
     int nb_keepers;
     Keeper keeper_array[1];
 };
diff --git a/src/lanes.lua b/src/lanes.lua
index b4c0070..6af286a 100644
--- a/src/lanes.lua
+++ b/src/lanes.lua
@@ -70,6 +70,7 @@ lanes.configure = function( settings_)
     local default_params =
     {
         nb_keepers = 1,
+        keepers_gc_threshold = -1,
         on_state_create = nil,
         shutdown_timeout = 0.25,
         with_timers = true,
@@ -91,6 +92,10 @@ lanes.configure = function( settings_)
             -- nb_keepers should be a number > 0
             return type( val_) == "number" and val_ > 0
         end,
+        keepers_gc_threshold = function( val_)
+            -- keepers_gc_threshold should be a number
+            return type( val_) == "number"
+        end,
         with_timers = boolean_param_checker,
         allocator = function( val_)
             -- can be nil, "protected", or a function
diff --git a/src/linda.cpp b/src/linda.cpp
index 37a74b0..5ee4768 100644
--- a/src/linda.cpp
+++ b/src/linda.cpp
@@ -885,15 +885,22 @@ static void* linda_id( lua_State* L, DeepOp op_)
         {
             Linda* const linda{ lua_tolightuserdata<Linda>(L, 1) };
             ASSERT_L(linda);
-
-            // Clean associated structures in the keeper state.
-            Keeper* const K{ keeper_acquire(linda->U->keepers, linda->hashSeed()) };
-            if (K && K->L) // can be nullptr if this happens during main state shutdown (lanes is GC'ed -> no keepers -> no need to cleanup)
+            Keeper* const myK{ which_keeper(linda->U->keepers, linda->hashSeed()) };
+            // if collected after the universe, keepers are already destroyed, and there is nothing to clear
+            if (myK)
             {
+                // if collected from my own keeper, we can't acquire/release it
+                // 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 };
                 // hopefully this won't ever raise an error as we would jump to the closest pcall site while forgetting to release the keeper mutex...
                 keeper_call(linda->U, K->L, KEEPER_API(clear), L, linda, 0);
+                if (need_acquire_release)
+                {
+                    keeper_release(K);
+                }
             }
-            keeper_release(K);
 
             delete linda; // operator delete overload ensures things go as expected
             return nullptr;
diff --git a/tests/keeper.lua b/tests/keeper.lua
index 9b38f02..6dbbd15 100644
--- a/tests/keeper.lua
+++ b/tests/keeper.lua
@@ -4,7 +4,7 @@
 -- Test program for Lua Lanes
 --
 
-local lanes = require "lanes".configure{ with_timers = false, nb_keepers = 200}
+local lanes = require "lanes".configure{ with_timers = false, nb_keepers = 1, keepers_gc_threshold = 500}
 
 do
     print "Linda names test:"
@@ -12,7 +12,20 @@ do
     local unnamedLinda2 = lanes.linda("")
     local veeeerrrryyyylooongNamedLinda= lanes.linda( "veeeerrrryyyylooongNamedLinda", 1)
     print(unnamedLinda, unnamedLinda2, veeeerrrryyyylooongNamedLinda)
+    print "GC deadlock test start"
+    -- store a linda in another linda (-> in a keeper)
+    unnamedLinda:set("here", lanes.linda("temporary linda"))
+    -- repeatedly add and remove stuff in the linda so that a GC happens during the keeper operation
+    for i = 1, 1000 do
+        for j = 1, 1000 do -- send 1000 tables
+            unnamedLinda:send("here", {"a", "table", "with", "some", "stuff"})
+        end
+        unnamedLinda:set("here") -- clear everything
+    end
 end
+print "collecting garbage"
+collectgarbage()
+print "GC deadlock test done"
 
 local print_id = 0
 local PRINT = function(...)
diff --git a/tests/linda_perf.lua b/tests/linda_perf.lua
index 9177852..c736428 100644
--- a/tests/linda_perf.lua
+++ b/tests/linda_perf.lua
@@ -1,5 +1,5 @@
 local lanes = require "lanes"
-lanes.configure{ with_timers = false }
+lanes.configure{ with_timers = false, keepers_gc_threshold=20000 }
 
 -- set TEST1, PREFILL1, FILL1, TEST2, PREFILL2, FILL2 from the command line
 
@@ -17,6 +17,8 @@ local finalizer = function(err, stk)
         end
 end
 
+--##################################################################################################
+
 -- this lane eats items in the linda one by one
 local eater = function( l, loop)
         set_finalizer(finalizer)
@@ -32,6 +34,8 @@ local eater = function( l, loop)
         print("eater: done ("..val..")")
 end
 
+--##################################################################################################
+
 -- this lane eats items in the linda in batches
 local gobbler = function( l, loop, batch)
         set_finalizer(finalizer)
@@ -47,9 +51,13 @@ local gobbler = function( l, loop, batch)
         print("gobbler: done ("..val..")")
 end
 
+--##################################################################################################
+
 local lane_eater_gen = lanes.gen( "*", {priority = 3}, eater)
 local lane_gobbler_gen = lanes.gen( "*", {priority = 3}, gobbler)
 
+--##################################################################################################
+
 -- main thread writes data while a lane reads it
 local function ziva( preloop, loop, batch)
         -- prefill the linda a bit to increase fifo stress
@@ -94,6 +102,8 @@ local function ziva( preloop, loop, batch)
         return lanes.now_secs() - t1
 end
 
+--##################################################################################################
+
 TEST1 = TEST1 or 1000
 PREFILL1 = PREFILL1 or 10000
 FILL1 = FILL1 or 2000000
@@ -109,6 +119,7 @@ local tests1 =
         { PREFILL1, FILL1, 13},
         { PREFILL1, FILL1, 21},
         { PREFILL1, FILL1, 44},
+        { PREFILL1, FILL1, 65},
 }
 print "############################################ tests #1"
 for i, v in ipairs( tests1) do
@@ -119,38 +130,7 @@ for i, v in ipairs( tests1) do
         print("DURATION = " .. ziva( pre, loop, batch) .. "\n")
 end
 
---[[
-        V 2.1.0:
-        ziva( 20000, 0) -> 4s           ziva( 10000, 20000) -> 3s
-        ziva( 30000, 0) -> 8s     ziva( 20000, 30000) -> 7s
-        ziva( 40000, 0) -> 15s    ziva( 30000, 40000) -> 15s
-        ziva( 50000, 0) -> 24s    ziva( 40000, 50000) -> 23s
-        ziva( 60000, 0) -> 34s    ziva( 50000, 60000) -> 33s
-
-        SIMPLIFIED:
-        ziva( 20000, 0) -> 4s           ziva( 10000, 20000) -> 3s
-        ziva( 30000, 0) -> 9s     ziva( 20000, 30000) -> 8s
-        ziva( 40000, 0) -> 15s    ziva( 30000, 40000) -> 15s
-        ziva( 50000, 0) -> 25s    ziva( 40000, 50000) -> 24s
-        ziva( 60000, 0) -> 35s    ziva( 50000, 60000) -> 35s
-
-        FIFO:
-        ziva( 2000000, 0) -> 9s   ziva( 1000000, 2000000) -> 33s
-        ziva( 3000000, 0) -> 14s  ziva( 2000000, 3000000) -> 40s
-        ziva( 4000000, 0) -> 20s  ziva( 3000000, 4000000) -> 27s
-        ziva( 5000000, 0) -> 24s  ziva( 4000000, 5000000) -> 42s
-        ziva( 6000000, 0) -> 29s  ziva( 5000000, 6000000) -> 55s
-
-        FIFO BATCHED:
-        ziva( 4000000, 0, 1)  -> 20s
-        ziva( 4000000, 0, 2)  -> 11s
-        ziva( 4000000, 0, 3)  -> 7s
-        ziva( 4000000, 0, 5)  -> 5s
-        ziva( 4000000, 0, 8)  -> 3s
-        ziva( 4000000, 0, 13) -> 3s
-        ziva( 4000000, 0, 21) -> 3s
-        ziva( 4000000, 0, 44) -> 2s
-]]
+--##################################################################################################
 
 -- sequential write/read (no parallelization involved)
 local function ziva2( preloop, loop, batch)
@@ -183,7 +163,7 @@ local function ziva2( preloop, loop, batch)
         for i = 1, preloop, step do
                 batch_send()
         end
-        print( "stored " .. (l:count( "key") or 0) .. " items in the linda before starting consumer lane")
+        print( "stored " .. (l:count( "key") or 0) .. " items in the linda before starting the alternating reads and writes")
         -- loop that alternatively sends and reads data off the linda
         if loop > preloop then
                 for i = preloop + 1, loop, step do
@@ -198,25 +178,14 @@ local function ziva2( preloop, loop, batch)
         return lanes.now_secs() - t1
 end
 
+--##################################################################################################
+
 TEST2 = TEST2 or 1000
 PREFILL2 = PREFILL2 or 0
 FILL2 = FILL2 or 4000000
 
 local tests2 =
 {
-        -- prefill, then consume everything
-        --[[
-        { 4000000, 0},
-        { 4000000, 0, 1},
-        { 4000000, 0, 2},
-        { 4000000, 0, 3},
-        { 4000000, 0, 5},
-        { 4000000, 0, 8},
-        { 4000000, 0, 13},
-        { 4000000, 0, 21},
-        { 4000000, 0, 44},
-        --]]
-        -- alternatively fill and consume
         { PREFILL2, FILL2},
         { PREFILL2, FILL2, 1},
         { PREFILL2, FILL2, 2},
@@ -226,6 +195,7 @@ local tests2 =
         { PREFILL2, FILL2, 13},
         { PREFILL2, FILL2, 21},
         { PREFILL2, FILL2, 44},
+        { PREFILL2, FILL2, 65},
 }
 
 print "############################################ tests #2"