move resolution of profile output path further up

lz1998 · Jan 24, 2024 · a0f72b0 · a0f72b0
1 parent 609567f
commit a0f72b0
Show file tree

Hide file tree

Showing 14 changed files with 132 additions and 233 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/sui-replay/src/lib.rs b/crates/sui-replay/src/lib.rs
@@ -7,7 +7,6 @@ use config::ReplayableNetworkConfigSet;
 use fuzz::ReplayFuzzer;
 use fuzz::ReplayFuzzerConfig;
 use fuzz_mutations::base_fuzzers;
-use move_vm_config::runtime::DEFAULT_PROFILE_OUTPUT_PATH;
 use sui_types::digests::get_mainnet_chain_identifier;
 use sui_types::digests::get_testnet_chain_identifier;
 use sui_types::message_envelope::Message;
@@ -377,6 +376,14 @@ pub async fn execute_replay_command(
             protocol_version,
             profile_output,
         } => {
+            let now = std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .expect("Error getting system time")
+                .as_nanos();
+            let mut default_name = std::path::PathBuf::from(".");
+            default_name.push(format!("gas_profile_{}_{}.json", tx_digest, now));
+            let output_path = profile_output.or(Some(default_name));
+
             let tx_digest = TransactionDigest::from_str(&tx_digest)?;
             info!("Executing tx: {}", tx_digest);
             let sandbox_state = LocalExec::replay_with_network_config(
@@ -387,7 +394,7 @@ pub async fn execute_replay_command(
                 use_authority,
                 executor_version,
                 protocol_version,
-                profile_output.or(Some((*DEFAULT_PROFILE_OUTPUT_PATH.clone()).to_path_buf())),
+                output_path,
             )
             .await?;
 

diff --git a/crates/sui-replay/src/replay.rs b/crates/sui-replay/src/replay.rs
@@ -238,7 +238,7 @@ pub struct LocalExec {
     // None implies use the protocol version at the time of execution
     pub protocol_version: Option<i64>,
     // Whether or not to enable the gas profiler, the PathBuf contains either a user specified
-    // filepath or DEFAULT_PROFILE_OUTPUT_PATH for the profile output
+    // filepath or the default current directory and name format for the profile output
     pub enable_profiler: Option<PathBuf>,
     // Retry policies due to RPC errors
     pub num_retries_for_timeout: u32,

diff --git a/external-crates/move/crates/move-vm-config/src/runtime.rs b/external-crates/move/crates/move-vm-config/src/runtime.rs
@@ -4,7 +4,6 @@
 use crate::verifier::{VerifierConfig, DEFAULT_MAX_CONSTANT_VECTOR_LEN};
 use move_binary_format::file_format_common::VERSION_MAX;
 use once_cell::sync::Lazy;
-use std::path::PathBuf;
 
 #[cfg(feature = "gas-profiler")]
 const MOVE_VM_PROFILER_ENV_VAR_NAME: &str = "MOVE_VM_PROFILE";
@@ -15,8 +14,6 @@ static PROFILER_ENABLED: Lazy<bool> =
 
 pub const DEFAULT_MAX_VALUE_NEST_DEPTH: u64 = 128;
 
-pub static DEFAULT_PROFILE_OUTPUT_PATH: Lazy<PathBuf> = Lazy::new(|| std::path::PathBuf::from("."));
-
 /// Dynamic config options for the Move VM.
 pub struct VMConfig {
     pub verifier: VerifierConfig,
@@ -71,7 +68,7 @@ impl Default for VMRuntimeLimitsConfig {
 #[derive(Clone, Debug, Default)]
 pub struct VMProfilerConfig {
     /// User configured full path override
-    pub full_path: Option<std::path::PathBuf>,
+    pub full_path: std::path::PathBuf,
     /// Whether or not to track bytecode instructions
     pub track_bytecode_instructions: bool,
     /// Whether or not to use the long name for functions

diff --git a/external-crates/move/crates/move-vm-profiler/Cargo.toml b/external-crates/move/crates/move-vm-profiler/Cargo.toml
@@ -12,6 +12,7 @@ serde_json.workspace = true
 once_cell.workspace = true
 
 move-vm-config = { workspace = true, features = ["gas-profiler"] }
+log = "0.4.20"
 
 [features]
 gas-profiler = ["move-vm-config/gas-profiler"]
diff --git a/external-crates/move/crates/move-vm-profiler/src/lib.rs b/external-crates/move/crates/move-vm-profiler/src/lib.rs
@@ -1,8 +1,6 @@
 // Copyright (c) Mysten Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 use move_vm_config::runtime::VMProfilerConfig;
-#[cfg(feature = "gas-profiler")]
-use move_vm_config::runtime::DEFAULT_PROFILE_OUTPUT_PATH;
 use serde::Serialize;
 use std::collections::BTreeMap;
 
@@ -107,10 +105,6 @@ impl GasProfiler {
         )
     }
 
-    fn profile_name(&self) -> String {
-        self.name.clone()
-    }
-
     pub fn short_name(s: &String) -> String {
         s.split("::").last().unwrap_or(s).to_string()
     }
@@ -184,19 +178,8 @@ impl GasProfiler {
             return;
         }
 
-        let mut p = (*DEFAULT_PROFILE_OUTPUT_PATH.clone()).to_path_buf();
-        if let Some(f) = &config.full_path {
-            p = f.clone();
-        } else {
-            // Get the unix timestamp
-            let now = std::time::SystemTime::now()
-                .duration_since(std::time::UNIX_EPOCH)
-                .expect("Clock may have gone backwards")
-                .as_nanos();
-            p.push(format!("gas_profile_{}_{}.json", self.profile_name(), now));
-        }
+        let p = &config.full_path.clone();
         let path_str = p.as_os_str().to_string_lossy().to_string();
-
         let mut file = std::fs::File::create(p).expect("Unable to create file");
 
         let json = serde_json::to_string_pretty(&self).expect("Unable to serialize profile");

diff --git a/external-crates/move/crates/move-vm-runtime/src/interpreter.rs b/external-crates/move/crates/move-vm-runtime/src/interpreter.rs
@@ -215,9 +215,8 @@ impl Interpreter {
                 ExitCode::Call(fh_idx) => {
                     let func = resolver.function_from_handle(fh_idx);
                     // Compiled out in release mode
-                    #[cfg(feature = "gas-profiler")]
-                    let func_name = func.pretty_string();
-                    profile_open_frame!(gas_meter, func_name.clone());
+
+                    profile_open_frame!(gas_meter, func.pretty_string().clone());
 
                     // Charge gas
                     let module_id = func.module_id();
@@ -233,10 +232,11 @@ impl Interpreter {
                         .map_err(|e| set_err_info!(current_frame, e))?;
 
                     if func.is_native() {
-                        self.call_native(&resolver, gas_meter, extensions, func, vec![])?;
+                        self.call_native(&resolver, gas_meter, extensions, func.clone(), vec![])?;
+
                         current_frame.pc += 1; // advance past the Call instruction in the caller
 
-                        profile_close_frame!(gas_meter, func_name.clone());
+                        profile_close_frame!(gas_meter, func.pretty_string().clone());
                         continue;
                     }
                     let frame = self
@@ -257,10 +257,8 @@ impl Interpreter {
                         .instantiate_generic_function(idx, current_frame.ty_args())
                         .map_err(|e| set_err_info!(current_frame, e))?;
                     let func = resolver.function_from_instantiation(idx);
-                    // Compiled out in release mode
-                    #[cfg(feature = "gas-profiler")]
-                    let func_name = func.pretty_string();
-                    profile_open_frame!(gas_meter, func_name.clone());
+
+                    profile_open_frame!(gas_meter, func.pretty_string().clone());
 
                     // Charge gas
                     let module_id = func.module_id();
@@ -277,10 +275,10 @@ impl Interpreter {
                         .map_err(|e| set_err_info!(current_frame, e))?;
 
                     if func.is_native() {
-                        self.call_native(&resolver, gas_meter, extensions, func, ty_args)?;
+                        self.call_native(&resolver, gas_meter, extensions, func.clone(), ty_args)?;
                         current_frame.pc += 1; // advance past the Call instruction in the caller
 
-                        profile_close_frame!(gas_meter, func_name.clone());
+                        profile_close_frame!(gas_meter, func.pretty_string().clone());
 
                         continue;
                     }

diff --git a/external-crates/move/move-execution/next-vm/crates/move-vm-runtime/src/interpreter.rs b/external-crates/move/move-execution/next-vm/crates/move-vm-runtime/src/interpreter.rs
@@ -29,8 +29,8 @@ use move_vm_types::{
     gas::{GasMeter, SimpleInstruction},
     loaded_data::runtime_types::Type,
     values::{
-        self, IntegerValue, Locals, Reference, Struct, StructRef, VMValueCast, Value,
-        Vector, VectorRef,
+        self, IntegerValue, Locals, Reference, Struct, StructRef, VMValueCast, Value, Vector,
+        VectorRef,
     },
     views::TypeView,
 };
@@ -187,10 +187,9 @@ impl Interpreter {
             .map_err(|err| self.set_location(err))?;
         loop {
             let resolver = current_frame.resolver(link_context, loader);
-            let exit_code =
-                current_frame //self
-                    .execute_code(&resolver, &mut self, gas_meter)
-                    .map_err(|err| self.maybe_core_dump(err, &current_frame))?;
+            let exit_code = current_frame //self
+                .execute_code(&resolver, &mut self, gas_meter)
+                .map_err(|err| self.maybe_core_dump(err, &current_frame))?;
             match exit_code {
                 ExitCode::Return => {
                     let non_ref_vals = current_frame
@@ -217,8 +216,7 @@ impl Interpreter {
                     let func = resolver.function_from_handle(fh_idx);
                     // Compiled out in release mode
 
-                    let _func_name = func.pretty_string();
-                    profile_open_frame!(gas_meter, _func_name.clone());
+                    profile_open_frame!(gas_meter, func.pretty_string().clone());
 
                     // Charge gas
                     let module_id = func.module_id();
@@ -234,16 +232,10 @@ impl Interpreter {
                         .map_err(|e| set_err_info!(current_frame, e))?;
 
                     if func.is_native() {
-                        self.call_native(
-                            &resolver,
-                            gas_meter,
-                            extensions,
-                            func,
-                            vec![],
-                        )?;
+                        self.call_native(&resolver, gas_meter, extensions, func.clone(), vec![])?;
                         current_frame.pc += 1; // advance past the Call instruction in the caller
 
-                        profile_close_frame!(gas_meter, _func_name.clone());
+                        profile_close_frame!(gas_meter, func.pretty_string().clone());
                         continue;
                     }
                     let frame = self
@@ -266,8 +258,7 @@ impl Interpreter {
                     let func = resolver.function_from_instantiation(idx);
                     // Compiled out in release mode
 
-                    let _func_name = func.pretty_string();
-                    profile_open_frame!(gas_meter, _func_name.clone());
+                    profile_open_frame!(gas_meter, func.pretty_string().clone());
 
                     // Charge gas
                     let module_id = func.module_id();
@@ -284,10 +275,10 @@ impl Interpreter {
                         .map_err(|e| set_err_info!(current_frame, e))?;
 
                     if func.is_native() {
-                        self.call_native(&resolver, gas_meter, extensions, func, ty_args)?;
+                        self.call_native(&resolver, gas_meter, extensions, func.clone(), ty_args)?;
                         current_frame.pc += 1; // advance past the Call instruction in the caller
 
-                        profile_close_frame!(gas_meter, _func_name.clone());
+                        profile_close_frame!(gas_meter, func.pretty_string().clone());
 
                         continue;
                     }
@@ -358,23 +349,17 @@ impl Interpreter {
         ty_args: Vec<Type>,
     ) -> VMResult<()> {
         // Note: refactor if native functions push a frame on the stack
-        self.call_native_impl(
-            resolver,
-            gas_meter,
-            extensions,
-            function.clone(),
-            ty_args,
-        )
-        .map_err(|e| {
-            let id = function.module_id();
-            let e = if resolver.loader().vm_config().error_execution_state {
-                e.with_exec_state(self.get_internal_state())
-            } else {
-                e
-            };
-            e.at_code_offset(function.index(), 0)
-                .finish(Location::Module(id.clone()))
-        })
+        self.call_native_impl(resolver, gas_meter, extensions, function.clone(), ty_args)
+            .map_err(|e| {
+                let id = function.module_id();
+                let e = if resolver.loader().vm_config().error_execution_state {
+                    e.with_exec_state(self.get_internal_state())
+                } else {
+                    e
+                };
+                e.at_code_offset(function.index(), 0)
+                    .finish(Location::Module(id.clone()))
+            })
     }
 
     fn call_native_impl(
@@ -417,12 +402,8 @@ impl Interpreter {
             args.push_front(self.operand_stack.pop()?);
         }
 
-        let mut native_context = NativeContext::new(
-            self,
-            resolver,
-            extensions,
-            gas_meter.remaining_gas(),
-        );
+        let mut native_context =
+            NativeContext::new(self, resolver, extensions, gas_meter.remaining_gas());
         let native_function = function.get_native()?;
 
         gas_meter.charge_native_function_before_execution(