improve upvalue

Author: CppCXY

crates/luars/src/lua_value/mod.rs

@@ -419,8 +419,7 @@ impl PartialEq for LuaString {
 }
 
 /// Inline storage for upvalue pointers — avoids heap allocation for 0-2 upvalues.
-/// Used as a temporary construction helper in handle_closure.
-/// Converted to Box<[UpvaluePtr]> for storage in LuaFunction (branchless deref).
+/// Stored directly in LuaFunction: 0-2 upvalues are inline, 3+ use Box<[UpvaluePtr]>.
 pub enum UpvalueStore {
     Empty,
     One(UpvaluePtr),
@@ -453,6 +452,30 @@ impl UpvalueStore {
         }
     }
 
+    /// Get upvalue pointers as a slice (branchless for Many, single match for 0-2).
+    /// Called once per function entry — branch cost is negligible.
+    #[inline(always)]
+    pub fn as_slice(&self) -> &[UpvaluePtr] {
+        match self {
+            UpvalueStore::Empty => &[],
+            UpvalueStore::One(p) => std::slice::from_ref(p),
+            UpvalueStore::Two(a) => a.as_slice(),
+            UpvalueStore::Many(v) => v,
+        }
+    }
+
+    /// Get mutable upvalue pointers as a slice.
+    /// Used by debug.upvaluejoin (rare).
+    #[inline(always)]
+    pub fn as_mut_slice(&mut self) -> &mut [UpvaluePtr] {
+        match self {
+            UpvalueStore::Empty => &mut [],
+            UpvalueStore::One(p) => std::slice::from_mut(p),
+            UpvalueStore::Two(a) => a.as_mut_slice(),
+            UpvalueStore::Many(v) => v,
+        }
+    }
+
     #[inline(always)]
     pub fn len(&self) -> usize {
         match self {
@@ -466,14 +489,14 @@ impl UpvalueStore {
 
 pub struct LuaFunction {
     chunk: Rc<Chunk>,
-    upvalue_ptrs: Box<[UpvaluePtr]>,
+    upvalue_ptrs: UpvalueStore,
 }
 
 impl LuaFunction {
     pub fn new(chunk: Rc<Chunk>, upvalue_ptrs: UpvalueStore) -> Self {
         LuaFunction {
             chunk,
-            upvalue_ptrs: upvalue_ptrs.into_boxed_slice(),
+            upvalue_ptrs,
         }
     }
 
@@ -483,17 +506,17 @@ impl LuaFunction {
         &self.chunk
     }
 
-    /// Get cached upvalues (direct pointers for fast access)
-    /// Box<[T]> deref is branchless — no match overhead
+    /// Get upvalue pointers as a slice.
+    /// Called once per 'startfunc iteration (function entry) — branch is negligible.
     #[inline(always)]
     pub fn upvalues(&self) -> &[UpvaluePtr] {
-        &self.upvalue_ptrs
+        self.upvalue_ptrs.as_slice()
     }
 
-    /// Get mutable access to cached upvalues for updating pointers
+    /// Get mutable access to upvalue pointers (used by debug.upvaluejoin)
     #[inline(always)]
     pub fn upvalues_mut(&mut self) -> &mut [UpvaluePtr] {
-        &mut self.upvalue_ptrs
+        self.upvalue_ptrs.as_mut_slice()
     }
 }
 

crates/luars/src/lua_vm/execute/closure_handler.rs

@@ -38,14 +38,15 @@ pub fn handle_closure(
     // Create closure from child prototype
     handle_closure_internal(lua_state, base, a, bx, chunk, &upvalue_ptrs)?;
 
-    let a = instr.get_a() as usize;
+    // Save PC for GC, then set top to ra+1 for GC scan scope (like C Lua's checkGC).
+    // Use set_top_raw: stack is already grown by push_lua_frame, so no bounds check needed.
     let new_top = base + a + 1;
     lua_state.set_frame_pc(frame_idx, pc as u32);
-    lua_state.set_top(new_top)?;
+    lua_state.set_top_raw(new_top);
     lua_state.check_gc()?;
 
     let frame_top = lua_state.get_call_info(frame_idx).top;
-    lua_state.set_top(frame_top)?;
+    lua_state.set_top_raw(frame_top);
     Ok(())
 }
 

crates/luars/src/lua_vm/lua_state.rs

@@ -2,7 +2,6 @@
 // Represents a single thread/coroutine execution context
 // Multiple LuaStates can share the same LuaVM (global_State)
 
-use std::collections::HashMap;
 use std::rc::Rc;
 
 use crate::lua_value::{LuaUserdata, LuaValue, LuaValueKind, LuaValuePtr, UpvalueStore};
@@ -45,9 +44,9 @@ pub struct LuaState {
     call_depth: usize,
 
     /// Open upvalues - upvalues pointing to stack locations
-    /// Uses HashMap for O(1) lookup by stack index (unlike Lua's sorted linked list)
-    /// Also maintains a sorted Vec(higher indices first) for efficient traversal during close operations
-    open_upvalues_map: HashMap<usize, UpvaluePtr>,
+    /// Sorted Vec (higher stack indices first) for efficient lookup and close traversal.
+    /// Linear scan is faster than HashMap for typical 0-5 open upvalues due to
+    /// no hashing overhead and better cache locality. Matches C Lua's sorted list design.
     open_upvalues_list: Vec<UpvaluePtr>,
 
     /// Error message storage (lightweight error handling)
@@ -109,7 +108,6 @@ impl LuaState {
             stack_top: 0, // Start with empty stack (Lua's L->top.p = L->stack)
             call_stack: Vec::with_capacity(call_stack_size),
             call_depth: 0,
-            open_upvalues_map: HashMap::new(),
             open_upvalues_list: Vec::new(),
             error_msg: String::new(),
             error_object: LuaValue::nil(),
@@ -862,16 +860,12 @@ impl LuaState {
 
         if count > 0 {
             // Process upvalues to close in-place, then drain.
-            // First pass: close each upvalue and remove from map.
             for i in 0..count {
                 let upval_ptr = self.open_upvalues_list[i];
                 let data = &upval_ptr.as_ref().data;
                 if data.is_open() {
                     let stack_idx = data.get_stack_index();
 
-                    // Remove from map (maintain consistency)
-                    self.open_upvalues_map.remove(&stack_idx);
-
                     // Capture value from stack
                     let value = self
                         .stack
@@ -1288,13 +1282,20 @@ impl LuaState {
         result
     }
 
-    /// Find or create an open upvalue for the given stack index
-    /// Uses HashMap for O(1) lookup instead of O(n) linear search
-    /// This is a major optimization over the naive linked list approach
+    /// Find or create an open upvalue for the given stack index.
+    /// Uses linear scan on sorted Vec — faster than HashMap for typical 0-5 open upvalues.
     pub fn find_or_create_upvalue(&mut self, stack_index: usize) -> LuaResult<UpvaluePtr> {
-        // O(1) lookup in HashMap
-        if let Some(&upval_ptr) = self.open_upvalues_map.get(&stack_index) {
-            return Ok(upval_ptr);
+        // Linear scan on sorted list (descending by stack index).
+        // For 0-5 elements, this is faster than HashMap (no hash computation).
+        for &upval_ptr in &self.open_upvalues_list {
+            let idx = upval_ptr.as_ref().data.get_stack_index();
+            if idx == stack_index {
+                return Ok(upval_ptr);
+            }
+            if idx < stack_index {
+                // Passed the insertion point — not found (list is sorted descending)
+                break;
+            }
         }
 
         // Not found, create a new one
@@ -1306,17 +1307,12 @@ impl LuaState {
             vm.create_upvalue_open(stack_index, ptr)?
         };
 
-        // Add to HashMap for O(1) future lookups
-        self.open_upvalues_map.insert(stack_index, upval_ptr);
-
-        // Also add to sorted list for traversal (insert in sorted position, higher indices first)
-        // Collect existing upvalue IDs and their stack indices
-        let insert_pos = {
-            self.open_upvalues_list
-                .iter()
-                .position(|&ptr| ptr.as_ref().data.get_stack_index() < stack_index)
-                .unwrap_or(self.open_upvalues_list.len())
-        };
+        // Insert in sorted position (higher indices first)
+        let insert_pos = self
+            .open_upvalues_list
+            .iter()
+            .position(|&ptr| ptr.as_ref().data.get_stack_index() < stack_index)
+            .unwrap_or(self.open_upvalues_list.len());
 
         self.open_upvalues_list.insert(insert_pos, upval_ptr);