Rename table.shrink() reorder parameter to be more descriptive, handle no-op better

Author: HaroldCindy

VM/src/ltable.cpp

@@ -1063,11 +1063,11 @@ void luaH_overrideiterorder(lua_State* L, LuaTable* t, int override)
 }
 
 // ServerLua: shrink table to optimal size
-void luaH_shrink(lua_State* L, LuaTable* t, bool reorder)
+void luaH_shrink(lua_State* L, LuaTable* t, bool shrink_sparse)
 {
     // Shrink table to reduce memory usage.
-    // If reorder=false (default), preserves iteration order by not moving elements to hash.
-    // If reorder=true, may shrink to boundary and move sparse elements to hash.
+    // If shrink_sparse=false (default), elements stay in their current part (array or hash).
+    // If shrink_sparse=true, sparse array elements beyond boundary may move to hash to save memory.
 
     // No-op for empty tables
     if (!t->sizearray && !t->lsizenode)
@@ -1098,8 +1098,8 @@ void luaH_shrink(lua_State* L, LuaTable* t, bool reorder)
             hash_count++;
     }
 
-    // If reorder allowed and there are sparse elements, try shrinking to boundary
-    if (reorder && sparse_count > 0)
+    // If shrink_sparse, try shrinking array to boundary and moving sparse elements to hash
+    if (shrink_sparse && sparse_count > 0)
     {
         // We're allowed to move things to the hash part of the table, check if it would be cheaper
         // memory-wise to do that than it would be for us to just shrink array and node to their minimum
@@ -1108,20 +1108,33 @@ void luaH_shrink(lua_State* L, LuaTable* t, bool reorder)
         int new_hash_count = hash_count + sparse_count;
         // hash portion resizes in pow2 increments, sometimes putting things in the hash is basically
         // free if we're already using it.
-        int reorder_hash_capacity = 1 << ceillog2(new_hash_count);
-        int no_reorder_hash_capacity = hash_count == 0 ? 0 : (1 << ceillog2(hash_count));
-        int reorder_cost = boundary + (2 * reorder_hash_capacity);
-        int no_reorder_cost = max_used_idx + (2 * no_reorder_hash_capacity);
+        int sparse_hash_capacity = 1 << ceillog2(new_hash_count);
+        int keep_hash_capacity = hash_count == 0 ? 0 : (1 << ceillog2(hash_count));
+        int sparse_cost = boundary + (2 * sparse_hash_capacity);
+        int keep_cost = max_used_idx + (2 * keep_hash_capacity);
 
         // Would this be smaller memory-wise if we moved some things to the hash portion?
-        if (reorder_cost < no_reorder_cost)
+        if (sparse_cost < keep_cost)
         {
             resize(L, t, boundary, new_hash_count);
             return;
         }
     }
 
-    // Default: shrink to max_used_idx (no elements move, preserves iteration order)
-    if (max_used_idx < t->sizearray || hash_count < orig_sizenode)
+    // Compute actual new hash capacity to avoid rebuilds that can't shrink
+    bool array_shrinks = max_used_idx < t->sizearray;
+    int new_node_capacity = hash_count == 0 ? 0 : twoto(ceillog2(hash_count));
+    bool hash_shrinks = new_node_capacity < orig_sizenode;
+
+    if (hash_shrinks)
+    {
+        // Hash genuinely shrinks (possibly array too), full resize needed
         resize(L, t, max_used_idx, hash_count);
+    }
+    else if (array_shrinks)
+    {
+        // Only array needs shrinking. All elements past max_used_idx are nil
+        // so no elements move to hash, plain array realloc suffices.
+        setarrayvector(L, t, max_used_idx);
+    }
 }

VM/src/ltable.h

@@ -37,8 +37,8 @@ LUAI_FUNC void luaH_clear(LuaTable* tt);
 // ServerLua: set whether or not a custom iter order should be used
 LUAI_FUNC void luaH_overrideiterorder(lua_State* L, LuaTable* t, int override);
 // ServerLua: shrink table to optimal size
-// If reorder=true, may move sparse array elements to hash (changes iteration order)
-LUAI_FUNC void luaH_shrink(lua_State* L, LuaTable* t, bool reorder);
+// If shrink_sparse=true, may move sparse array elements to hash to save memory
+LUAI_FUNC void luaH_shrink(lua_State* L, LuaTable* t, bool shrink_sparse);
 
 #define luaH_setslot(L, t, slot, key) (invalidateTMcache(t), (slot == luaO_nilobject ? luaH_newkey(L, t, key) : cast_to(TValue*, slot)))
 

VM/src/ltablib.cpp

@@ -865,8 +865,8 @@ static int tshrink(lua_State* L)
     if (t->memcat < LUA_FIRST_USER_MEMCAT)
         luaL_argerror(L, 1, "cannot shrink system table");
 
-    bool reorder = lua_toboolean(L, 2) != 0;
-    luaH_shrink(L, t, reorder);
+    bool shrink_sparse = lua_toboolean(L, 2) != 0;
+    luaH_shrink(L, t, shrink_sparse);
     lua_pushvalue(L, 1);
     return 1;
 }

tests/conformance/table_sizing.lua

@@ -172,6 +172,20 @@ t.c = 3
 table.shrink(t)
 check_sizes(t, 0, 4, "shrunk hash only")  -- 4 is smallest power-of-2 >= 3
 
+-- Shrink is no-op when hash is already at minimal capacity (no useless rebuild)
+t = {}
+for i = 1, 12 do t[`key {i}`] = true end
+check_sizes(t, 0, 16, "12 elements in 16-slot hash")
+local order_before = {}
+for k in t do table.insert(order_before, k) end
+table.shrink(t)
+check_sizes(t, 0, 16, "hash capacity unchanged after shrink")
+local order_after = {}
+for k in t do table.insert(order_after, k) end
+for i = 1, #order_before do
+    assert(order_before[i] == order_after[i], "iteration order preserved when hash cannot shrink")
+end
+
 -- Shrink mixed array/hash - dense path shrinks to exact boundary
 t = make_array(100)
 t.foo = "bar"
@@ -218,19 +232,19 @@ assert(t[1] == true, "t[1] preserved after shrink")
 assert(t[100] == true, "t[100] preserved after shrink")
 assert(t.key == "value", "hash data preserved after shrink")
 
--- Default shrink preserves iteration order (no elements move to hash)
+-- Default shrink keeps elements in their current part (no elements move to hash)
 t = table.create(2000)
 for i = 1, 600 do t[i] = true end
 t[1500] = true  -- Sparse element in array at high index
 table.shrink(t)
 check_sizes(t, 1500, 0, "default shrink - preserves order")
 
--- With reorder=true, sparse elements move to hash for better memory usage
+-- With shrink_sparse=true, sparse elements move to hash for better memory usage
 t = table.create(2000)
 for i = 1, 600 do t[i] = true end
 t[1500] = true
 table.shrink(t, true)
-check_sizes(t, 600, 1, "reorder shrink - sparse element to hash")
+check_sizes(t, 600, 1, "shrink_sparse - sparse element to hash")
 assert(t[1500] == true, "sparse element preserved after move to hash")
 
 -- Reorder with element closer to boundary
@@ -239,17 +253,17 @@ for i = 1, 600 do t[i] = true end
 t[700] = true  -- Sparse element
 table.shrink(t, true)
 -- boundary=600, element at 700 moves to hash, array shrinks to 600
-check_sizes(t, 600, 1, "reorder shrink - element near boundary to hash")
+check_sizes(t, 600, 1, "shrink_sparse - element near boundary to hash")
 
--- Even with reorder=true, if moving elements to hash would grow the table,
+-- Even with shrink_sparse=true, if moving elements to hash would grow the table,
 -- we fall back to shrinking to max_used_idx instead.
 t = table.create(100)
 t[1] = true
 for i = 3, 52 do t[i] = true end
 check_sizes(t, 100, 0, "starts at 100 array elements")
 table.shrink(t, true)
 -- Falls back to max_used_idx since boundary shrink would grow table
-check_sizes(t, 52, 0, "reorder fallback - boundary too expensive")
+check_sizes(t, 52, 0, "shrink_sparse fallback - boundary too expensive")
 
 -- Idempotent - second shrink is no-op
 t = make_array(1000)