Reconfigure some of the manual inlining

third_party/move/move-vm/runtime/Cargo.toml

@@ -46,16 +46,19 @@ smallbitvec = { workspace = true }
 test-case = { workspace = true }
 
 [features]
-default = []
+default = [
+    "inline-callstack",
+]
+testing = []
 fuzzing = ["move-vm-types/fuzzing"]
 failpoints = ["fail/failpoints"]
 # Enable tracing and debugging also for release builds. By default, it is only enabled for debug builds.
 debugging = []
-testing = []
 stacktrace = []
 # Enable this features for tests that check bytecode verification.
 disable_verifier_cache = []
 force-inline = []
+inline-callstack = []
 
 [lints.rust]
 unexpected_cfgs = { level = "warn", check-cfg = ['cfg(fuzzing)'] }

third_party/move/move-vm/runtime/src/access_control.rs

@@ -20,6 +20,8 @@ pub struct AccessControlState {
 
 impl AccessControlState {
     /// Enters a function, applying its access specifier to the state.
+    // note(inline): do not inline, they are called once per function, and increase `execute_main`
+    // quite a bit, we want to avoid those compile times
     #[cfg_attr(feature = "force-inline", inline(always))]
     pub(crate) fn enter_function(
         &mut self,
@@ -47,6 +49,8 @@ impl AccessControlState {
     }
 
     /// Exit function, restoring access state before entering.
+    // note(inline): do not inline, they are called once per function, and increase `execute_main`
+    // quite a bit, we want to avoid those compile times
     #[cfg_attr(feature = "force-inline", inline(always))]
     pub(crate) fn exit_function(&mut self, fun: &LoadedFunction) -> PartialVMResult<()> {
         if !matches!(fun.access_specifier(), AccessSpecifier::Any) {

third_party/move/move-vm/runtime/src/frame_type_cache.rs

@@ -85,7 +85,11 @@ pub(crate) struct FrameTypeCache {
 }
 
 impl FrameTypeCache {
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline):
+    // needs to always be inlined, closure will be optimized out in this case. When it gets inlined,
+    // LLVM also inlines `BTreeMap::entry()` with it to optimize, so the final instruction count is pretty big -
+    // do not inline the dependent functions.
+    #[inline(always)]
     fn get_or<K: Copy + Ord + Eq, V, F>(
         map: &mut BTreeMap<K, V>,
         idx: K,
@@ -103,7 +107,7 @@ impl FrameTypeCache {
         }
     }
 
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline): do not inline, increases size a lot, might even decrease the performance
     pub(crate) fn get_field_type_and_struct_type(
         &mut self,
         idx: FieldInstantiationIndex,
@@ -120,6 +124,7 @@ impl FrameTypeCache {
         Ok(((field_ty, *field_ty_count), (struct_ty, *struct_ty_count)))
     }
 
+    // note(inline): do not inline, increases size a lot, might even decrease the performance
     pub(crate) fn get_variant_field_type_and_struct_type(
         &mut self,
         idx: VariantFieldInstantiationIndex,
@@ -141,7 +146,7 @@ impl FrameTypeCache {
         Ok(((field_ty, *field_ty_count), (struct_ty, *struct_ty_count)))
     }
 
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline): do not inline, increases size a lot, might even decrease the performance
     pub(crate) fn get_struct_type(
         &mut self,
         idx: StructDefInstantiationIndex,
@@ -155,7 +160,7 @@ impl FrameTypeCache {
         Ok((ty, *ty_count))
     }
 
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline): do not inline, increases size a lot, might even decrease the performance
     pub(crate) fn get_struct_variant_type(
         &mut self,
         idx: StructVariantInstantiationIndex,
@@ -174,7 +179,7 @@ impl FrameTypeCache {
         Ok((ty, *ty_count))
     }
 
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline): do not inline, increases size a lot, might even decrease the performance
     pub(crate) fn get_struct_fields_types(
         &mut self,
         idx: StructDefInstantiationIndex,
@@ -196,7 +201,7 @@ impl FrameTypeCache {
         )?)
     }
 
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline): do not inline, increases size a lot, might even decrease the performance
     pub(crate) fn get_struct_variant_fields_types(
         &mut self,
         idx: StructVariantInstantiationIndex,
@@ -218,7 +223,7 @@ impl FrameTypeCache {
         )?)
     }
 
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline): do not inline, increases size a lot, might even decrease the performance
     pub(crate) fn get_signature_index_type(
         &mut self,
         idx: SignatureIndex,

third_party/move/move-vm/runtime/src/interpreter.rs

@@ -900,7 +900,8 @@ where
     ///
     /// Native functions do not push a frame at the moment and as such errors from a native
     /// function are incorrectly attributed to the caller.
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline): single usage
+    #[inline(always)]
     fn make_call_frame<
         RTTCheck: RuntimeTypeCheck,
         RTRCheck: RuntimeRefCheck,
@@ -1789,7 +1790,7 @@ impl Stack {
 
     /// Push a `Value` on the stack if the max stack size has not been reached. Abort execution
     /// otherwise.
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline): increases function size 25%, DOES NOT improve performance, do not inline.
     fn push(&mut self, value: Value) -> PartialVMResult<()> {
         if self.value.len() < OPERAND_STACK_SIZE_LIMIT {
             self.value.push(value);
@@ -1800,7 +1801,7 @@ impl Stack {
     }
 
     /// Pop a `Value` off the stack or abort execution if the stack is empty.
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    #[inline]
     fn pop(&mut self) -> PartialVMResult<Value> {
         self.value
             .pop()
@@ -1809,8 +1810,8 @@ impl Stack {
 
     /// Pop a `Value` of a given type off the stack. Abort if the value is not of the given
     /// type or if the stack is empty.
-    // Note(inline): expensive to inline, adds a few seconds of compile time
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline): do not inline this, it bloats interpreter loop 20% and does not adds enough perf to justify,
+    // instead we're inlining `value_as()` and all VM casts.
     fn pop_as<T>(&mut self) -> PartialVMResult<T>
     where
         Value: VMValueCast<T>,
@@ -1819,7 +1820,6 @@ impl Stack {
     }
 
     /// Pop n values off the stack.
-    #[cfg_attr(feature = "force-inline", inline(always))]
     fn popn(&mut self, n: u16) -> PartialVMResult<Vec<Value>> {
         let remaining_stack_size = self
             .value
@@ -1830,7 +1830,6 @@ impl Stack {
         Ok(args)
     }
 
-    #[cfg_attr(feature = "force-inline", inline(always))]
     fn last_n(&self, n: usize) -> PartialVMResult<impl ExactSizeIterator<Item = &Value> + Clone> {
         if self.value.len() < n {
             return Err(
@@ -1845,7 +1844,7 @@ impl Stack {
 
     /// Push a type on the stack if the max stack size has not been reached. Abort execution
     /// otherwise.
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline): bloats runtime_type_checks
     pub(crate) fn push_ty(&mut self, ty: Type) -> PartialVMResult<()> {
         if self.types.len() < OPERAND_STACK_SIZE_LIMIT {
             self.types.push(ty);
@@ -1856,15 +1855,15 @@ impl Stack {
     }
 
     /// Pop a type off the stack or abort execution if the stack is empty.
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline): bloats runtime_type_checks
     pub(crate) fn pop_ty(&mut self) -> PartialVMResult<Type> {
         self.types.pop().ok_or_else(|| {
             PartialVMError::new(StatusCode::EMPTY_VALUE_STACK)
                 .with_message("runtime type stack empty")
         })
     }
 
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline): bloats runtime_type_checks
     pub(crate) fn top_ty(&mut self) -> PartialVMResult<&Type> {
         self.types.last().ok_or_else(|| {
             PartialVMError::new(StatusCode::EMPTY_VALUE_STACK)
@@ -1873,7 +1872,7 @@ impl Stack {
     }
 
     /// Pop n types off the stack.
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline): bloats runtime_type_checks
     pub(crate) fn popn_tys(&mut self, n: u16) -> PartialVMResult<Vec<Type>> {
         let remaining_stack_size = self.types.len().checked_sub(n as usize).ok_or_else(|| {
             PartialVMError::new(StatusCode::EMPTY_VALUE_STACK)
@@ -1930,7 +1929,7 @@ impl CallStack {
     }
 
     /// Push a `Frame` on the call stack.
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    #[cfg_attr(feature = "inline-callstack", inline(always))]
     fn push(&mut self, frame: Frame) -> Result<(), Frame> {
         if self.0.len() < CALL_STACK_SIZE_LIMIT {
             self.0.push(frame);
@@ -1941,7 +1940,7 @@ impl CallStack {
     }
 
     /// Pop a `Frame` off the call stack.
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    #[cfg_attr(feature = "inline-callstack", inline(always))]
     fn pop(&mut self) -> Option<Frame> {
         self.0.pop()
     }

third_party/move/move-vm/runtime/src/reentrancy_checker.rs

@@ -60,7 +60,8 @@ impl CallType {
 }
 
 impl ReentrancyChecker {
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline): as `call_type` is sometimes a fixed value, this inline is very valuable
+    #[inline(always)]
     pub fn enter_function(
         &mut self,
         caller_module: Option<&ModuleId>,
@@ -108,7 +109,7 @@ impl ReentrancyChecker {
         Ok(())
     }
 
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline): bloats code, not a hot path
     pub fn exit_function(
         &mut self,
         caller_module: &ModuleId,

third_party/move/move-vm/runtime/src/runtime_type_checks.rs

@@ -108,6 +108,7 @@ pub(crate) trait RuntimeTypeCheck {
     ) -> PartialVMResult<()>;
 }
 
+// note(inline): improves perf a little bit, but increases `post_execution_type_stack_transition` by 20%
 #[cfg_attr(feature = "force-inline", inline(always))]
 fn verify_pack<'a>(
     operand_stack: &mut Stack,
@@ -274,7 +275,8 @@ impl RuntimeTypeCheck for NoRuntimeTypeCheck {
 impl RuntimeTypeCheck for FullRuntimeTypeCheck {
     /// Note that most of the checks should happen after instruction execution, because gas charging will happen during
     /// instruction execution and we want to avoid running code without charging proper gas as much as possible.
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline): it should not be inlined, function calling overhead
+    // is not big enough to justify the increase in function size
     fn pre_execution_type_stack_transition(
         frame: &Frame,
         operand_stack: &mut Stack,
@@ -419,7 +421,8 @@ impl RuntimeTypeCheck for FullRuntimeTypeCheck {
     /// This function and `pre_execution_type_stack_transition` should
     /// constitute the full type stack transition for the paranoid
     /// mode.
-    #[cfg_attr(feature = "force-inline", inline(always))]
+    // note(inline): it should not be inlined, function calling overhead
+    // is not big enough to justify the increase in function size
     fn post_execution_type_stack_transition(
         frame: &Frame,
         operand_stack: &mut Stack,

third_party/move/move-vm/types/Cargo.toml

@@ -37,7 +37,12 @@ proptest = { workspace = true }
 rand = { workspace = true }
 
 [features]
-default = []
+default = [
+    "inline-vm-casts",
+    "inline-locals"
+]
 testing = []
 fuzzing = ["proptest", "move-binary-format/fuzzing"]
 force-inline = []
+inline-vm-casts = []
+inline-locals = []