Skip to content

Commit

Permalink
review revisions
Browse files Browse the repository at this point in the history
  • Loading branch information
@lanvidr
lanvidr committed Jan 24, 2024
1 parent 1eb9677 commit 817479f
Show file tree
Hide file tree
Showing 14 changed files with 66 additions and 37 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 1 addition & 3 deletions crates/sui-node/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,7 @@ fn main() {
// ProtocolConfig::poison_get_for_min_version();

move_vm_profiler::gas_profiler_feature! {
if true {
panic!("Cannot run the sui-node binary with gas-profiler feature enabled");
}
panic!("Cannot run the sui-node binary with gas-profiler feature enabled");
}

let args = Args::parse();
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-replay/src/replay.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ impl LocalExec {
use_authority,
executor_version,
protocol_version,
enable_profiler.clone(),
enable_profiler,
)
.await
{
Expand Down
1 change: 1 addition & 0 deletions crates/sui/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ move-core-types.workspace = true
move-package.workspace = true
csv.workspace = true
workspace-hack.workspace = true
move-vm-profiler.workspace = true

[target.'cfg(not(target_env = "msvc"))'.dependencies]
jemalloc-ctl.workspace = true
Expand Down
9 changes: 3 additions & 6 deletions crates/sui/src/client_commands.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::{
sync::Arc,
};

use anyhow::{anyhow, ensure};
use anyhow::{anyhow, bail, ensure};
use bip32::DerivationPath;
use clap::*;
use colored::Colorize;
Expand Down Expand Up @@ -720,11 +720,8 @@ impl SuiClientCommands {
tx_digest,
profile_output,
} => {
#[cfg(not(feature = "gas-profiler"))]
{
if true {
return Err(anyhow!("gas-profiler feature is not enabled, rebuild or reinstall with --features gas-profiler"));
}
if !cfg!(feature = "gas-profiler") {
bail!("gas-profiler feature is not enabled, rebuild or reinstall with --features gas-profiler");
}

let cmd = ReplayToolCommand::ProfileTransaction {
Expand Down
13 changes: 13 additions & 0 deletions crates/sui/src/unit_tests/profiler_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,19 @@ use sui_replay::ReplayToolCommand;
///
/// Note this crate will always have the feature enabled in testing due to the addition of
/// `sui = { path = ".", features = ["gas-profiler"] }` to our dev-dependencies.
#[cfg(feature = "gas-profiler")]
#[test]
fn test_macro_shows_feature_enabled() {
let mut called = false;
assert!(!called);
move_vm_profiler::gas_profiler_feature! {
called = true;
}
assert!(called);
}

#[ignore]
#[cfg(feature = "gas-profiler")]
#[tokio::test(flavor = "multi_thread")]
async fn test_profiler() {
Expand Down
3 changes: 3 additions & 0 deletions external-crates/move/crates/move-vm-profiler/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,10 @@ macro_rules! profile_dump_file {
#[macro_export]
macro_rules! gas_profiler_feature {
($($tt:tt)*) => {
#[cfg(feature = "gas-profiler")]
{
$($tt)*
}
};
}

Expand Down
17 changes: 10 additions & 7 deletions external-crates/move/crates/move-vm-runtime/src/interpreter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use move_core_types::{
use move_vm_config::runtime::VMRuntimeLimitsConfig;
#[cfg(feature = "gas-profiler")]
use move_vm_profiler::GasProfiler;
#[cfg(feature = "gas-profiler")]
use move_vm_profiler::{
profile_close_frame, profile_close_instr, profile_open_frame, profile_open_instr,
};
Expand Down Expand Up @@ -116,7 +117,7 @@ impl Interpreter {
call_stack: CallStack::new(),
runtime_limits_config: loader.vm_config().runtime_limits_config.clone(),
};

#[cfg(feature = "gas-profiler")]
profile_open_frame!(gas_meter, function.pretty_string());

if function.is_native() {
Expand All @@ -141,7 +142,7 @@ impl Interpreter {
e.at_code_offset(function.index(), 0)
.finish(Location::Module(function.module_id().clone()))
})?;

#[cfg(feature = "gas-profiler")]
profile_close_frame!(gas_meter, function.pretty_string());

Ok(return_values.into_iter().collect())
Expand Down Expand Up @@ -201,7 +202,7 @@ impl Interpreter {
gas_meter
.charge_drop_frame(non_ref_vals.into_iter())
.map_err(|e| self.set_location(e))?;

#[cfg(feature = "gas-profiler")]
profile_close_frame!(gas_meter, current_frame.function.pretty_string());
if let Some(frame) = self.call_stack.pop() {
// Note: the caller will find the callee's return values at the top of the shared operand stack
Expand All @@ -216,6 +217,7 @@ impl Interpreter {
let func = resolver.function_from_handle(fh_idx);
#[cfg(feature = "gas-profiler")]
let _func_name = func.pretty_string();
#[cfg(feature = "gas-profiler")]
profile_open_frame!(gas_meter, _func_name.clone());

// Charge gas
Expand All @@ -235,7 +237,7 @@ impl Interpreter {
self.call_native(&resolver, gas_meter, extensions, func.clone(), vec![])?;

current_frame.pc += 1; // advance past the Call instruction in the caller

#[cfg(feature = "gas-profiler")]
profile_close_frame!(gas_meter, _func_name.clone());
continue;
}
Expand All @@ -260,6 +262,7 @@ impl Interpreter {

#[cfg(debug_assertions)]
let _func_name = func.pretty_string();
#[cfg(feature = "gas-profiler")]
profile_open_frame!(gas_meter, _func_name.clone());

// Charge gas
Expand All @@ -279,7 +282,7 @@ impl Interpreter {
if func.is_native() {
self.call_native(&resolver, gas_meter, extensions, func.clone(), ty_args)?;
current_frame.pc += 1; // advance past the Call instruction in the caller

#[cfg(feature = "gas-profiler")]
profile_close_frame!(gas_meter, _func_name.clone());

continue;
Expand Down Expand Up @@ -1303,7 +1306,7 @@ impl Frame {
),
)
});

#[cfg(feature = "gas-profiler")]
profile_open_instr!(gas_meter, format!("{:?}", instruction));

let r = Self::execute_instruction(
Expand All @@ -1316,7 +1319,7 @@ impl Frame {
gas_meter,
instruction,
)?;

#[cfg(feature = "gas-profiler")]
profile_close_instr!(gas_meter, format!("{:?}", instruction));

match r {
Expand Down
17 changes: 10 additions & 7 deletions external-crates/move/move-execution/next-vm/crates/move-vm-runtime/src/interpreter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use move_core_types::{
use move_vm_config::runtime::VMRuntimeLimitsConfig;
#[cfg(feature = "gas-profiler")]
use move_vm_profiler::GasProfiler;
#[cfg(feature = "gas-profiler")]
use move_vm_profiler::{
profile_close_frame, profile_close_instr, profile_open_frame, profile_open_instr,
};
Expand Down Expand Up @@ -116,7 +117,7 @@ impl Interpreter {
call_stack: CallStack::new(),
runtime_limits_config: loader.vm_config().runtime_limits_config.clone(),
};

#[cfg(feature = "gas-profiler")]
profile_open_frame!(gas_meter, function.pretty_string());

if function.is_native() {
Expand All @@ -141,7 +142,7 @@ impl Interpreter {
e.at_code_offset(function.index(), 0)
.finish(Location::Module(function.module_id().clone()))
})?;

#[cfg(feature = "gas-profiler")]
profile_close_frame!(gas_meter, function.pretty_string());

Ok(return_values.into_iter().collect())
Expand Down Expand Up @@ -201,7 +202,7 @@ impl Interpreter {
gas_meter
.charge_drop_frame(non_ref_vals.into_iter())
.map_err(|e| self.set_location(e))?;

#[cfg(feature = "gas-profiler")]
profile_close_frame!(gas_meter, current_frame.function.pretty_string());
if let Some(frame) = self.call_stack.pop() {
// Note: the caller will find the callee's return values at the top of the shared operand stack
Expand All @@ -216,6 +217,7 @@ impl Interpreter {
let func = resolver.function_from_handle(fh_idx);
#[cfg(feature = "gas-profiler")]
let func_name = func.pretty_string();
#[cfg(feature = "gas-profiler")]
profile_open_frame!(gas_meter, func_name.clone());

// Charge gas
Expand All @@ -234,7 +236,7 @@ impl Interpreter {
if func.is_native() {
self.call_native(&resolver, gas_meter, extensions, func.clone(), vec![])?;
current_frame.pc += 1; // advance past the Call instruction in the caller

#[cfg(feature = "gas-profiler")]
profile_close_frame!(gas_meter, func_name.clone());
continue;
}
Expand All @@ -258,6 +260,7 @@ impl Interpreter {
let func = resolver.function_from_instantiation(idx);
#[cfg(feature = "gas-profiler")]
let func_name = func.pretty_string();
#[cfg(feature = "gas-profiler")]
profile_open_frame!(gas_meter, func_name.clone());

// Charge gas
Expand All @@ -277,7 +280,7 @@ impl Interpreter {
if func.is_native() {
self.call_native(&resolver, gas_meter, extensions, func, ty_args)?;
current_frame.pc += 1; // advance past the Call instruction in the caller

#[cfg(feature = "gas-profiler")]
profile_close_frame!(gas_meter, func_name.clone());

continue;
Expand Down Expand Up @@ -1302,7 +1305,7 @@ impl Frame {
),
)
});

#[cfg(feature = "gas-profiler")]
profile_open_instr!(gas_meter, format!("{:?}", instruction));

let r = Self::execute_instruction(
Expand All @@ -1315,7 +1318,7 @@ impl Frame {
gas_meter,
instruction,
)?;

#[cfg(feature = "gas-profiler")]
profile_close_instr!(gas_meter, format!("{:?}", instruction));

match r {
Expand Down
13 changes: 10 additions & 3 deletions external-crates/move/move-execution/v0/move-vm/runtime/src/interpreter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use move_core_types::{
use move_vm_config::runtime::VMRuntimeLimitsConfig;
#[cfg(feature = "gas-profiler")]
use move_vm_profiler::GasProfiler;
#[cfg(feature = "gas-profiler")]
use move_vm_profiler::{
profile_close_frame, profile_close_instr, profile_open_frame, profile_open_instr,
};
Expand Down Expand Up @@ -115,7 +116,7 @@ impl Interpreter {
call_stack: CallStack::new(),
runtime_limits_config: loader.vm_config().runtime_limits_config.clone(),
};

#[cfg(feature = "gas-profiler")]
profile_open_frame!(gas_meter, function.pretty_string());

if function.is_native() {
Expand Down Expand Up @@ -146,7 +147,7 @@ impl Interpreter {
)
.finish(Location::Undefined),
})?;

#[cfg(feature = "gas-profiler")]
profile_close_frame!(gas_meter, function.pretty_string());

Ok(return_values.into_iter().collect())
Expand Down Expand Up @@ -206,6 +207,7 @@ impl Interpreter {
gas_meter
.charge_drop_frame(non_ref_vals.into_iter())
.map_err(|e| self.set_location(e))?;
#[cfg(feature = "gas-profiler")]
profile_close_frame!(gas_meter, current_frame.function.pretty_string());

if let Some(frame) = self.call_stack.pop() {
Expand All @@ -221,6 +223,7 @@ impl Interpreter {
let func = resolver.function_from_handle(fh_idx);
#[cfg(feature = "gas-profiler")]
let func_name = func.pretty_string();
#[cfg(feature = "gas-profiler")]
profile_open_frame!(gas_meter, func_name.clone());

// Charge gas
Expand All @@ -245,6 +248,7 @@ impl Interpreter {
if func.is_native() {
self.call_native(&resolver, gas_meter, extensions, func.clone(), vec![])?;
current_frame.pc += 1; // advance past the Call instruction in the caller
#[cfg(feature = "gas-profiler")]
profile_close_frame!(gas_meter, func_name.clone());

continue;
Expand All @@ -269,6 +273,7 @@ impl Interpreter {
let func = resolver.function_from_instantiation(idx);
#[cfg(feature = "gas-profiler")]
let func_name = func.pretty_string();
#[cfg(feature = "gas-profiler")]
profile_open_frame!(gas_meter, func_name.clone());

// Charge gas
Expand All @@ -294,6 +299,7 @@ impl Interpreter {
if func.is_native() {
self.call_native(&resolver, gas_meter, extensions, func.clone(), ty_args)?;
current_frame.pc += 1; // advance past the Call instruction in the caller
#[cfg(feature = "gas-profiler")]
profile_close_frame!(gas_meter, func_name.clone());
continue;
}
Expand Down Expand Up @@ -1325,7 +1331,7 @@ impl Frame {
),
)
});

#[cfg(feature = "gas-profiler")]
profile_open_instr!(gas_meter, format!("{:?}", instruction));
let r = Self::execute_instruction(
&mut self.pc,
Expand All @@ -1337,6 +1343,7 @@ impl Frame {
gas_meter,
instruction,
)?;
#[cfg(feature = "gas-profiler")]
profile_close_instr!(gas_meter, format!("{:?}", instruction));

match r {
Expand Down
Loading

0 comments on commit 817479f

Please sign in to comment.