Trigger Lua GC after script loading (#13407)

Nowdays we do not trigger LUA GC after loading lua script. This means
that when a large number of scripts are loaded, such as when functions
are propagating from the master to the replica, if the LUA scripts are
never touched on the replica, the garbage might remain there
indefinitely.

Before this PR, we would share a gc_count between scripts and functions.
This means that, under certain circumstances, the GC trigger for scripts
and functions was not fair.
For example, loading a large number of scripts followed by a small
number of functions could result in the functions triggering GC.
In this PR, we assign a unique `gc_count` to each of them, so the GC
triggers between them will no longer affect each other.

on the other hand, this PR will to bring regession for script loading
commands(`FUNCTION LOAD` and `SCRIPT LOAD`), but they are not hot path,
we can ignore it, and it will be replaced
https://github.com/redis/redis/pull/13375 in the future.

---------

Co-authored-by: Oran Agra <oran@redislabs.com>

src/eval.c

@@ -23,6 +23,8 @@
 #include <ctype.h>
 #include <math.h>
 
+static int gc_count = 0; /* Counter for the number of GC requests, reset after each GC execution */
+
 void ldbInit(void);
 void ldbDisable(client *c);
 void ldbEnable(client *c);
@@ -454,6 +456,7 @@ sds luaCreateFunction(client *c, robj *body, int evalsha) {
                 lua_tostring(lctx.lua,-1));
         }
         lua_pop(lctx.lua,1);
+        luaGC(lctx.lua, &gc_count);
         return NULL;
     }
 
@@ -473,6 +476,11 @@ sds luaCreateFunction(client *c, robj *body, int evalsha) {
     serverAssertWithInfo(c ? c : lctx.lua_client,NULL,retval == DICT_OK);
     lctx.lua_scripts_mem += sdsZmallocSize(sha) + getStringObjectSdsUsedMemory(body);
     incrRefCount(body);
+
+    /* Perform GC after creating the script and adding it to the LRU list,
+     * as script may be evicted during addition. */
+    luaGC(lctx.lua, &gc_count);
+
     return sha;
 }
 
@@ -600,6 +608,7 @@ void evalGenericCommand(client *c, int evalsha) {
     luaCallFunction(&rctx, lua, c->argv+3, numkeys, c->argv+3+numkeys, c->argc-3-numkeys, ldb.active);
     lua_pop(lua,1); /* Remove the error handler. */
     scriptResetRun(&rctx);
+    luaGC(lua, &gc_count);
 
     if (l->node) {
         /* Quick removal and re-insertion after the script is called to

src/function_lua.c

@@ -31,6 +31,8 @@
 #define LIBRARY_API_NAME "__LIBRARY_API__"
 #define GLOBALS_API_NAME "__GLOBALS_API__"
 
+static int gc_count = 0; /* Counter for the number of GC requests, reset after each GC execution */
+
 /* Lua engine ctx */
 typedef struct luaEngineCtx {
     lua_State *lua;
@@ -131,6 +133,7 @@ done:
 
     lua_sethook(lua,NULL,0,0); /* Disable hook */
     luaSaveOnRegistry(lua, REGISTRY_LOAD_CTX_NAME, NULL);
+    luaGC(lua, &gc_count);
     return ret;
 }
 
@@ -159,6 +162,7 @@ static void luaEngineCall(scriptRunCtx *run_ctx,
 
     luaCallFunction(run_ctx, lua, keys, nkeys, args, nargs, 0);
     lua_pop(lua, 1); /* Pop error handler */
+    luaGC(lua, &gc_count);
 }
 
 static size_t luaEngineGetUsedMemoy(void *engine_ctx) {

src/script_lua.c

@@ -1650,23 +1650,6 @@ void luaCallFunction(scriptRunCtx* run_ctx, lua_State *lua, robj** keys, size_t
         err = lua_pcall(lua,2,1,-4);
     }
 
-    /* Call the Lua garbage collector from time to time to avoid a
-     * full cycle performed by Lua, which adds too latency.
-     *
-     * The call is performed every LUA_GC_CYCLE_PERIOD executed commands
-     * (and for LUA_GC_CYCLE_PERIOD collection steps) because calling it
-     * for every command uses too much CPU. */
-    #define LUA_GC_CYCLE_PERIOD 50
-    {
-        static long gc_count = 0;
-
-        gc_count++;
-        if (gc_count == LUA_GC_CYCLE_PERIOD) {
-            lua_gc(lua,LUA_GCSTEP,LUA_GC_CYCLE_PERIOD);
-            gc_count = 0;
-        }
-    }
-
     if (err) {
         /* Error object is a table of the following format:
          * {err='<error msg>', source='<source file>', line=<line>}
@@ -1709,3 +1692,21 @@ void luaCallFunction(scriptRunCtx* run_ctx, lua_State *lua, robj** keys, size_t
 unsigned long luaMemory(lua_State *lua) {
     return lua_gc(lua, LUA_GCCOUNT, 0) * 1024LL;
 }
+
+/* Call the Lua garbage collector from time to time to avoid a
+ * full cycle performed by Lua, which adds too latency.
+ *
+ * The call is performed every LUA_GC_CYCLE_PERIOD executed commands
+ * (and for LUA_GC_CYCLE_PERIOD collection steps) because calling it
+ * for every command uses too much CPU.
+ * 
+ * Each script VM / State (Eval and Functions) maintains its own unique `gc_count`
+ * to control GC independently. */
+#define LUA_GC_CYCLE_PERIOD 50
+void luaGC(lua_State *lua, int *gc_count) {
+    (*gc_count)++;
+    if (*gc_count >= LUA_GC_CYCLE_PERIOD) {
+        lua_gc(lua, LUA_GCSTEP, LUA_GC_CYCLE_PERIOD);
+        *gc_count = 0;
+    }
+}

src/script_lua.h

@@ -61,6 +61,6 @@ void luaCallFunction(scriptRunCtx* r_ctx, lua_State *lua, robj** keys, size_t nk
 void luaExtractErrorInformation(lua_State *lua, errorInfo *err_info);
 void luaErrorInformationDiscard(errorInfo *err_info);
 unsigned long luaMemory(lua_State *lua);
-
+void luaGC(lua_State *lua, int *gc_count);
 
 #endif /* __SCRIPT_LUA_H_ */

tests/unit/functions.tcl

@@ -296,10 +296,10 @@ start_server {tags {"scripting"}} {
 
     test {FUNCTION - async function flush rebuilds Lua VM without causing race condition between main and lazyfree thread} {
         # LAZYFREE_THRESHOLD is 64
-        for {set i 0} {$i < 100} {incr i} {
-            r function load REPLACE [get_function_code lua test$i test$i {local a = 1 while true do a = a + 1 end}]
+        for {set i 0} {$i < 1000} {incr i} {
+            r function load [get_function_code lua test$i test$i {local a = 1 while true do a = a + 1 end}]
         }
-        assert_morethan [s used_memory_vm_functions] 70000
+        assert_morethan [s used_memory_vm_functions] 100000
         r config resetstat
         r function flush async
         assert_lessthan [s used_memory_vm_functions] 40000
@@ -309,11 +309,11 @@ start_server {tags {"scripting"}} {
         while {1} {
             # Tests for race conditions between async function flushes and main thread Lua VM operations.
             r function load REPLACE [get_function_code lua test test {local a = 1 while true do a = a + 1 end}]
-            if {[s lazyfreed_objects] == 101 || [expr {[clock seconds] - $start_time}] > 5} {
+            if {[s lazyfreed_objects] == 1001 || [expr {[clock seconds] - $start_time}] > 5} {
                 break
             }
         }
-        if {[s lazyfreed_objects] != 101} {
+        if {[s lazyfreed_objects] != 1001} {
             error "Timeout or unexpected number of lazyfreed_objects: [s lazyfreed_objects]"
         }
         assert_match {{library_name test engine LUA functions {{name test description {} flags {}}}}} [r function list]

tests/unit/scripting.tcl

@@ -1876,6 +1876,27 @@ start_server {tags {"scripting needs:debug"}} {
 
     r debug set-disable-deny-scripts 0
 }
+
+start_server {tags {"scripting"}} {
+    test "Verify Lua performs GC correctly after script loading" {
+        set dummy_script "--[string repeat x 10]\nreturn "
+        set n 50000
+        for {set i 0} {$i < $n} {incr i} {
+            set script "$dummy_script[format "%06d" $i]"
+            if {$is_eval} {
+                r script load $script
+            } else {
+                r function load "#!lua name=test$i\nredis.register_function('test$i', function(KEYS, ARGV)\n $script \nend)"
+            }
+        }
+
+        if {$is_eval} {
+            assert_lessthan [s used_memory_lua] 17500000
+        } else {
+            assert_lessthan [s used_memory_vm_functions] 14500000
+        }
+    }
+}
 } ;# foreach is_eval