From ffc0916a6b244402272dedd2579cbb60b6395201 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=88=B1=E7=89=B9=E6=AF=8F=E7=BE=8A=E9=98=9F?= Date: Thu, 25 Jun 2026 22:08:27 +0800 Subject: [PATCH] =?UTF-8?q?refactor:=20Symbol/Iter/Profiling=20=E5=AD=90?= =?UTF-8?q?=E7=BB=93=E6=9E=84=E6=96=B9=E6=B3=95=E4=B8=8B=E6=B2=89=EF=BC=8C?= =?UTF-8?q?IC=20helper=20=E5=AE=8C=E6=88=90=E6=8F=90=E5=8F=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/oxide_vm/src/builtins/symbol.rs | 21 ++-- crates/oxide_vm/src/dispatch/property.rs | 52 +++------- crates/oxide_vm/src/ic_helper.rs | 42 ++++++-- crates/oxide_vm/src/vm.rs | 19 ++-- crates/oxide_vm/src/vm_dispatch_misc.rs | 20 ++-- crates/oxide_vm/src/vm_state.rs | 116 ++++++++++++++++++++++- crates/oxide_vm/src/vm_support.rs | 10 +- 7 files changed, 185 insertions(+), 95 deletions(-) diff --git a/crates/oxide_vm/src/builtins/symbol.rs b/crates/oxide_vm/src/builtins/symbol.rs index 08e8cd9..49c6b15 100644 --- a/crates/oxide_vm/src/builtins/symbol.rs +++ b/crates/oxide_vm/src/builtins/symbol.rs @@ -33,9 +33,7 @@ pub fn symbol_constructor(vm: &mut Vm, args: &[u8]) -> NativeResult { String::new() }; - vm.symbols.symbol_counter = vm.symbols.symbol_counter.wrapping_add(1); - let idx = vm.symbols.symbol_descriptions.len() as u32; - vm.symbols.symbol_descriptions.push(description); + let idx = vm.symbols.intern(description); NativeResult::Ok(JsValue::symbol(idx)) } @@ -49,7 +47,7 @@ pub fn symbol_to_string(vm: &mut Vm, args: &[u8]) -> NativeResult { } let idx = this_val.as_symbol_index(); - let desc = vm.symbols.symbol_descriptions.get(idx as usize).cloned().unwrap_or_default(); + let desc = vm.symbols.description(idx).unwrap_or("").to_string(); let result = format!("Symbol({})", desc); NativeResult::Ok(vm.new_string(&result)) } @@ -61,13 +59,12 @@ pub fn symbol_for(vm: &mut Vm, args: &[u8]) -> NativeResult { "undefined".to_string() }; - if let Some(&idx) = vm.symbols.symbol_registry.get(&key) { + if let Some(idx) = vm.symbols.lookup_global(&key) { return NativeResult::Ok(JsValue::symbol(idx)); } - let idx = vm.symbols.symbol_descriptions.len() as u32; - vm.symbols.symbol_descriptions.push(key.clone()); - vm.symbols.symbol_registry.insert(key, idx); + let idx = vm.symbols.intern(key.clone()); + vm.symbols.register_global(key, idx); NativeResult::Ok(JsValue::symbol(idx)) } @@ -78,13 +75,7 @@ pub fn symbol_key_for(vm: &mut Vm, args: &[u8]) -> NativeResult { } let idx = sym.as_symbol_index(); - let found = vm - .symbols - .symbol_registry - .iter() - .find(|(_, ®istered_idx)| registered_idx == idx) - .map(|(key, _)| key.clone()); - match found { + match vm.symbols.key_for_id(idx) { Some(key) => NativeResult::Ok(vm.new_string(&key)), None => NativeResult::Ok(JsValue::undefined()), } diff --git a/crates/oxide_vm/src/dispatch/property.rs b/crates/oxide_vm/src/dispatch/property.rs index def10c9..6665324 100644 --- a/crates/oxide_vm/src/dispatch/property.rs +++ b/crates/oxide_vm/src/dispatch/property.rs @@ -5,27 +5,9 @@ use oxide_types::object::JsObject; use oxide_types::private_key::make_private_name_id; use oxide_types::value::JsValue; +use crate::ic_helper::{self, ic_get_hit, ic_set_hit}; use crate::vm::{Vm, MAX_PROTO_CHAIN_DEPTH}; -#[inline(always)] -fn ic_get_hit(obj: &JsObject, shape_id: u32, slot_index: u32) -> Option { - if shape_id != 0 && obj.shape_id() == shape_id && slot_index < obj.prop_vec_len() as u32 { - Some(obj.get_prop_at(slot_index)) - } else { - None - } -} - -#[inline(always)] -fn ic_set_hit(obj: &mut JsObject, shape_id: u32, slot_index: u32, value: JsValue) -> bool { - if shape_id != 0 && obj.shape_id() == shape_id && slot_index < obj.prop_vec_len() as u32 { - obj.set_prop_at(slot_index, value); - true - } else { - false - } -} - #[cold] fn prop_cache_miss() {} @@ -182,10 +164,7 @@ impl Vm { let obj = unsafe { &*obj_ptr }; let prop_name_si = self.property_key_si(self.regs[b]); - let ext0 = self.bytecode[self.pc]; - let ext1 = self.bytecode[self.pc + 1]; - let _ext2 = self.bytecode[self.pc + 2]; - self.pc += 3; + let (cached_shape_id, cached_slot) = ic_helper::read_ic_entry(&self.bytecode, &mut self.pc); if obj.has_prop_meta() { let val = self.ordinary_get_with_target(obj, prop_name_si, val, a as u8)?; if self.accessor_frame_target_reg.take().is_none() { @@ -193,19 +172,17 @@ impl Vm { } return Ok(()); } - let cached_shape_id = ext0 & 0x00FF_FFFF; - let cached_slot = ext1; if let Some(value) = ic_get_hit(obj, cached_shape_id, cached_slot) { self.regs[a] = value; - self.profiling.ic_hits.set(self.profiling.ic_hits.get() + 1); + self.profiling.record_ic_hit(); ic_trace!("IC_GET hit shape={} slot={}", cached_shape_id, cached_slot); } else if let Some(template) = self.kernel_core.prop_forge().get_template(obj.shape_id()) { - self.profiling.ic_misses.set(self.profiling.ic_misses.get() + 1); + self.profiling.record_ic_miss(); prop_cache_miss(); if template.prop_name == prop_name_si { if template.position < obj.prop_vec_len() as u32 { - crate::ic_helper::write_ic_back(&mut self.bytecode, self.pc, obj.shape_id(), template.position); + ic_helper::write_ic_back(&mut self.bytecode, self.pc, obj.shape_id(), template.position); ic_debug!( "IC_GET propforge hit shape={} prop={} slot={}", obj.shape_id(), @@ -226,7 +203,7 @@ impl Vm { ic_debug!("IC_GET miss shape={} prop={}", obj.shape_id(), prop_name_si); let resolved = self.ordinary_get(obj, prop_name_si, val)?; if let Some(pos) = self.kernel_core.shape_forge().lookup_position(obj.shape_id(), prop_name_si) { - crate::ic_helper::write_ic_back(&mut self.bytecode, self.pc, obj.shape_id(), pos); + ic_helper::write_ic_back(&mut self.bytecode, self.pc, obj.shape_id(), pos); } self.regs[a] = resolved; } @@ -253,10 +230,7 @@ impl Vm { return Ok(()); } - let ext0 = self.bytecode[self.pc]; - let ext1 = self.bytecode[self.pc + 1]; - let _ext2 = self.bytecode[self.pc + 2]; - self.pc += 3; + let (cached_shape_id, cached_slot) = ic_helper::read_ic_entry(&self.bytecode, &mut self.pc); let value = self.promote_if_needed_for_write_ptr(obj_ptr, self.regs[a]); let receiver = self.regs[rd]; let obj = unsafe { &mut *obj_ptr }; @@ -264,26 +238,24 @@ impl Vm { self.ordinary_set_dispatch(obj, prop_name_si, value, receiver)?; return Ok(()); } - let cached_shape_id = ext0 & 0x00FF_FFFF; - let cached_slot = ext1; if ic_set_hit(obj, cached_shape_id, cached_slot, value) { - self.profiling.ic_hits.set(self.profiling.ic_hits.get() + 1); + self.profiling.record_ic_hit(); ic_trace!("IC_SET hit shape={} slot={}", cached_shape_id, cached_slot); } else if let Some(pos) = self.kernel_core.shape_forge().lookup_position(obj.shape_id(), prop_name_si) { - self.profiling.ic_misses.set(self.profiling.ic_misses.get() + 1); + self.profiling.record_ic_miss(); prop_cache_miss(); obj.set_prop_at(pos, value); - crate::ic_helper::write_ic_back(&mut self.bytecode, self.pc, obj.shape_id(), pos); + ic_helper::write_ic_back(&mut self.bytecode, self.pc, obj.shape_id(), pos); ic_debug!("IC_SET write-back shape={} slot={}", obj.shape_id(), pos); } else { - self.profiling.ic_misses.set(self.profiling.ic_misses.get() + 1); + self.profiling.record_ic_miss(); prop_cache_miss(); let old_shape = obj.shape_id(); self.ordinary_set_dispatch(obj, prop_name_si, value, receiver)?; if old_shape != obj.shape_id() { if let Some(pos) = self.kernel_core.shape_forge().lookup_position(obj.shape_id(), prop_name_si) { - crate::ic_helper::write_ic_back(&mut self.bytecode, self.pc, obj.shape_id(), pos); + ic_helper::write_ic_back(&mut self.bytecode, self.pc, obj.shape_id(), pos); ic_debug!("IC_SET write-back shape={} slot={}", obj.shape_id(), pos); self.kernel_core.prop_forge().upsert( obj.shape_id(), diff --git a/crates/oxide_vm/src/ic_helper.rs b/crates/oxide_vm/src/ic_helper.rs index 7f3b3ce..3bbafc8 100644 --- a/crates/oxide_vm/src/ic_helper.rs +++ b/crates/oxide_vm/src/ic_helper.rs @@ -1,15 +1,26 @@ //! Inline-cache (IC) bytecode-stream helpers. //! -//! These operate purely on the bytecode `Instr` slice — they read/clear/write -//! the IC extension words embedded after IC-bearing opcodes. They take no `Vm` -//! state, so they live outside `Vm` for clearer ownership and to ease the future -//! IC side-table migration. +//! Contains ALL IC read/write/clear logic — property dispatch and member +//! update handlers route through here, so IC format changes (like the future +//! side-table migration) require editing only this file. use oxide_bytecode::opcode::{self, Instr}; +use oxide_types::object::JsObject; +use oxide_types::value::JsValue; + +/// Read the three IC extension words at `pc` (the bytes following an +/// IC-bearing opcode), advance `pc` past them, and return the decoded +/// `(shape_id, slot)`. +pub(crate) fn read_ic_entry(bytecode: &[Instr], pc: &mut usize) -> (u32, u32) { + let ext0 = bytecode[*pc]; + let ext1 = bytecode[*pc + 1]; + *pc += 3; + (ext0 & 0x00FF_FFFF, ext1) +} /// Write a resolved `(shape_id, slot)` back into the three IC extension words -/// that precede the current `pc`. Called on an IC miss after resolving a -/// property, so the next execution of this site hits the cache. +/// at `pc - 3` (they precede the current instruction). Called on an IC miss +/// after resolving a property, so the next execution of this site hits the cache. pub(crate) fn write_ic_back(bytecode: &mut [Instr], pc: usize, shape_id: u32, slot_index: u32) { debug_assert!(pc >= 3, "IC write-back requires 3 extension words before pc"); bytecode[pc - 3] = shape_id & 0x00FF_FFFF; @@ -35,3 +46,22 @@ pub(crate) fn clear_ic_caches(bytecode: &mut [Instr]) { } } } + +#[inline(always)] +pub(crate) fn ic_get_hit(obj: &JsObject, shape_id: u32, slot_index: u32) -> Option { + if shape_id != 0 && obj.shape_id() == shape_id && slot_index < obj.prop_vec_len() as u32 { + Some(obj.get_prop_at(slot_index)) + } else { + None + } +} + +#[inline(always)] +pub(crate) fn ic_set_hit(obj: &mut JsObject, shape_id: u32, slot_index: u32, value: JsValue) -> bool { + if shape_id != 0 && obj.shape_id() == shape_id && slot_index < obj.prop_vec_len() as u32 { + obj.set_prop_at(slot_index, value); + true + } else { + false + } +} diff --git a/crates/oxide_vm/src/vm.rs b/crates/oxide_vm/src/vm.rs index 336027f..140e00e 100644 --- a/crates/oxide_vm/src/vm.rs +++ b/crates/oxide_vm/src/vm.rs @@ -281,7 +281,7 @@ impl Vm { pub(crate) fn alloc_object(&mut self, obj: JsObject) -> *mut JsObject { let ptr = self.epoch.alloc(obj); - self.gc_state.epoch_object_ptrs.push(ptr); + self.gc_state.track_epoch_object(ptr); ptr } @@ -367,12 +367,7 @@ impl Vm { } pub fn ic_hit_rate(&self) -> f64 { - let total = self.profiling.ic_hits.get() + self.profiling.ic_misses.get(); - if total == 0 { - 0.0 - } else { - self.profiling.ic_hits.get() as f64 / total as f64 - } + self.profiling.ic_hit_rate() } pub fn instruction_count(&self) -> u64 { @@ -380,7 +375,7 @@ impl Vm { } pub fn symbol_registry_len(&self) -> usize { - self.symbols.symbol_registry.len() + self.symbols.registry_len() } pub fn ic_hit_count(&self) -> u64 { @@ -707,7 +702,7 @@ impl Vm { steps += 1; if let Some(max_steps) = self.kernel_core.config.max_steps { if steps > max_steps { - self.profiling.instruction_count = steps; + self.profiling.set_instruction_count(steps); return Err(format!("VM step limit exceeded at pc={}", self.pc)); } } @@ -727,7 +722,7 @@ impl Vm { OpCode::NOP => {} OpCode::HALT => { - self.profiling.instruction_count = steps; + self.profiling.set_instruction_count(steps); return Ok(self.regs[0]); } @@ -1257,9 +1252,7 @@ mod tests { #[test] fn full_reset_clears_symbol_state() { let mut vm = Vm::new(); - vm.symbols.symbol_counter = 1; - vm.symbols.symbol_descriptions.push("shared".to_string()); - vm.symbols.symbol_registry.insert("shared".to_string(), 0); + vm.symbols.intern("shared".to_string()); vm.full_reset(); diff --git a/crates/oxide_vm/src/vm_dispatch_misc.rs b/crates/oxide_vm/src/vm_dispatch_misc.rs index 4ae8c46..c32e2d7 100644 --- a/crates/oxide_vm/src/vm_dispatch_misc.rs +++ b/crates/oxide_vm/src/vm_dispatch_misc.rs @@ -309,12 +309,12 @@ impl Vm { } let iter = self.epoch.alloc(ForInIter { keys: keys_vec, index: 0 }); - self.iters.for_in_iters.push(iter.cast::>()); + self.iters.push_for_in(iter.cast::>()); Ok(()) } pub(crate) fn dispatch_for_in_next(&mut self, rd: usize) -> Result<(), String> { - let iter_ptr = self.iters.for_in_iters.last().copied().unwrap_or(std::ptr::null_mut()); + let iter_ptr = self.iters.last_for_in(); if iter_ptr.is_null() { return Err("FOR_IN_NEXT without active iterator".into()); } @@ -329,7 +329,7 @@ impl Vm { } pub(crate) fn dispatch_for_in_done(&mut self, rd: usize) { - let iter_ptr = self.iters.for_in_iters.last().copied().unwrap_or(std::ptr::null_mut()); + let iter_ptr = self.iters.last_for_in(); if iter_ptr.is_null() { self.regs[rd] = JsValue::bool(true); } else { @@ -339,15 +339,15 @@ impl Vm { } pub(crate) fn dispatch_for_in_cleanup(&mut self) { - self.iters.for_in_iters.pop(); + self.iters.pop_for_in(); } pub(crate) fn dispatch_for_of_init(&mut self, a: usize) -> Result<(), String> { let iterable = self.regs[a]; match crate::builtins::iterator::make_iterator_for_value(self, iterable) { Ok(iterator) => { - self.iters.for_of_iters.push(iterator); - self.iters.last_for_of_result = JsValue::undefined(); + self.iters.push_for_of(iterator); + self.iters.clear_last_for_of_result(); Ok(()) } Err(err) => { @@ -359,7 +359,7 @@ impl Vm { } pub(crate) fn dispatch_for_of_done(&mut self, rd: usize) -> Result<(), String> { - let Some(iterator) = self.iters.for_of_iters.last().copied() else { + let Some(iterator) = self.iters.last_for_of() else { return Err("FOR_OF_DONE without active iterator".into()); }; if !iterator.is_object() { @@ -378,13 +378,13 @@ impl Vm { let done_si = self.kernel_core.perm_interner().intern("done").0; let done_val = self.ordinary_get(result_obj, done_si, result)?; let done = crate::coercion::to_boolean(done_val); - self.iters.last_for_of_result = result; + self.iters.set_last_for_of_result(result); self.regs[rd] = JsValue::bool(!done); Ok(()) } pub(crate) fn dispatch_for_of_next(&mut self, rd: usize) -> Result<(), String> { - let result = self.iters.last_for_of_result; + let result = self.iters.last_for_of_result(); if !result.is_object() { self.regs[rd] = JsValue::undefined(); return Ok(()); @@ -396,7 +396,7 @@ impl Vm { } pub(crate) fn dispatch_for_of_close(&mut self) -> Result<(), String> { - let Some(iterator) = self.iters.for_of_iters.pop() else { + let Some(iterator) = self.iters.pop_for_of() else { return Ok(()); }; if !iterator.is_object() { diff --git a/crates/oxide_vm/src/vm_state.rs b/crates/oxide_vm/src/vm_state.rs index 5c0a87f..ed5bdb6 100644 --- a/crates/oxide_vm/src/vm_state.rs +++ b/crates/oxide_vm/src/vm_state.rs @@ -21,6 +21,10 @@ use crate::session_gc::SessionGc; use crate::vm::ForInIter; /// Session-arena and garbage-collection bookkeeping. +/// +/// Field-classify only. `mark`/`sweep`/`rewrite_vm_roots` live on `Vm` because +/// GC scans all sub-struct roots (regs, frames, for_in_iters, for_of_iters, +/// exception_value, etc.) — they cannot be confined to GcState. pub(crate) struct GcState { pub(crate) session_epoch: bumpalo::Bump, pub(crate) session_gc: SessionGc, @@ -28,13 +32,15 @@ pub(crate) struct GcState { pub(crate) session_object_ptrs: Vec<*mut JsObject>, pub(crate) session_string_ptrs: Vec<*mut JsString>, pub(crate) session_bytes_allocated: usize, - /// Long-lived forwarding map reused by GC sweep and promote. - /// `mem::take`'d out for use, `clear()`'d and put back — capacity retained - /// across collections, so no per-event `HashMap` allocation. `FxBuildHasher` - /// because the keys are raw pointers (collision resistance is irrelevant). pub(crate) forwarding: HashMap<*mut JsObject, *mut JsObject, FxBuildHasher>, } +impl GcState { + pub(crate) fn track_epoch_object(&mut self, ptr: *mut JsObject) { + self.epoch_object_ptrs.push(ptr); + } +} + /// `Symbol` interning state. pub(crate) struct SymbolState { pub(crate) symbol_counter: u32, @@ -42,6 +48,41 @@ pub(crate) struct SymbolState { pub(crate) symbol_registry: HashMap, } +impl SymbolState { + pub(crate) fn reset(&mut self) { + self.symbol_counter = 0; + self.symbol_descriptions.clear(); + self.symbol_registry.clear(); + } + + pub(crate) fn intern(&mut self, description: String) -> u32 { + self.symbol_counter = self.symbol_counter.wrapping_add(1); + let idx = self.symbol_descriptions.len() as u32; + self.symbol_descriptions.push(description); + idx + } + + pub(crate) fn register_global(&mut self, key: String, id: u32) { + self.symbol_registry.insert(key, id); + } + + pub(crate) fn lookup_global(&self, key: &str) -> Option { + self.symbol_registry.get(key).copied() + } + + pub(crate) fn description(&self, id: u32) -> Option<&str> { + self.symbol_descriptions.get(id as usize).map(|s| s.as_str()) + } + + pub(crate) fn key_for_id(&self, id: u32) -> Option { + self.symbol_registry.iter().find(|(_, &v)| v == id).map(|(k, _)| k.clone()) + } + + pub(crate) fn registry_len(&self) -> usize { + self.symbol_registry.len() + } +} + /// Live iterator state for `for-in` / `for-of`. pub(crate) struct IterState { pub(crate) for_in_iters: Vec<*mut ForInIter<'static>>, @@ -49,9 +90,76 @@ pub(crate) struct IterState { pub(crate) last_for_of_result: JsValue, } +impl IterState { + pub(crate) fn reset(&mut self) { + self.for_in_iters.clear(); + self.for_of_iters.clear(); + self.last_for_of_result = JsValue::undefined(); + } + + pub(crate) fn push_for_in(&mut self, iter: *mut ForInIter<'static>) { + self.for_in_iters.push(iter); + } + + pub(crate) fn pop_for_in(&mut self) { + self.for_in_iters.pop(); + } + + pub(crate) fn last_for_in(&self) -> *mut ForInIter<'static> { + self.for_in_iters.last().copied().unwrap_or(std::ptr::null_mut()) + } + + pub(crate) fn push_for_of(&mut self, val: JsValue) { + self.for_of_iters.push(val); + } + + pub(crate) fn last_for_of(&self) -> Option { + self.for_of_iters.last().copied() + } + + pub(crate) fn pop_for_of(&mut self) -> Option { + self.for_of_iters.pop() + } + + pub(crate) fn last_for_of_result(&self) -> JsValue { + self.last_for_of_result + } + + pub(crate) fn set_last_for_of_result(&mut self, val: JsValue) { + self.last_for_of_result = val; + } + + pub(crate) fn clear_last_for_of_result(&mut self) { + self.last_for_of_result = JsValue::undefined(); + } +} + /// Inline-cache hit/miss and instruction counters. pub(crate) struct ProfilingState { pub(crate) ic_hits: Cell, pub(crate) ic_misses: Cell, pub(crate) instruction_count: u64, } + +impl ProfilingState { + pub(crate) fn record_ic_hit(&self) { + self.ic_hits.set(self.ic_hits.get() + 1); + } + + pub(crate) fn record_ic_miss(&self) { + self.ic_misses.set(self.ic_misses.get() + 1); + } + + pub(crate) fn set_instruction_count(&mut self, count: u64) { + self.instruction_count = count; + } + + pub(crate) fn ic_hit_rate(&self) -> f64 { + let total = self.ic_hits.get() + self.ic_misses.get(); + if total == 0 { + 0.0 + } else { + self.ic_hits.get() as f64 / total as f64 + } + } +} diff --git a/crates/oxide_vm/src/vm_support.rs b/crates/oxide_vm/src/vm_support.rs index 46ea936..2e1144c 100644 --- a/crates/oxide_vm/src/vm_support.rs +++ b/crates/oxide_vm/src/vm_support.rs @@ -161,9 +161,7 @@ impl Vm { self.gc_state.session_bytes_allocated = 0; self.gc_state.session_gc = crate::session_gc::SessionGc::new(); self.free_session_string_heap_data(); - self.symbols.symbol_counter = 0; - self.symbols.symbol_descriptions.clear(); - self.symbols.symbol_registry.clear(); + self.symbols.reset(); self.root_reg_limit = 0; self.active_reg_limit = 0; } @@ -189,9 +187,7 @@ impl Vm { self.regs = [JsValue::undefined(); 256]; self.pc = 0; self.frames.clear(); - self.iters.for_in_iters.clear(); - self.iters.for_of_iters.clear(); - self.iters.last_for_of_result = JsValue::undefined(); + self.iters.reset(); self.saved_bytecode_stack.clear(); self.saved_immutables_stack.clear(); self.save_stack.clear(); @@ -278,7 +274,7 @@ impl Vm { let prototype_obj = self .epoch .alloc(JsObject::new_empty(EMPTY_SHAPE_ID, JsValue::from_js_object(object_proto_ptr))); - self.gc_state.epoch_object_ptrs.push(prototype_obj); + self.gc_state.track_epoch_object(prototype_obj); let prototype_val = JsValue::from_js_object(prototype_obj); let constructor_si = self.kernel_core.perm_interner().intern("constructor").0;