feat: Add an impl for ProfileData and add it as an option to ApplyState #3730
Conversation
CLion betrayed me
Also updated profile data to include all of the gas so that it can be used to print debug info
was having difficulty in other projects not seeing clone method on type alias.
runtime/near-vm-runner/src/runner.rs
Outdated
| @@ -27,8 +27,10 @@ pub fn run<'a>( | |||
| promise_results: &'a [PromiseResult], | |||
| current_protocol_version: ProtocolVersion, | |||
| cache: Option<&'a dyn CompiledContractCache>, | |||
| #[allow(unused_variables)] | |||
There was a problem hiding this comment.
why is this variable not guarded by the feature flag?
| ) -> (Option<VMOutcome>, Option<VMError>) { | ||
| #[cfg(feature = "costs_counting")] |
There was a problem hiding this comment.
if we do this why do we not put the function argument behind a feature flag so that we don't have to do something like #[allow(unused_variables)]
There was a problem hiding this comment.
Because the caller always passes the parameter since it's in ApplyState and by default is None, but we only use it when the feature is on.
Making the parameter be behind the feature would require moving each call behind a feature flag too. Is this what you want?
There was a problem hiding this comment.
It sounds like a good idea. For example on-disk compilation cache introduced a few unwanted changes. Hiding a feature behind a flag makes it safer for non-prod changes.
| @@ -457,6 +457,7 @@ impl NightshadeRuntime { | |||
| cache: Some(Arc::new(StoreCompiledContractCache { store: self.store.clone() })), | |||
| #[cfg(feature = "protocol_feature_evm")] | |||
| evm_chain_id: self.evm_chain_id(), | |||
| profile: None, | |||
There was a problem hiding this comment.
Let's wrap profile field in the ApplyState as well with a feature.
It was causing issues because it requires `costs_counting` feature to be on
This change will allow the simulator to pass a profile data reference via ApplyState to get the break down of how gas was spent.
Currently this means only function calls are profiled, not view calls because the ViewApplyState is in the primitives crate, while ProfileData and ApplyState are in the
near-vm-logiccrate. A future PR that reorganizes these core types will allow adding the profile data to ViewApplyState.