Fully remove memcatbyteslimit logic

Author: HaroldCindy

VM/include/llsl.h

@@ -68,8 +68,6 @@ LUA_API int luaSL_pushnativeinteger(lua_State *L, int val);
 LUA_API uint8_t luaSL_lsl_type(lua_State *L, int idx);
 /// Should only be called in an interrupt handler!
 LUA_API YieldableStatus luaSL_may_interrupt(lua_State *L);
-// Keep going with GC until we hit the sweep phase, we need some memory.
-LUA_API void luaSL_emergencyfinishgc(lua_State *L);
 
 typedef struct lua_TValue TValue;
 uint8_t lua_lsl_type(const TValue *tv);

VM/include/lua.h

@@ -342,10 +342,6 @@ LUA_API void lua_setmemcat(lua_State* L, int category);
 LUA_API int lua_getmemcat(lua_State* L);
 LUA_API size_t lua_totalbytes(lua_State* L, int category);
 
-// ServerLua: used for setting a memcat byte size limit for memcat IDs above 1.
-// setting a limit of "0" disables the memory limit.
-LUA_API void lua_setmemcatbyteslimit(lua_State* L, lua_Unsigned bytes_limit);
-
 /*
 ** miscellaneous functions
 */

VM/src/ares.cpp

@@ -2823,8 +2823,6 @@ static void scavenge_sl_vm_internals(lua_State *L, bool forUnpersist) {
     eris_ifassert(const int top = lua_gettop(L));
     int perms_idx = lua_gettop(L);
 
-    auto *runtime_state = (lua_SLRuntimeState*)lua_getthreaddata(L);
-
     const std::vector<std::pair<const char *, int>> lsl_vm_refs {
         // none for now
     };
@@ -3004,6 +3002,10 @@ void eris_register_perms(lua_State *L, bool for_unpersist) {
 
 static void
 unchecked_persist(lua_State *L, std::ostream *writer) {
+  // pause GC for the duration of serialization - some objects we're creating aren't rooted
+  // Also prevents beforeallocate callbacks from being invoked during setup
+  ScopedDisableGC _disable_gc(L);
+
   eris_ifassert(int old_top=lua_gettop(L));
   Info info;                                            /* perms buff rootobj */
   info.L = L;
@@ -3064,11 +3066,7 @@ unchecked_persist(lua_State *L, std::ostream *writer) {
 #endif
 
   p_header(&info);
-  {
-      ScopedDisableGC _disable_gc(L);
-
-      persist(&info);                     /* perms reftbl buff path? rootobj */
-  }
+  persist(&info);                          /* perms reftbl buff path? rootobj */
 
 #if HARDSTACKTESTS
   lua_pop(L, LUA_MINSTACK - pre_pad_top);
@@ -3084,6 +3082,10 @@ unchecked_persist(lua_State *L, std::ostream *writer) {
 
 static void
 unchecked_unpersist(lua_State *L, std::istream *reader) {/* perms str? */
+  // pause GC for the duration of deserialization - some objects we're creating aren't rooted
+  // Also prevents beforeallocate callbacks from being invoked during setup
+  ScopedDisableGC _disable_gc(L);
+
   eris_ifassert(int old_top = lua_gettop(L));
   Info info;
   info.L = L;
@@ -3128,32 +3130,27 @@ unchecked_unpersist(lua_State *L, std::istream *reader) {/* perms str? */
   eris_populate_perms(L, true);
   lua_pop(L, 1);                              /* perms reftbl nil? path? str? */
 
-  // pause GC for the duration of deserialization - some objects we're creating aren't rooted
-  {
-    ScopedDisableGC _disable_gc(L);
-
 #if HARDSTACKTESTS
-    // Arrange the stack to make it more likely that we hit any lua_checkstack() misuse
-    int pre_pad_top = lua_gettop(L);
-    lua_checkstack(L, LUA_MINSTACK);
-    while (lua_gettop(L) != LUA_MINSTACK - 1) {
-        lua_pushnil(L);
-    }
-    // A reference to the root obj needs to end back up on top
-    lua_pushvalue(L, pre_pad_top);
-    eris_assert(lua_gettop(L) == LUA_MINSTACK);
+  // Arrange the stack to make it more likely that we hit any lua_checkstack() misuse
+  int pre_pad_top = lua_gettop(L);
+  lua_checkstack(L, LUA_MINSTACK);
+  while (lua_gettop(L) != LUA_MINSTACK - 1) {
+      lua_pushnil(L);
+  }
+  // A reference to the root obj needs to end back up on top
+  lua_pushvalue(L, pre_pad_top);
+  eris_assert(lua_gettop(L) == LUA_MINSTACK);
 #endif
 
-    u_header(&info);
-    unpersist(&info);                 /* perms reftbl nil? path? str? rootobj */
+  u_header(&info);
+  unpersist(&info);                   /* perms reftbl nil? path? str? rootobj */
 
 #if HARDSTACKTESTS
-    // Slot the top of the stack back in where it should be
-    lua_replace(L, pre_pad_top + 1);
-    lua_pop(L, LUA_MINSTACK - pre_pad_top - 1);
-    eris_assert(lua_gettop(L) == pre_pad_top + 1);
+  // Slot the top of the stack back in where it should be
+  lua_replace(L, pre_pad_top + 1);
+  lua_pop(L, LUA_MINSTACK - pre_pad_top - 1);
+  eris_assert(lua_gettop(L) == pre_pad_top + 1);
 #endif
-  }
 
   if (info.generatePath) {              /* perms reftbl nil path str? rootobj */
     lua_remove(L, PATHIDX);                  /* perms reftbl nil str? rootobj */
@@ -3514,18 +3511,10 @@ eris_fork_thread(lua_State *Lforker, uint8_t default_state, uint8_t memcat) {
   // Make sure any objects we create during deserialization are created with the desired memcat
   lua_setmemcat(Lforker, memcat);
 
-  // We need to relax the bytes limit so that we don't fail due to garbage generated during deserialization.
-  int old_bytes_limit = Lforker->global->memcatbyteslimit;
-  lua_setmemcatbyteslimit(Lforker, 0);
   int status = eris_unpersist(Lforker, -1, -2); /* Lforker: state serialized uperms new_th */
 
-  // Get rid of any garbage created during deserialization if we're using memcat-based memory limits
-  if (old_bytes_limit)
-    lua_gc(Lforker, LUA_GCCOLLECT, 0);
-
   // Done, set the memcat back to the main one.
   lua_setmemcat(Lforker, 0);
-  lua_setmemcatbyteslimit(Lforker, old_bytes_limit);
 
   if (status == LUA_OK) {
     eris_assert(lua_isthread(Lforker, -1));

VM/src/lapi.cpp

@@ -1661,11 +1661,6 @@ int lua_getmemcat(lua_State* L)
     return L->activememcat;
 }
 
-void lua_setmemcatbyteslimit(lua_State* L, lua_Unsigned bytes_limit)
-{
-    L->global->memcatbyteslimit = bytes_limit;
-}
-
 size_t lua_totalbytes(lua_State* L, int category)
 {
     api_check(L, category < LUA_MEMORY_CATEGORIES);
@@ -2197,7 +2192,6 @@ CLANG_NOOPT void GCC_NOOPT lua_useconstsstate(lua_State *L, lua_State * constsL)
         runtime_state->uuidCompressedWeakTab = consts_runtime_state->uuidCompressedWeakTab;
     }
 
-    L->global->memcatbyteslimit = constsL->global->memcatbyteslimit;
     *lua_callbacks(L) = *lua_callbacks(constsL);
 
     // Make sure we didn't muck up `L` very much

VM/src/llsl.cpp

@@ -1189,34 +1189,6 @@ YieldableStatus luaSL_may_interrupt(lua_State *L)
     return YieldableStatus::OK;
 }
 
-void luaSL_emergencyfinishgc(lua_State *L)
-{
-    global_State *g = L->global;
-    size_t limit = g->memcatbyteslimit;
-    if (!limit)
-    {
-        // Nothing to do here, we're not using memcat memory limits
-        // so we don't need to dynamically tune the GC.
-        return;
-    }
-
-    // We want GC to get more aggressive as memory fills up, but we don't
-    // want to waste a lot of time on GC if we're not really allocating
-    // things very fast anyway. Only start getting aggressive once we start
-    // getting near limits.
-    int cycle_count = 0;
-    while (true) {
-        // We need to fix `GCthreshold` so assertions in lgc hold true.
-        g->GCthreshold = std::min(g->GCthreshold, g->totalbytes);
-
-        // This logic is copied from Lua 5.1.4's emergency collector.
-        /* only allow the GC to finish at least 1 full cycle. */
-        if (g->gcstate == GCSpause && ++cycle_count > 1)
-            break;
-        luaC_step(L, false);
-    }
-}
-
 int luaSL_pushnativeinteger(lua_State *L, int val)
 {
     if (LUAU_IS_LSL_VM(L))

VM/src/lmem.cpp

@@ -507,15 +507,10 @@ void* luaM_new_(lua_State* L, size_t nsize, uint8_t memcat)
     global_State* g = L->global;
 
     // ServerLua: enforce memory allocation limits in user-owned memcats
-    if (memcat > 1 && g->memcatbyteslimit)
-    {
-        if (g->memcatbytes[memcat] + nsize >= g->memcatbyteslimit)
-            luaD_throw(L, LUA_ERRMEM);
-    }
-
     if (LUAU_LIKELY(!!g->cb.beforeallocate) && memcat > 1)
     {
-        if (g->cb.beforeallocate(L, 0, nsize))
+        // But don't do invoke the hook if GC is disabled
+        if (LUAU_LIKELY(g->GCthreshold != SIZE_MAX) && g->cb.beforeallocate(L, 0, nsize))
             luaD_throw(L, LUA_ERRMEM);
     }
 
@@ -551,15 +546,9 @@ GCObject* luaM_newgco_(lua_State* L, size_t nsize, uint8_t memcat)
     global_State* g = L->global;
 
     // ServerLua: enforce memory allocation limits in user-owned memcats
-    if (memcat > 1 && g->memcatbyteslimit)
-    {
-        if (g->memcatbytes[memcat] + nsize >= g->memcatbyteslimit)
-            luaD_throw(L, LUA_ERRMEM);
-    }
-
     if (LUAU_LIKELY(!!g->cb.beforeallocate) && memcat > 1)
     {
-        if (g->cb.beforeallocate(L, 0, nsize))
+        if (LUAU_LIKELY(g->GCthreshold != SIZE_MAX) && g->cb.beforeallocate(L, 0, nsize))
             luaD_throw(L, LUA_ERRMEM);
     }
 
@@ -669,15 +658,9 @@ void* luaM_realloc_(lua_State* L, void* block, size_t osize, size_t nsize, uint8
     LUAU_ASSERT((osize == 0) == (block == NULL));
 
     // ServerLua: enforce memory allocation limits in user-owned memcats
-    if (memcat > 1 && g->memcatbyteslimit)
-    {
-        if (g->memcatbytes[memcat] - osize + nsize >= g->memcatbyteslimit)
-            luaD_throw(L, LUA_ERRMEM);
-    }
-
     if (LUAU_LIKELY(!!g->cb.beforeallocate) && memcat > 1)
     {
-        if (g->cb.beforeallocate(L, 0, nsize))
+        if (LUAU_LIKELY(g->GCthreshold != SIZE_MAX) && g->cb.beforeallocate(L, osize, nsize))
             luaD_throw(L, LUA_ERRMEM);
     }
 

VM/src/lstate.cpp

@@ -233,7 +233,6 @@ lua_State* lua_newstate(lua_Alloc f, void* ud)
 
     g->memcatbytes[0] = sizeof(LG);
     // ServerLua: additional book-keeping
-    g->memcatbyteslimit = 0;
     g->calltailinterruptcheck = 0;
 
     g->cb = lua_Callbacks();

VM/src/lstate.h

@@ -235,11 +235,6 @@ typedef struct global_State
 #ifdef LUAI_GCMETRICS
     GCMetrics gcmetrics;
 #endif
-    // ServerLua: byte allocation limit for memcats above 1,
-    // memcat "1" is treated as the "garbage" memcat, and everything in
-    // it is immediately collectible. 0 is the "system" memcat, and is
-    // used for all allocations that users don't directly control.
-    uint32_t memcatbyteslimit;
 } global_State;
 // clang-format on
 

tests/Conformance.test.cpp

@@ -959,6 +959,9 @@ TEST_CASE("Ares bad serialize state")
     luaC_validate(GL);
 }
 
+// Used just for this test
+static lua_OpaqueGCObjectSet free_objects;
+
 TEST_CASE("ServerLua memory limits")
 {
     std::string source = getConformanceTestSource("serverlua_memory.lua");
@@ -1008,8 +1011,34 @@ TEST_CASE("ServerLua memory limits")
     }
 
     lua_State* Lforker = eris_make_forkserver(L);
-    // 10kb memory limit
-    lua_setmemcatbyteslimit(Lforker, 10000);
+
+    // Collect free objects from the loaded bytecode
+    free_objects = lua_collectfreeobjects(Lforker);
+
+    // 10kb memory limit using beforeallocate callback with reachability-based accounting
+    lua_callbacks(Lforker)->beforeallocate = [](lua_State* L, size_t osize, size_t nsize) -> int {
+        constexpr size_t MAX_MEM = 10000;
+        static size_t actual_size = 0;
+        static size_t approximate_size = 0;
+
+        // Ignore shrinking allocations
+        if (osize >= nsize)
+            return 0;
+
+        size_t net_gain = nsize - osize;
+
+        // Re-calculate actual size if we haven't yet or if approximate would exceed limit
+        if (actual_size == 0 || (approximate_size + net_gain > MAX_MEM))
+        {
+            approximate_size = actual_size = lua_userthreadsize(L, &free_objects);
+
+            if (actual_size + net_gain > MAX_MEM)
+                return 1;
+        }
+
+        approximate_size += net_gain;
+        return 0;
+    };
     // Don't need the original thread on the main stack anymore
     lua_remove(GL, -2);
 

tests/conformance/serverlua_memory.lua

@@ -14,6 +14,7 @@ local success, ret = xpcall(
 		end)
 assert(not success)
 assert(ret == "not enough memory")
+assert(#foo > 225)
 assert(#foo < 20000)
 
 return 'OK'