diff --git a/third_party/move/move-vm/integration-tests/src/tests/instruction_cache_bench.rs b/third_party/move/move-vm/integration-tests/src/tests/instruction_cache_bench.rs new file mode 100644 index 00000000000..f2197b0232b --- /dev/null +++ b/third_party/move/move-vm/integration-tests/src/tests/instruction_cache_bench.rs @@ -0,0 +1,101 @@ +// Copyright (c) The Move Contributors +// SPDX-License-Identifier: Apache-2.0 + +//! Micro-benchmark for the per-instruction cache hot path (Vec vs HashMap +//! representation). Exercises all four memoized instruction kinds — `Call`, +//! `CallGeneric`, `Pack`, `PackGeneric` — on the cache-hit path inside a tight +//! loop, which is where a HashMap lookup would cost more than a Vec index. +//! +//! Ignored by default (it is a benchmark, not a correctness test). Run with: +//! cargo test -p move-vm-integration-tests instruction_cache_hot_path_bench \ +//! -- --ignored --nocapture + +use crate::{ + compiler::{as_module, compile_units}, + tests::execute_function_with_single_storage_for_test, +}; +use move_core_types::{ + account_address::AccountAddress, identifier::Identifier, value::MoveValue, +}; +use move_vm_test_utils::InMemoryStorage; +use std::time::Instant; + +const TEST_ADDR: AccountAddress = AccountAddress::new([42; AccountAddress::LENGTH]); + +#[ignore] +#[test] +fn instruction_cache_hot_path_bench() { + // Each loop iteration performs: foo->step (Call), step->wrap + // (CallGeneric), Box construction (PackGeneric), step->pk (Call), + // P construction (Pack). After the first iteration every one of these is a + // cache hit, so the loop measures hit-path lookup cost. + let code = format!( + r#" + module 0x{}::M {{ + struct Box has drop {{ v: T }} + struct P has drop {{ a: u64 }} + + fun wrap(v: T): Box {{ Box {{ v }} }} + fun pk(x: u64): u64 {{ let _p = P {{ a: x }}; x }} + + fun step(x: u64): u64 {{ + let _b = wrap(x); + pk(x) + }} + + fun foo(n: u64): u64 {{ + let i = 0; + let acc = 0; + while (i < n) {{ + acc = acc + step(i); + i = i + 1; + }}; + acc + }} + }} + "#, + TEST_ADDR.to_hex() + ); + + let mut units = compile_units(&code).unwrap(); + let m = as_module(units.pop().unwrap()); + let mut blob = vec![]; + m.serialize(&mut blob).unwrap(); + + let mut storage = InMemoryStorage::new(); + storage.add_module_bytes(m.self_addr(), m.self_name(), blob.into()); + + let module_id = m.self_id(); + let fun_name = Identifier::new("foo").unwrap(); + + const N: u64 = 3_000_000; + const RUNS: usize = 5; + + let call = || { + let args = vec![MoveValue::U64(N).simple_serialize().unwrap()]; + execute_function_with_single_storage_for_test( + &storage, &module_id, &fun_name, &[], args, + ) + .expect("execution succeeds"); + }; + + // Warm up (module load, type resolution) so we time steady-state execution. + call(); + + let mut best = f64::MAX; + for run in 0..RUNS { + let start = Instant::now(); + call(); + let elapsed = start.elapsed(); + let ns_per_iter = elapsed.as_nanos() as f64 / N as f64; + best = best.min(ns_per_iter); + println!( + "run {}: {:?} total, {:.3} ns/iter ({} iters)", + run, elapsed, ns_per_iter, N + ); + } + println!( + "instruction_cache_hot_path_bench: best = {:.3} ns/iter over {} runs of {} iters", + best, RUNS, N + ); +} diff --git a/third_party/move/move-vm/integration-tests/src/tests/mod.rs b/third_party/move/move-vm/integration-tests/src/tests/mod.rs index 0887d9bb861..b0891ce4ffd 100644 --- a/third_party/move/move-vm/integration-tests/src/tests/mod.rs +++ b/third_party/move/move-vm/integration-tests/src/tests/mod.rs @@ -24,6 +24,7 @@ mod binary_format_version; mod exec_func_effects_tests; mod function_arg_tests; mod instantiation_tests; +mod instruction_cache_bench; mod invariant_violation_tests; mod leak_tests; mod loader_tests; diff --git a/third_party/move/move-vm/runtime/src/frame_type_cache.rs b/third_party/move/move-vm/runtime/src/frame_type_cache.rs index f60de5209ef..2bd9195f40b 100644 --- a/third_party/move/move-vm/runtime/src/frame_type_cache.rs +++ b/third_party/move/move-vm/runtime/src/frame_type_cache.rs @@ -12,7 +12,41 @@ use move_binary_format::{ }; use move_core_types::gas_algebra::NumTypeNodes; use move_vm_types::loaded_data::runtime_types::Type; -use std::{cell::RefCell, collections::BTreeMap, rc::Rc}; +use std::{ + cell::RefCell, + collections::{BTreeMap, HashMap}, + hash::{BuildHasherDefault, Hasher}, + rc::Rc, +}; + +/// Hasher for the per-instruction cache's `u16` program-counter keys. The keys +/// are small, dense, and non-adversarial (bytecode offsets), so the value is +/// used directly as the hash rather than paying for the default SipHash. This +/// keeps a lookup close to a direct index while still allowing the cache to be +/// a sparse map that only holds entries for instructions that actually ran. +#[derive(Default)] +pub(crate) struct PcHasher(u64); + +impl Hasher for PcHasher { + fn finish(&self) -> u64 { + self.0 + } + + fn write_u16(&mut self, i: u16) { + self.0 = i as u64; + } + + fn write(&mut self, bytes: &[u8]) { + // Keys are `u16`, so `write_u16` is what actually gets called. This + // fallback only exists to satisfy the trait and is never on the hot + // path. + for &b in bytes { + self.0 = (self.0 << 8) ^ u64::from(b); + } + } +} + +type BuildPcHasher = BuildHasherDefault; #[allow(dead_code)] pub(crate) trait RuntimeCacheTraits { @@ -40,7 +74,6 @@ impl RuntimeCacheTraits for AllRuntimeCaches { #[derive(Clone)] #[allow(dead_code)] pub(crate) enum PerInstructionCache { - Nothing, Pack(u16), PackGeneric(u16), Call(Rc, Rc>), @@ -79,12 +112,12 @@ pub(crate) struct FrameTypeCache { /// small. The caches are indexed by the index of the given /// bytecode instruction in the function body. /// - /// Important! - If entry is present for a given instruction, then + /// Important! - If an entry is present for a given instruction, then /// we do NOT need to re-check for any errors that only depend on /// the argument of the bytecode instructions, for which it is /// guaranteed that everything will be exactly the same as when we /// did the insertion. - pub(crate) per_instruction_cache: Vec, + pub(crate) per_instruction_cache: HashMap, } impl FrameTypeCache { @@ -239,13 +272,8 @@ impl FrameTypeCache { Rc::new(RefCell::::new(Default::default())) } - pub(crate) fn make_rc_for_function(function: &LoadedFunction) -> Rc> { - let frame_cache = Rc::new(RefCell::::new(Default::default())); - - frame_cache - .borrow_mut() - .per_instruction_cache - .resize(function.code_size(), PerInstructionCache::Nothing); - frame_cache + /// Creates a frame cache for a callee frame. + pub(crate) fn make_rc_for_function(_function: &LoadedFunction) -> Rc> { + Self::make_rc() } } diff --git a/third_party/move/move-vm/runtime/src/interpreter.rs b/third_party/move/move-vm/runtime/src/interpreter.rs index b6af52e4f22..05e49d81b5f 100644 --- a/third_party/move/move-vm/runtime/src/interpreter.rs +++ b/third_party/move/move-vm/runtime/src/interpreter.rs @@ -355,21 +355,23 @@ impl InterpreterImpl<'_> { let (function, frame_cache) = if RTCaches::caches_enabled() { let current_frame_cache = &mut *current_frame.frame_cache.borrow_mut(); - if let PerInstructionCache::Call(ref function, ref frame_cache) = - current_frame_cache.per_instruction_cache[current_frame.pc as usize] + if let Some(PerInstructionCache::Call(function, frame_cache)) = + current_frame_cache.per_instruction_cache.get(¤t_frame.pc) { (Rc::clone(function), Rc::clone(frame_cache)) } else { match current_frame_cache.sub_frame_cache.entry(fh_idx) { btree_map::Entry::Occupied(entry) => { - let entry = entry.get(); - current_frame_cache.per_instruction_cache - [current_frame.pc as usize] = PerInstructionCache::Call( - Rc::clone(&entry.0), - Rc::clone(&entry.1), + let (function, frame_cache) = entry.get(); + current_frame_cache.per_instruction_cache.insert( + current_frame.pc, + PerInstructionCache::Call( + Rc::clone(function), + Rc::clone(frame_cache), + ), ); - (Rc::clone(&entry.0), Rc::clone(&entry.1)) + (Rc::clone(function), Rc::clone(frame_cache)) }, btree_map::Entry::Vacant(entry) => { let function = Rc::new(self.load_function( @@ -381,10 +383,12 @@ impl InterpreterImpl<'_> { FrameTypeCache::make_rc_for_function(&function); entry.insert((Rc::clone(&function), Rc::clone(&frame_cache))); - current_frame_cache.per_instruction_cache - [current_frame.pc as usize] = PerInstructionCache::Call( - Rc::clone(&function), - Rc::clone(&frame_cache), + current_frame_cache.per_instruction_cache.insert( + current_frame.pc, + PerInstructionCache::Call( + Rc::clone(&function), + Rc::clone(&frame_cache), + ), ); (function, frame_cache) @@ -449,22 +453,23 @@ impl InterpreterImpl<'_> { let (function, frame_cache) = if RTCaches::caches_enabled() { let current_frame_cache = &mut *current_frame.frame_cache.borrow_mut(); - if let PerInstructionCache::CallGeneric(ref function, ref frame_cache) = - current_frame_cache.per_instruction_cache[current_frame.pc as usize] + if let Some(PerInstructionCache::CallGeneric(function, frame_cache)) = + current_frame_cache.per_instruction_cache.get(¤t_frame.pc) { (Rc::clone(function), Rc::clone(frame_cache)) } else { match current_frame_cache.generic_sub_frame_cache.entry(idx) { btree_map::Entry::Occupied(entry) => { - let entry = entry.get(); - current_frame_cache.per_instruction_cache - [current_frame.pc as usize] = + let (function, frame_cache) = entry.get(); + current_frame_cache.per_instruction_cache.insert( + current_frame.pc, PerInstructionCache::CallGeneric( - Rc::clone(&entry.0), - Rc::clone(&entry.1), - ); + Rc::clone(function), + Rc::clone(frame_cache), + ), + ); - (Rc::clone(&entry.0), Rc::clone(&entry.1)) + (Rc::clone(function), Rc::clone(frame_cache)) }, btree_map::Entry::Vacant(entry) => { let function = @@ -478,12 +483,13 @@ impl InterpreterImpl<'_> { FrameTypeCache::make_rc_for_function(&function); entry.insert((Rc::clone(&function), Rc::clone(&frame_cache))); - current_frame_cache.per_instruction_cache - [current_frame.pc as usize] = + current_frame_cache.per_instruction_cache.insert( + current_frame.pc, PerInstructionCache::CallGeneric( Rc::clone(&function), Rc::clone(&frame_cache), - ); + ), + ); (function, frame_cache) }, } @@ -2128,14 +2134,15 @@ impl Frame { }; let field_count = if RTCaches::caches_enabled() { - let cached_field_count = - &frame_cache.per_instruction_cache[self.pc as usize]; - if let PerInstructionCache::Pack(ref field_count) = cached_field_count { + if let Some(PerInstructionCache::Pack(field_count)) = + frame_cache.per_instruction_cache.get(&self.pc) + { *field_count } else { let field_count = get_field_count_charge_gas_and_check_depth()?; - frame_cache.per_instruction_cache[self.pc as usize] = - PerInstructionCache::Pack(field_count); + frame_cache + .per_instruction_cache + .insert(self.pc, PerInstructionCache::Pack(field_count)); field_count } } else { @@ -2189,18 +2196,16 @@ impl Frame { }; let field_count = if RTCaches::caches_enabled() { - let cached_field_count = - &frame_cache.per_instruction_cache[self.pc as usize]; - - if let PerInstructionCache::PackGeneric(ref field_count) = - cached_field_count + if let Some(PerInstructionCache::PackGeneric(field_count)) = + frame_cache.per_instruction_cache.get(&self.pc) { *field_count } else { let field_count = get_field_count_charge_gas_and_check_depth(frame_cache)?; - frame_cache.per_instruction_cache[self.pc as usize] = - PerInstructionCache::PackGeneric(field_count); + frame_cache + .per_instruction_cache + .insert(self.pc, PerInstructionCache::PackGeneric(field_count)); field_count } } else { diff --git a/third_party/move/move-vm/runtime/src/loader/function.rs b/third_party/move/move-vm/runtime/src/loader/function.rs index 7e3952f508c..000dfd2a6d6 100644 --- a/third_party/move/move-vm/runtime/src/loader/function.rs +++ b/third_party/move/move-vm/runtime/src/loader/function.rs @@ -432,10 +432,6 @@ impl LoadedFunction { &self.function.code } - pub(crate) fn code_size(&self) -> usize { - self.function.code.len() - } - pub(crate) fn access_specifier(&self) -> &AccessSpecifier { &self.function.access_specifier }