addressing comments

aptos-move/aptos-gas-calibration/src/measurements.rs

@@ -172,7 +172,7 @@ fn compile_and_run_samples_ir(
                                 ExecFuncTimerDynamicArgs::NoArgs,
                                 GasMeterType::UnmeteredGasMeter,
                             )
-                            .elapsed_micros;
+                            .elapsed_micros();
                         gas_measurement.regular_meter.push(elapsed);
 
                         // record with abstract gas meter

aptos-move/aptos-gas-calibration/src/measurements_helpers.rs

@@ -104,7 +104,7 @@ pub fn execute_user_txn(
             ExecFuncTimerDynamicArgs::NoArgs,
             GasMeterType::UnmeteredGasMeter,
         )
-        .elapsed_micros;
+        .elapsed_micros();
     println!("running time (microseconds): {}", elapsed);
     elapsed
 }

aptos-move/aptos-vm-benchmarks/src/helper.rs

@@ -74,7 +74,7 @@ pub fn execute_user_txn(executor: &mut FakeExecutor, module_name: &ModuleId, fun
             ExecFuncTimerDynamicArgs::NoArgs,
             GasMeterType::UnmeteredGasMeter,
         )
-        .elapsed_micros;
+        .elapsed_micros();
     println!("running time (microseconds): {}", elapsed);
 }
 

aptos-move/e2e-benchmark/src/main.rs

@@ -3,7 +3,7 @@
 
 use aptos_language_e2e_tests::{
     account::Account,
-    executor::{ExecFuncTimerDynamicArgs, FakeExecutor, GasMeterType, TimeAndGas},
+    executor::{ExecFuncTimerDynamicArgs, FakeExecutor, GasMeterType, Measurement},
 };
 use aptos_transaction_generator_lib::{
     publishing::{
@@ -46,7 +46,7 @@ fn execute_and_time_entry_point(
     publisher_address: &AccountAddress,
     executor: &mut FakeExecutor,
     iterations: u64,
-) -> TimeAndGas {
+) -> Measurement {
     let mut rng = StdRng::seed_from_u64(14);
     let entry_fun = entry_point
         .create_payload(
@@ -77,9 +77,9 @@ fn execute_and_time_entry_point(
     )
 }
 
-const ALLOWED_REGRESSION: f32 = 0.15;
-const ALLOWED_IMPROVEMENT: f32 = 0.15;
-const ABSOLUTE_BUFFER_US: f32 = 2.0;
+const ALLOWED_REGRESSION: f64 = 0.15;
+const ALLOWED_IMPROVEMENT: f64 = 0.15;
+const ABSOLUTE_BUFFER_US: f64 = 2.0;
 
 const CALIBRATION_VALUES: &str = "
 Loop { loop_count: Some(100000), loop_type: NoOp }	6	0.988	1.039	41212.4
@@ -88,9 +88,9 @@ CreateObjects { num_objects: 10, object_payload_size: 0 }	6	0.940	1.026	152.1
 CreateObjects { num_objects: 10, object_payload_size: 10240 }	6	0.934	1.051	9731.3
 CreateObjects { num_objects: 100, object_payload_size: 0 }	6	0.966	1.051	1458.3
 CreateObjects { num_objects: 100, object_payload_size: 10240 }	6	0.969	1.077	11196.4
-InitializeVectorPicture { length: 128 }	6	0.973	1.066	75.0
-VectorPicture { length: 128 }	6	0.955	1.092	22.0
-VectorPictureRead { length: 128 }	6	0.952	1.047	21.0
+InitializeVectorPicture { length: 128 }	6	0.973	1.066	170.3
+VectorPicture { length: 128 }	6	0.955	1.092	46.2
+VectorPictureRead { length: 128 }	6	0.952	1.047	45.1
 InitializeVectorPicture { length: 30720 }	6	0.969	1.071	27295.8
 VectorPicture { length: 30720 }	6	0.957	1.066	6560.2
 VectorPictureRead { length: 30720 }	6	0.948	1.053	6642.8
@@ -103,16 +103,16 @@ TokenV1MintAndTransferNFTSequential	6	0.991	1.067	543.7
 TokenV2AmbassadorMint { numbered: true }	6	0.987	1.052	474.4
 LiquidityPoolSwap { is_stable: true }	6	0.970	1.042	555.4
 LiquidityPoolSwap { is_stable: false }	6	0.925	1.001	535.3
-CoinInitAndMint	6	0.925	1.001	146
-FungibleAssetMint	6	0.925	1.001	154
-IncGlobalMilestoneAggV2 { milestone_every: 1 }	6	0.925	1.001	23
-IncGlobalMilestoneAggV2 { milestone_every: 2 }	6	0.925	1.001	12
-EmitEvents { count: 1000 }	6	0.925	1.001	6871
+CoinInitAndMint	6	0.925	1.001	197.1
+FungibleAssetMint	6	0.925	1.001	231.6
+IncGlobalMilestoneAggV2 { milestone_every: 1 }	6	0.925	1.001	33.3
+IncGlobalMilestoneAggV2 { milestone_every: 2 }	6	0.925	1.001	19.1
+EmitEvents { count: 1000 }	6	0.925	1.001	8493.7
 ";
 
 struct CalibrationInfo {
     // count: usize,
-    expected_time: f32,
+    expected_time_micros: f64,
 }
 
 fn get_parsed_calibration_values() -> HashMap<String, CalibrationInfo> {
@@ -123,7 +123,7 @@ fn get_parsed_calibration_values() -> HashMap<String, CalibrationInfo> {
             let parts = line.split('\t').collect::<Vec<_>>();
             (parts[0].to_string(), CalibrationInfo {
                 // count: parts[1].parse().unwrap(),
-                expected_time: parts[parts.len() - 1].parse().unwrap(),
+                expected_time_micros: parts[parts.len() - 1].parse().unwrap(),
             })
         })
         .collect()
@@ -210,10 +210,10 @@ fn main() {
 
     for (index, entry_point) in entry_points.into_iter().enumerate() {
         let entry_point_name = format!("{:?}", entry_point);
-        let expected_time = calibration_values
+        let expected_time_micros = calibration_values
             .get(&entry_point_name)
             .expect(&entry_point_name)
-            .expected_time;
+            .expected_time_micros;
         let publisher = executor.new_account_at(AccountAddress::random());
 
         let mut package_handler = PackageHandler::new(entry_point.package_name());
@@ -244,52 +244,52 @@ fn main() {
             &package,
             publisher.address(),
             &mut executor,
-            if expected_time > 10000.0 {
+            if expected_time_micros > 10000.0 {
                 6
-            } else if expected_time > 1000.0 {
+            } else if expected_time_micros > 1000.0 {
                 10
             } else {
                 100
             },
         );
-        let diff = (measurement.elapsed_micros as f32 - expected_time as f32)
-            / (expected_time as f32)
-            * 100.0;
+        let elapsed_micros = measurement.elapsed_micros_f64();
+        let diff = (elapsed_micros - expected_time_micros) / expected_time_micros * 100.0;
+        let execution_gas_units = measurement.execution_gas_units();
+        let io_gas_units = measurement.io_gas_units();
+        let gps = (execution_gas_units + io_gas_units) / measurement.elapsed_secs_f64();
         println!(
-            "{:13} {:13.1} {:12.1}% {:13} {:13} {:13}  {:?}",
-            measurement.elapsed_micros,
-            expected_time,
+            "{:13.1} {:13.1} {:12.1}% {:13.0} {:13.2} {:13.2}  {:?}",
+            elapsed_micros,
+            expected_time_micros,
             diff,
-            (measurement.execution_gas + measurement.io_gas) as u128 / measurement.elapsed_micros,
-            measurement.execution_gas,
-            measurement.io_gas,
+            gps,
+            execution_gas_units,
+            io_gas_units,
             entry_point
         );
 
         json_lines.push(json!({
             "grep": "grep_json_aptos_move_vm_perf",
             "transaction_type": entry_point_name,
-            "wall_time_us": measurement.elapsed_micros,
-            "gps": (measurement.execution_gas + measurement.io_gas) as u128 / measurement.elapsed_micros,
-            "execution_gas": measurement.execution_gas,
-            "io_gas": measurement.io_gas,
-            "expected_wall_time_us": expected_time,
+            "wall_time_us": elapsed_micros,
+            "gas_units_per_second": gps,
+            "execution_gas_units": execution_gas_units,
+            "io_gas_units": io_gas_units,
+            "expected_wall_time_us": expected_time_micros,
             "test_index": index,
         }));
 
-        if measurement.elapsed_micros as f32
-            > expected_time as f32 * (1.0 + ALLOWED_REGRESSION) + ABSOLUTE_BUFFER_US
-        {
+        if elapsed_micros > expected_time_micros * (1.0 + ALLOWED_REGRESSION) + ABSOLUTE_BUFFER_US {
             failures.push(format!(
-                "Performance regression detected: {}us, expected: {}us, diff: {}%, for {:?}",
-                measurement.elapsed_micros, expected_time, diff, entry_point
+                "Performance regression detected: {:.1}us, expected: {:.1}us, diff: {}%, for {:?}",
+                elapsed_micros, expected_time_micros, diff, entry_point
             ));
-        } else if measurement.elapsed_micros as f32 + ABSOLUTE_BUFFER_US
-            < expected_time as f32 * (1.0 - ALLOWED_IMPROVEMENT)
+        } else if elapsed_micros + ABSOLUTE_BUFFER_US
+            < expected_time_micros * (1.0 - ALLOWED_IMPROVEMENT)
         {
             failures.push(format!(
-                "Performance improvement detected: {}us, expected {}us, diff: {}%, for {:?}. You need to adjust expected time!",
-                measurement.elapsed_micros, expected_time, diff, entry_point
+                "Performance improvement detected: {:.1}us, expected {:.1}us, diff: {}%, for {:?}. You need to adjust expected time!",
+                elapsed_micros, expected_time_micros, diff, entry_point
             ));
         }
     }

aptos-move/e2e-tests/src/executor.rs

@@ -89,7 +89,7 @@ use std::{
     path::{Path, PathBuf},
     str::FromStr,
     sync::{Arc, Mutex},
-    time::Instant,
+    time::{Duration, Instant},
 };
 
 static RNG_SEED: [u8; 32] = [9u8; 32];
@@ -145,10 +145,36 @@ pub enum GasMeterType {
 }
 
 #[derive(Clone)]
-pub struct TimeAndGas {
-    pub elapsed_micros: u128,
-    pub execution_gas: u64,
-    pub io_gas: u64,
+pub struct Measurement {
+    elapsed: Duration,
+    /// In internal gas units
+    execution_gas: u64,
+    /// In internal gas units
+    io_gas: u64,
+}
+
+const GAS_SCALING_FACTOR: f64 = 1_000_000.0;
+
+impl Measurement {
+    pub fn elapsed_micros(&self) -> u128 {
+        self.elapsed.as_micros()
+    }
+
+    pub fn elapsed_secs_f64(&self) -> f64 {
+        self.elapsed.as_secs_f64()
+    }
+
+    pub fn elapsed_micros_f64(&self) -> f64 {
+        self.elapsed.as_secs_f64() * 1_000_000.0
+    }
+
+    pub fn execution_gas_units(&self) -> f64 {
+        self.execution_gas as f64 / GAS_SCALING_FACTOR
+    }
+
+    pub fn io_gas_units(&self) -> f64 {
+        self.io_gas as f64 / GAS_SCALING_FACTOR
+    }
 }
 
 pub enum ExecFuncTimerDynamicArgs {
@@ -980,7 +1006,7 @@ impl FakeExecutor {
         iterations: u64,
         dynamic_args: ExecFuncTimerDynamicArgs,
         gas_meter_type: GasMeterType,
-    ) -> TimeAndGas {
+    ) -> Measurement {
         let mut extra_accounts = match &dynamic_args {
             ExecFuncTimerDynamicArgs::DistinctSigners
             | ExecFuncTimerDynamicArgs::DistinctSignersAndFixed(_) => (0..iterations)
@@ -1000,7 +1026,7 @@ impl FakeExecutor {
 
         // start measuring here to reduce measurement errors (i.e., the time taken to load vm, module, etc.)
         let mut i = 0;
-        let mut times = Vec::new();
+        let mut measurements = Vec::new();
         while i < iterations {
             let mut session = vm.new_session(&resolver, SessionId::void(), None);
 
@@ -1075,8 +1101,8 @@ impl FakeExecutor {
                     );
                 }
             }
-            times.push(TimeAndGas {
-                elapsed_micros: elapsed.as_micros(),
+            measurements.push(Measurement {
+                elapsed,
                 execution_gas: regular
                     .as_ref()
                     .map_or(0, |gas| gas.algebra().execution_gas_used().into()),
@@ -1088,20 +1114,22 @@ impl FakeExecutor {
         }
 
         // take median of all running time iterations as a more robust measurement
-        times.sort_by_key(|v| v.elapsed_micros);
-        let length = times.len();
+        measurements.sort_by_key(|v| v.elapsed);
+        let length = measurements.len();
         let mid = length / 2;
-        let mut running_time = times[mid].clone();
+        let mut measurement = measurements[mid].clone();
 
         if length % 2 == 0 {
-            running_time = TimeAndGas {
-                elapsed_micros: (times[mid - 1].elapsed_micros + times[mid].elapsed_micros) / 2,
-                execution_gas: (times[mid - 1].execution_gas + times[mid].execution_gas) / 2,
-                io_gas: (times[mid - 1].io_gas + times[mid].io_gas) / 2,
+            measurement = Measurement {
+                elapsed: (measurements[mid - 1].elapsed + measurements[mid].elapsed) / 2,
+                execution_gas: (measurements[mid - 1].execution_gas
+                    + measurements[mid].execution_gas)
+                    / 2,
+                io_gas: (measurements[mid - 1].io_gas + measurements[mid].io_gas) / 2,
             };
         }
 
-        running_time
+        measurement
     }
 
     /// record abstract usage using a modified gas meter

execution/executor-benchmark/src/lib.rs

@@ -43,7 +43,7 @@ use aptos_transaction_generator_lib::{
     create_txn_generator_creator, AlwaysApproveRootAccountHandle, TransactionGeneratorCreator,
     TransactionType::{self, CoinTransfer},
 };
-use aptos_types::on_chain_config::Features;
+use aptos_types::on_chain_config::{FeatureFlag, Features};
 use aptos_vm::VMBlockExecutor;
 use db_reliable_submitter::DbReliableTransactionSubmitter;
 use metrics::TIMER;
@@ -57,6 +57,12 @@ use std::{
 };
 use tokio::runtime::Runtime;
 
+pub fn default_benchmark_features() -> Features {
+    let mut init_features = Features::default();
+    init_features.disable(FeatureFlag::REMOVE_DETAILED_ERROR_FROM_HASH);
+    init_features
+}
+
 pub fn init_db_and_executor<V>(config: &NodeConfig) -> (DbReaderWriter, BlockExecutor<V>)
 where
     V: VMBlockExecutor,
@@ -778,7 +784,7 @@ fn log_total_supply(db_reader: &Arc<dyn DbReader>) {
 #[cfg(test)]
 mod tests {
     use crate::{
-        db_generator::bootstrap_with_genesis, init_db_and_executor,
+        db_generator::bootstrap_with_genesis, default_benchmark_features, init_db_and_executor,
         native::native_config::NativeConfig, pipeline::PipelineConfig,
         transaction_executor::BENCHMARKS_BLOCK_EXECUTOR_ONCHAIN_CONFIG,
         transaction_generator::TransactionGenerator, BenchmarkWorkload,
@@ -789,10 +795,7 @@ mod tests {
     use aptos_sdk::{transaction_builder::aptos_stdlib, types::LocalAccount};
     use aptos_temppath::TempPath;
     use aptos_transaction_generator_lib::{args::TransactionTypeArg, WorkflowProgress};
-    use aptos_types::{
-        on_chain_config::{FeatureFlag, Features},
-        transaction::Transaction,
-    };
+    use aptos_types::{on_chain_config::FeatureFlag, transaction::Transaction};
     use aptos_vm::{aptos_vm::AptosVMBlockExecutor, AptosVM, VMBlockExecutor};
     use rand::thread_rng;
     use std::fs;
@@ -805,7 +808,7 @@ mod tests {
 
         fs::create_dir_all(db_dir.as_ref()).unwrap();
 
-        let mut init_features = Features::default();
+        let mut init_features = default_benchmark_features();
         init_features.enable(FeatureFlag::NEW_ACCOUNTS_DEFAULT_TO_FA_APT_STORE);
         init_features.enable(FeatureFlag::OPERATIONS_DEFAULT_TO_FA_APT_STORE);
 
@@ -898,7 +901,7 @@ mod tests {
 
         println!("db_generator::create_db_with_accounts");
 
-        let mut features = Features::default();
+        let mut features = default_benchmark_features();
         features.enable(FeatureFlag::NEW_ACCOUNTS_DEFAULT_TO_FA_APT_STORE);
         features.enable(FeatureFlag::OPERATIONS_DEFAULT_TO_FA_APT_STORE);
 

execution/executor-benchmark/src/main.rs

@@ -14,8 +14,8 @@ use aptos_config::config::{
     EpochSnapshotPrunerConfig, LedgerPrunerConfig, PrunerConfig, StateMerklePrunerConfig,
 };
 use aptos_executor_benchmark::{
-    native::native_config::NativeConfig, native_executor::NativeExecutor, pipeline::PipelineConfig,
-    BenchmarkWorkload,
+    default_benchmark_features, native::native_config::NativeConfig,
+    native_executor::NativeExecutor, pipeline::PipelineConfig, BenchmarkWorkload,
 };
 use aptos_executor_service::remote_executor_client;
 use aptos_experimental_ptx_executor::PtxBlockExecutor;
@@ -413,7 +413,7 @@ fn get_init_features(
         "Enable and disable feature flags cannot overlap."
     );
 
-    let mut init_features = Features::default();
+    let mut init_features = default_benchmark_features();
     for feature in enable_feature.iter() {
         init_features.enable(*feature);
     }