Remove rr-validate feature given minimal performance improvements

Author: arjunr2

Cargo.toml

@@ -526,7 +526,6 @@ pulley = ["wasmtime-cli-flags/pulley"]
 stack-switching = ["wasmtime/stack-switching", "wasmtime-cli-flags/stack-switching"]
 rr = ["wasmtime/rr", "wasmtime-cli-flags/rr"]
 rr-component = ["rr", "wasmtime/rr-component"]
-rr-validate = ["rr", "wasmtime/rr-validate", "wasmtime-cli-flags/rr-validate"]
 
 # CLI subcommands for the `wasmtime` executable. See `wasmtime $cmd --help`
 # for more information on each subcommand.

crates/cli-flags/Cargo.toml

@@ -41,4 +41,3 @@ memory-protection-keys = ["wasmtime/memory-protection-keys"]
 pulley = ["wasmtime/pulley"]
 stack-switching = ["wasmtime/stack-switching"]
 rr = ["wasmtime/rr"]
-rr-validate = ["wasmtime/rr-validate"]

crates/cli-flags/src/lib.rs

@@ -1022,11 +1022,6 @@ impl CommonOptions {
         match_feature! {
             ["rr" : &record.path]
             _path => {
-                match_feature! {
-                    ["rr-validate": record.validation_metadata]
-                    _v => (),
-                    _ => err,
-                }
                 config.rr(wasmtime::RRConfig::Recording);
             },
             _ => err,

crates/wasmtime/Cargo.toml

@@ -410,7 +410,3 @@ rr-component = ["component-model", "rr"]
 # Enable support for the common base infrastructure of record/replay
 # By default, this supports core wasm `rr`. For components, enable `rr-component`
 rr = ["dep:embedded-io"]
-
-# Enables record/replay with support for additional validation signatures/checks.
-rr-validate = ["rr"]
-

crates/wasmtime/src/runtime/component/func/options.rs

@@ -11,7 +11,7 @@ use crate::rr::{ConstMemorySliceCell, MemorySliceCell};
 use crate::rr::{
     RREvent, RecordBuffer, ReplayError, Replayer, component_events::ReallocEntryEvent,
 };
-#[cfg(all(feature = "rr-component", feature = "rr-validate"))]
+#[cfg(feature = "rr-component")]
 use crate::rr::{ResultEvent, Validate, component_events::ReallocReturnEvent};
 use crate::runtime::vm::component::{
     CallContexts, ComponentInstance, InstanceFlags, ResourceTable, ResourceTables,
@@ -403,7 +403,7 @@ impl<'a, T: 'static> LowerContext<'a, T> {
             new_size,
         })?;
         let result = self.realloc_inner(old, old_size, old_align, new_size);
-        #[cfg(all(feature = "rr-component", feature = "rr-validate"))]
+        #[cfg(feature = "rr-component")]
         self.store.0.record_event_validation(|| {
             ReallocReturnEvent(ResultEvent::from_anyhow_result(&result))
         })?;
@@ -576,23 +576,19 @@ impl<'a, T: 'static> LowerContext<'a, T> {
         mut result_storage: Option<&mut [MaybeUninit<ValRaw>]>,
         phase: ReplayLoweringPhase,
     ) -> Result<()> {
-        // There is a lot of `rr-validate` feature gating here for optimal replay performance
-        // and memory overhead in a non-validating scenario. If this proves to not produce a huge
-        // overhead in practice, gating can be removed in the future in favor of readability
         if self.store.0.replay_buffer_mut().is_none() {
             return Ok(());
         }
         let mut complete = false;
         let mut lowering_error: Option<ReplayError> = None;
         // No nested expected; these depths should only be 1
-        let mut _realloc_stack = Vec::<Result<usize>>::new();
+        let mut realloc_stack = Vec::<Result<usize>>::new();
         // Lowering tracks is only for ordering entry/exit events
-        let mut _lower_stack = Vec::<()>::new();
-        let mut _lower_store_stack = Vec::<()>::new();
+        let mut lower_stack = Vec::<()>::new();
+        let mut lower_store_stack = Vec::<()>::new();
         while !complete {
             let buf = self.store.0.replay_buffer_mut().unwrap();
             let event = buf.next_event()?;
-            #[cfg(feature = "rr-validate")]
             let run_validate = buf.settings().validate && buf.trace_settings().add_validation;
             match event {
                 RREvent::HostFuncReturn(e) => {
@@ -624,27 +620,22 @@ impl<'a, T: 'static> LowerContext<'a, T> {
                     complete = true;
                 }
                 RREvent::ComponentReallocEntry(e) => {
-                    let _result =
+                    let result =
                         self.realloc_inner(e.old_addr, e.old_size, e.old_align, e.new_size);
-                    #[cfg(feature = "rr-validate")]
                     if run_validate {
-                        _realloc_stack.push(_result);
+                        realloc_stack.push(result);
                     }
                 }
                 // No return value to validate for lower/lower-store; store error and just check that entry happened before
                 RREvent::ComponentLowerFlatReturn(e) => {
-                    #[cfg(feature = "rr-validate")]
                     if run_validate {
-                        _lower_stack
-                            .pop()
-                            .ok_or(ReplayError::InvalidEventPosition)?;
+                        lower_stack.pop().ok_or(ReplayError::InvalidEventPosition)?;
                     }
                     lowering_error = e.0.ret().map_err(Into::into).err();
                 }
                 RREvent::ComponentLowerMemoryReturn(e) => {
-                    #[cfg(feature = "rr-validate")]
                     if run_validate {
-                        _lower_store_stack
+                        lower_store_stack
                             .pop()
                             .ok_or(ReplayError::InvalidEventPosition)?;
                     }
@@ -664,25 +655,21 @@ impl<'a, T: 'static> LowerContext<'a, T> {
                     bail!("Cannot call into host during lowering")
                 }
                 // Unwrapping should never occur on valid executions since *Entry should be before *Return in trace
-                RREvent::ComponentReallocReturn(_e) =>
-                {
-                    #[cfg(feature = "rr-validate")]
+                RREvent::ComponentReallocReturn(e) => {
                     if run_validate {
-                        lowering_error = _e.0.validate(&_realloc_stack.pop().unwrap()).err()
+                        lowering_error = e.0.validate(&realloc_stack.pop().unwrap()).err()
                     }
                 }
                 RREvent::ComponentLowerFlatEntry(_) => {
                     // All we want here is ensuring Entry occurs before Return
-                    #[cfg(feature = "rr-validate")]
                     if run_validate {
-                        _lower_stack.push(())
+                        lower_stack.push(())
                     }
                 }
                 RREvent::ComponentLowerMemoryEntry(_) => {
                     // All we want here is ensuring Entry occurs before Return
-                    #[cfg(feature = "rr-validate")]
                     if run_validate {
-                        _lower_store_stack.push(())
+                        lower_store_stack.push(())
                     }
                 }
 

crates/wasmtime/src/runtime/rr/core.rs

@@ -7,18 +7,14 @@ use wasmtime_environ::EntityIndex;
 // Use component events internally even without feature flags enabled
 // so that [`RREvent`] has a well-defined serialization format, but export
 // it for other modules only when enabled
-#[cfg(all(feature = "rr-validate", feature = "rr-component"))]
-pub use events::RRFuncArgVals;
-#[cfg(any(feature = "rr-validate", feature = "rr-component"))]
 pub use events::Validate;
 #[cfg(feature = "rr-component")]
 pub use events::component_events;
-pub use events::{ResultEvent, core_events, marker_events};
-use events::{common_events, component_events as __component_events};
+use events::component_events as __component_events;
+pub use events::{RRFuncArgVals, ResultEvent, common_events, core_events, marker_events};
 pub use io::{IOError, RecordWriter, ReplayReader};
 
 /// Settings for execution recording.
-#[cfg(feature = "rr")]
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct RecordSettings {
     /// Flag to include additional signatures for replay validation.
@@ -27,7 +23,6 @@ pub struct RecordSettings {
     pub event_window_size: usize,
 }
 
-#[cfg(feature = "rr")]
 impl Default for RecordSettings {
     fn default() -> Self {
         Self {
@@ -38,7 +33,6 @@ impl Default for RecordSettings {
 }
 
 /// Settings for execution replay.
-#[cfg(feature = "rr")]
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct ReplaySettings {
     /// Flag to include additional signatures for replay validation.
@@ -47,7 +41,6 @@ pub struct ReplaySettings {
     pub deser_buffer_size: usize,
 }
 
-#[cfg(feature = "rr")]
 impl Default for ReplaySettings {
     fn default() -> Self {
         Self {
@@ -304,7 +297,6 @@ pub trait Recorder {
 
     /// Record a event only when validation is requested
     #[inline]
-    #[cfg(feature = "rr-validate")]
     fn record_event_validation<T, F>(&mut self, f: F) -> Result<()>
     where
         T: Into<RREvent>,
@@ -379,7 +371,6 @@ pub trait Replayer: Iterator<Item = Result<RREvent, ReplayError>> {
     /// In addition to errors in [`next_event_typed`](Replayer::next_event_typed),
     /// validation errors can be thrown
     #[inline]
-    #[cfg(feature = "rr-validate")]
     fn next_event_validation<T, Y>(&mut self, expect: &Y) -> Result<(), ReplayError>
     where
         T: TryFrom<RREvent> + Validate<Y>,
@@ -568,11 +559,19 @@ impl Replayer for ReplayBuffer {
     }
 
     #[inline]
+    #[allow(
+        unused,
+        reason = "method only used for gated validation, but will be extended in the future"
+    )]
     fn settings(&self) -> &ReplaySettings {
         &self.settings
     }
 
     #[inline]
+    #[allow(
+        unused,
+        reason = "method only used for gated validation, but will be extended in the future"
+    )]
     fn trace_settings(&self) -> &RecordSettings {
         &self.trace_settings
     }

crates/wasmtime/src/runtime/rr/core/events.rs

@@ -1,4 +1,3 @@
-#[cfg(any(feature = "rr-component", feature = "rr-validate"))]
 use super::ReplayError;
 use crate::rr::FlatBytes;
 use crate::{AsContextMut, Val, prelude::*};
@@ -116,13 +115,11 @@ impl RRFuncArgVals {
 /// Trait signifying types that can be validated on replay
 ///
 /// All `PartialEq` types are directly validatable with themselves.
-/// Note however that some [`Validate`] implementations are present even
-/// when feature `rr-validate` is disabled, when validation is needed
-/// for a faithful replay (e.g. [`component_events::InstantiationEvent`]).
+/// Note however that some [`Validate`] implementations are present and
+/// required for a faithful replay (e.g. [`component_events::InstantiationEvent`]).
 ///
 /// In terms of usage, an event that implements `Validate` can call
 /// any RR validation methods on a `Store`
-#[cfg(any(feature = "rr-component", feature = "rr-validate"))]
 pub trait Validate<T: ?Sized> {
     /// Perform a validation of the event to ensure replay consistency
     fn validate(&self, expect: &T) -> Result<(), ReplayError>;
@@ -136,7 +133,6 @@ pub trait Validate<T: ?Sized> {
     }
 }
 
-#[cfg(any(feature = "rr-component", feature = "rr-validate"))]
 impl<T> Validate<T> for T
 where
     T: PartialEq + fmt::Debug,

crates/wasmtime/src/runtime/rr/core/events/component_events.rs

@@ -3,8 +3,6 @@
 use super::*;
 use crate::component::ComponentInstanceId;
 use crate::vm::component::libcalls::ResourceDropRet;
-// Re-export common events from this module
-pub use common_events::*;
 use wasmtime_environ::{
     self,
     component::{ExportIndex, InterfaceType},

crates/wasmtime/src/runtime/rr/core/events/core_events.rs

@@ -1,8 +1,6 @@
 //! Module comprising of core wasm events
 use super::*;
 use crate::{WasmFuncOrigin, store::InstanceId};
-// Re-export common events from this module
-pub use common_events::*;
 
 /// A core Wasm instantiatation event
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Ord, PartialOrd)]

crates/wasmtime/src/runtime/rr/hooks/component_hooks.rs

@@ -2,15 +2,15 @@ use crate::ValRaw;
 #[cfg(feature = "component-model")]
 use crate::component::func::LowerContext;
 #[cfg(feature = "rr-component")]
-use crate::rr::ResultEvent;
-#[cfg(all(feature = "rr-component", feature = "rr-validate"))]
+use crate::rr::common_events::WasmFuncReturnEvent;
+#[cfg(feature = "rr-component")]
 use crate::rr::component_events::HostFuncEntryEvent;
 #[cfg(feature = "rr-component")]
 use crate::rr::component_events::{
-    HostFuncReturnEvent, LowerFlatReturnEvent, LowerMemoryReturnEvent, WasmFuncEntryEvent,
+    LowerFlatReturnEvent, LowerMemoryReturnEvent, WasmFuncEntryEvent,
 };
-#[cfg(all(feature = "rr-component", feature = "rr-validate"))]
-use crate::rr::{RRFuncArgVals, component_events::WasmFuncReturnEvent};
+#[cfg(feature = "rr-component")]
+use crate::rr::{RRFuncArgVals, ResultEvent, common_events::HostFuncReturnEvent};
 use crate::store::StoreOpaque;
 use crate::{StoreContextMut, prelude::*};
 use alloc::sync::Arc;
@@ -52,7 +52,7 @@ where
         }
     })?;
     let result = wasm_call(store);
-    #[cfg(all(feature = "rr-component", feature = "rr-validate"))]
+    #[cfg(feature = "rr-component")]
     {
         let flat_results = types.flat_types_storage(
             &InterfaceType::Tuple(types[type_idx].results),
@@ -70,7 +70,7 @@ where
         result?;
         return Ok(());
     }
-    #[cfg(not(all(feature = "rr-component", feature = "rr-validate")))]
+    #[cfg(not(feature = "rr-component"))]
     return result;
 }
 
@@ -82,7 +82,7 @@ pub fn record_validate_host_func_entry(
     type_idx: &TypeFuncIndex,
     store: &mut StoreOpaque,
 ) -> Result<()> {
-    #[cfg(all(feature = "rr-component", feature = "rr-validate"))]
+    #[cfg(feature = "rr-component")]
     store.record_event_validation(|| create_host_func_entry_event(args, types, type_idx))?;
     let _ = (args, types, type_idx, store);
     Ok(())
@@ -96,7 +96,7 @@ pub fn replay_validate_host_func_entry(
     type_idx: &TypeFuncIndex,
     store: &mut StoreOpaque,
 ) -> Result<()> {
-    #[cfg(all(feature = "rr-component", feature = "rr-validate"))]
+    #[cfg(feature = "rr-component")]
     store.next_replay_event_validation::<HostFuncEntryEvent, _, _>(|| {
         create_host_func_entry_event(args, types, type_idx)
     })?;
@@ -134,7 +134,7 @@ pub fn record_lower_memory<F, T>(
 where
     F: FnOnce(&mut LowerContext<'_, T>, InterfaceType, usize) -> Result<()>,
 {
-    #[cfg(all(feature = "rr-component", feature = "rr-validate"))]
+    #[cfg(feature = "rr-component")]
     {
         use crate::rr::component_events::LowerMemoryEntryEvent;
         cx.store
@@ -159,7 +159,7 @@ pub fn record_lower_flat<F, T>(
 where
     F: FnOnce(&mut LowerContext<'_, T>, InterfaceType) -> Result<()>,
 {
-    #[cfg(all(feature = "rr-component", feature = "rr-validate"))]
+    #[cfg(feature = "rr-component")]
     {
         use crate::rr::component_events::LowerFlatEntryEvent;
         cx.store
@@ -174,7 +174,7 @@ where
     lower_result
 }
 
-#[cfg(all(feature = "rr-component", feature = "rr-validate"))]
+#[cfg(feature = "rr-component")]
 #[inline(always)]
 fn create_host_func_entry_event(
     args: &mut [MaybeUninit<ValRaw>],