Skip to content

Commit b50ecbb

Browse files
committed
update
1 parent fc679f6 commit b50ecbb

File tree

6 files changed

+87
-67
lines changed

6 files changed

+87
-67
lines changed
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
use luars::GcUpvalue;
2+
use luars::LuaValue;
3+
use std::mem::size_of;
4+
5+
fn main() {
6+
println!("=== Size Check ===");
7+
println!("LuaValue: {} bytes", size_of::<LuaValue>());
8+
println!("GcUpvalue: {} bytes", size_of::<GcUpvalue>());
9+
}

crates/luars/src/gc/gc_object.rs

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -243,73 +243,76 @@ impl GcFunction {
243243
}
244244
}
245245

246-
/// Upvalue state - uses absolute stack index like Lua C implementation
247-
#[derive(Debug, Clone)]
248-
pub enum UpvalueState {
249-
Open { stack_index: usize },
250-
Closed(LuaValue),
251-
}
252-
253246
/// Upvalue with embedded GC header
247+
///
248+
/// Hybrid design for safety and performance:
249+
/// - When open: uses stack_index (safe, no dangling pointers)
250+
/// - When closed: stores value inline (fast, no indirection)
251+
///
252+
/// Note: We cannot use raw pointers for open upvalues because
253+
/// register_stack may reallocate, invalidating the pointers.
254254
pub struct GcUpvalue {
255255
pub header: GcHeader,
256-
pub state: UpvalueState,
256+
/// The stack index when open (used for accessing stack value)
257+
pub stack_index: usize,
258+
/// Storage for closed value
259+
pub closed_value: LuaValue,
260+
/// Whether the upvalue is open (still pointing to stack)
261+
pub is_open: bool,
257262
}
258263

259264
impl GcUpvalue {
260265
/// Check if this upvalue points to the given absolute stack index
261266
#[inline]
262267
pub fn points_to_index(&self, index: usize) -> bool {
263-
matches!(&self.state, UpvalueState::Open { stack_index } if *stack_index == index)
268+
self.is_open && self.stack_index == index
264269
}
265270

266271
/// Check if this upvalue is open (still points to stack)
267272
#[inline]
268273
pub fn is_open(&self) -> bool {
269-
matches!(&self.state, UpvalueState::Open { .. })
274+
self.is_open
270275
}
271276

272277
/// Close this upvalue with the given value
273278
#[inline]
274279
pub fn close(&mut self, value: LuaValue) {
275-
self.state = UpvalueState::Closed(value);
280+
self.closed_value = value;
281+
self.is_open = false;
276282
}
277283

278284
/// Get the value of a closed upvalue (returns None if still open)
279285
#[inline]
280286
pub fn get_closed_value(&self) -> Option<LuaValue> {
281-
match &self.state {
282-
UpvalueState::Closed(v) => Some(v.clone()),
283-
_ => None,
287+
if self.is_open {
288+
None
289+
} else {
290+
Some(self.closed_value)
284291
}
285292
}
286293

287294
/// Get the absolute stack index if this upvalue is open
288295
#[inline]
289296
pub fn get_stack_index(&self) -> Option<usize> {
290-
match &self.state {
291-
UpvalueState::Open { stack_index } => Some(*stack_index),
292-
_ => None,
297+
if self.is_open {
298+
Some(self.stack_index)
299+
} else {
300+
None
293301
}
294302
}
295303

296304
/// Set closed upvalue value directly without checking state
297305
/// SAFETY: Must only be called when upvalue is in Closed state
298306
#[inline(always)]
299307
pub unsafe fn set_closed_value_unchecked(&mut self, value: LuaValue) {
300-
if let UpvalueState::Closed(ref mut v) = self.state {
301-
*v = value;
302-
}
308+
self.closed_value = value;
303309
}
304310

305311
/// Get closed value reference directly without Option
306312
/// SAFETY: Must only be called when upvalue is in Closed state
307313
#[inline(always)]
308314
pub unsafe fn get_closed_value_ref_unchecked(&self) -> &LuaValue {
309-
match &self.state {
310-
UpvalueState::Closed(v) => v,
311-
_ => unsafe { std::hint::unreachable_unchecked() },
312-
}
315+
&self.closed_value
313316
}
314317
}
315318

crates/luars/src/gc/mod.rs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -562,13 +562,23 @@ impl GC {
562562
}
563563
}
564564
GcObjectType::Upvalue => {
565-
if let Some(upval) = pool.upvalues.get_mut(gc_id.index()) {
565+
// Get closed value first, then release borrow before marking
566+
let closed_value = if let Some(upval) = pool.upvalues.get_mut(gc_id.index()) {
566567
if upval.header.is_gray() {
567568
upval.header.make_black();
568-
if let UpvalueState::Closed(v) = upval.state {
569-
self.mark_value(&v, pool);
569+
if !upval.is_open {
570+
Some(upval.closed_value)
571+
} else {
572+
None
570573
}
574+
} else {
575+
None
571576
}
577+
} else {
578+
None
579+
};
580+
if let Some(v) = closed_value {
581+
self.mark_value(&v, pool);
572582
}
573583
}
574584
GcObjectType::Thread => {
@@ -1175,8 +1185,8 @@ impl GC {
11751185
if let Some(upval) = pool.upvalues.get_mut(upval_id.0) {
11761186
if upval.header.is_white() {
11771187
upval.header.make_black();
1178-
if let UpvalueState::Closed(v) = &upval.state {
1179-
worklist.push(*v);
1188+
if !upval.is_open {
1189+
worklist.push(upval.closed_value);
11801190
}
11811191
}
11821192
}

crates/luars/src/gc/object_pool.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use crate::gc::gc_object::{CFunction, FunctionBody};
1212
use crate::lua_value::{Chunk, LuaThread, LuaUserdata};
1313
use crate::{
1414
FunctionId, GcFunction, GcHeader, GcString, GcTable, GcThread, GcUpvalue, LuaString, LuaTable,
15-
LuaValue, StringId, TableId, ThreadId, UpvalueId, UpvalueState, UserdataId,
15+
LuaValue, StringId, TableId, ThreadId, UpvalueId, UserdataId,
1616
};
1717
use std::rc::Rc;
1818

@@ -1079,20 +1079,26 @@ impl ObjectPool {
10791079

10801080
// ==================== Upvalue Operations ====================
10811081

1082+
/// Create an open upvalue pointing to a stack location
10821083
#[inline]
10831084
pub fn create_upvalue_open(&mut self, stack_index: usize) -> UpvalueId {
10841085
let gc_uv = GcUpvalue {
10851086
header: GcHeader::default(),
1086-
state: UpvalueState::Open { stack_index },
1087+
stack_index,
1088+
closed_value: LuaValue::nil(),
1089+
is_open: true,
10871090
};
10881091
UpvalueId(self.upvalues.alloc(gc_uv))
10891092
}
10901093

1094+
/// Create a closed upvalue with a value
10911095
#[inline]
10921096
pub fn create_upvalue_closed(&mut self, value: LuaValue) -> UpvalueId {
10931097
let gc_uv = GcUpvalue {
10941098
header: GcHeader::default(),
1095-
state: UpvalueState::Closed(value),
1099+
stack_index: 0,
1100+
closed_value: value,
1101+
is_open: false,
10961102
};
10971103
UpvalueId(self.upvalues.alloc(gc_uv))
10981104
}

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ pub use upvalue_instructions::*;
1919
use super::{LuaError, LuaResult, LuaVM, OpCode};
2020
use crate::lua_value::{TAG_FALSE, TAG_FLOAT, TAG_INTEGER, TAG_NIL, TAG_TRUE, TYPE_MASK};
2121
use crate::lua_vm::LuaCallFrame;
22-
use crate::{LuaValue, UpvalueState, get_a, get_ax, get_b, get_bx, get_k, get_op, get_sbx, get_sj};
22+
use crate::{LuaValue, get_a, get_ax, get_b, get_bx, get_k, get_op, get_sbx, get_sj};
2323

2424
/// Save current pc to frame (like Lua C's savepc macro)
2525
/// Called before operations that may call Lua functions (CALL, metamethods, etc.)
@@ -452,6 +452,7 @@ pub fn luavm_execute(vm: &mut LuaVM) -> LuaResult<LuaValue> {
452452
// ============ Upvalue operations (inline for performance) ============
453453
OpCode::GetUpval => {
454454
// INLINED GETUPVAL: R[A] := UpValue[B]
455+
// OPTIMIZED: Direct field access with branch prediction
455456
let a = get_a!(instr);
456457
let b = get_b!(instr);
457458

@@ -460,11 +461,10 @@ pub fn luavm_execute(vm: &mut LuaVM) -> LuaResult<LuaValue> {
460461

461462
// Read upvalue value directly
462463
let uv = vm.object_pool.get_upvalue_unchecked(upvalue_id);
463-
let value = match &uv.state {
464-
UpvalueState::Open { stack_index } => {
465-
*vm.register_stack.as_ptr().add(*stack_index)
466-
}
467-
UpvalueState::Closed(val) => *val,
464+
let value = if uv.is_open {
465+
*vm.register_stack.get_unchecked(uv.stack_index)
466+
} else {
467+
uv.closed_value
468468
};
469469

470470
*vm.register_stack.as_mut_ptr().add(base_ptr + a) = value;
@@ -473,6 +473,7 @@ pub fn luavm_execute(vm: &mut LuaVM) -> LuaResult<LuaValue> {
473473
}
474474
OpCode::SetUpval => {
475475
// INLINED SETUPVAL: UpValue[B] := R[A]
476+
// OPTIMIZED: Direct field access with branch prediction
476477
let a = get_a!(instr);
477478
let b = get_b!(instr);
478479

@@ -485,12 +486,11 @@ pub fn luavm_execute(vm: &mut LuaVM) -> LuaResult<LuaValue> {
485486

486487
// Write upvalue value directly
487488
let uv = vm.object_pool.get_upvalue_mut_unchecked(upvalue_id);
488-
match &mut uv.state {
489-
UpvalueState::Open { stack_index } => {
490-
*vm.register_stack.as_mut_ptr().add(*stack_index) = value;
491-
}
492-
UpvalueState::Closed(val) => *val = value,
493-
};
489+
if uv.is_open {
490+
*vm.register_stack.get_unchecked_mut(uv.stack_index) = value;
491+
} else {
492+
uv.closed_value = value;
493+
}
494494

495495
// GC write barrier for upvalue
496496
vm.gc_barrier_upvalue(upvalue_id, &value);

crates/luars/src/lua_vm/mod.rs

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1554,21 +1554,17 @@ impl LuaVM {
15541554
}
15551555
}
15561556

1557-
/// Fast path for reading upvalue - no bounds checking
1558-
/// SAFETY: uv_id must be valid, and if open, stack_idx must be valid
1557+
/// Fast path for reading upvalue - optimized with branch prediction hint
1558+
/// SAFETY: uv_id must be valid
15591559
#[inline(always)]
15601560
pub unsafe fn read_upvalue_unchecked(&self, uv_id: UpvalueId) -> LuaValue {
15611561
unsafe {
15621562
let uv = self.object_pool.get_upvalue_unchecked(uv_id);
1563-
match &uv.state {
1564-
crate::gc::UpvalueState::Open { stack_index } => {
1565-
// Open upvalue - read directly from stack
1566-
*self.register_stack.get_unchecked(*stack_index)
1567-
}
1568-
crate::gc::UpvalueState::Closed(value) => {
1569-
// Closed upvalue - return stored value (copy)
1570-
*value
1571-
}
1563+
// Use likely hint: open upvalues are more common in hot paths
1564+
if uv.is_open {
1565+
*self.register_stack.get_unchecked(uv.stack_index)
1566+
} else {
1567+
uv.closed_value
15721568
}
15731569
}
15741570
}
@@ -1586,27 +1582,23 @@ impl LuaVM {
15861582
} else {
15871583
// Closed upvalue - update stored value
15881584
if let Some(uv_mut) = self.object_pool.get_upvalue_mut(uv_id) {
1589-
uv_mut.close(value);
1585+
uv_mut.closed_value = value;
15901586
}
15911587
}
15921588
}
15931589
}
15941590

1595-
/// Fast path for writing upvalue - no bounds checking
1596-
/// SAFETY: uv_id must be valid, and if open, stack_idx must be valid
1591+
/// Fast path for writing upvalue - optimized with branch prediction hint
1592+
/// SAFETY: uv_id must be valid
15971593
#[inline(always)]
15981594
pub unsafe fn write_upvalue_unchecked(&mut self, uv_id: UpvalueId, value: LuaValue) {
15991595
unsafe {
16001596
let uv = self.object_pool.get_upvalue_mut_unchecked(uv_id);
1601-
match &mut uv.state {
1602-
crate::gc::UpvalueState::Open { stack_index } => {
1603-
// Open upvalue - write directly to stack
1604-
*self.register_stack.get_unchecked_mut(*stack_index) = value;
1605-
}
1606-
crate::gc::UpvalueState::Closed(v) => {
1607-
// Closed upvalue - update stored value
1608-
*v = value;
1609-
}
1597+
// Use likely hint: open upvalues are more common in hot paths
1598+
if uv.is_open {
1599+
*self.register_stack.get_unchecked_mut(uv.stack_index) = value;
1600+
} else {
1601+
uv.closed_value = value;
16101602
}
16111603
}
16121604
}

0 commit comments

Comments
 (0)