feat: implement gas profiler. #3038

olonho · 2020-07-25T14:48:58Z

Implement profiler for gas consumption.

Command like

cd runtime/near-vm-runner-standalone  &&  cargo run -- --wasm-file /work/near/near-linkdrop/res/linkdrop.wasm --method-name create_account --input '{"new_account_id": "zest.near", "new_public_key": "ed25519:C1BYE6i7bDoqccHuXfGTHXQzJsdhjN7MQ6h3iCRxycCY" }' --profile-gas

produces

------------------------------
Total gas: 12849996969996
Host gas: 239484430401 [1% total]
Action gas: 12247300236960 [95% total]
Wasm execution: 363212302635 [2% total]
------ Host functions --------
base -> 5295362220 [0% total, 2% host]
contract_compile_base -> 35445963 [0% total, 0% host]
contract_compile_bytes -> 42827632500 [0% total, 17% host]
read_memory_base -> 28708495200 [0% total, 11% host]
read_memory_byte -> 809683929 [0% total, 0% host]
write_memory_base -> 11215179444 [0% total, 4% host]
write_memory_byte -> 359537904 [0% total, 0% host]
read_register_base -> 7551495558 [0% total, 3% host]
read_register_byte -> 11433192 [0% total, 0% host]
write_register_base -> 8596567458 [0% total, 3% host]
write_register_byte -> 440981424 [0% total, 0% host]
utf8_decoding_base -> 6223558122 [0% total, 2% host]
utf8_decoding_byte -> 4082126706 [0% total, 1% host]
storage_write_base -> 64196736000 [0% total, 26% host]
storage_write_key_byte -> 352414335 [0% total, 0% host]
storage_write_value_byte -> 1706019645 [0% total, 0% host]
storage_read_base -> 56356845750 [0% total, 23% host]
storage_read_key_byte -> 154762665 [0% total, 0% host]
promise_return -> 560152386 [0% total, 0% host]
------ Actions --------
create_account -> 99607375000 [0% total]
function_call -> 2320006835710 [18% total]
transfer -> 115123062500 [0% total]
add_key -> 101765125000 [0% total]
new_receipt -> 9610797838750 [74% total]
------------------------------

gitpod-io · 2020-07-25T14:49:02Z

MaksymZavershynskyi · 2020-07-25T16:03:04Z

runtime/near-vm-logic/src/gas_counter.rs

@@ -22,16 +22,19 @@ pub struct GasCounter {
    prepaid_gas: Gas,
    is_view: bool,
    ext_costs_config: ExtCostsConfig,
+    /// Where to store profile data, if needed.
+    profile: Option<&'a mut Vec<u64>>,


Please hide it behind costs_counting flag and consider Rc<RefCell<Vec<u64>> also to avoid lifetime poisoning (when introduction of lifetime into one type leads to introduction of it in dependent types).

MaksymZavershynskyi · 2020-07-25T16:06:06Z

runtime/near-vm-logic/Cargo.toml

@@ -19,6 +19,8 @@ base64 = "0.11"
 serde = { version = "1", features = ["derive"] }
 sha2 = "0.8"
 sha3 = "0.8"
+strum = "0.18"


Let's not add more dependencies, we are trying to keep near-vm-logic minimal because we have plans of compiling it to Wasm. Also strum is not a widely used library, let's implement enum iterator manually here.

MaksymZavershynskyi · 2020-07-25T16:06:29Z

runtime/near-vm-runner-standalone/Cargo.toml


 near-vm-logic = { path = "../near-vm-logic", version = "1.0.0"}
-near-vm-runner = { path = "../near-vm-runner", version = "1.0.0" }
+near-vm-runner = { path = "../near-vm-runner", version = "1.0.0", features = ["wasmtime_vm"] }


Nope, intentional, so that we could run in standalone mode with both VMs.

ailisp

Looks good, maybe more helpful to output number as space seperate columns at each line begining, so unix sort command can list cost from max to minimum easier

codecov · 2020-07-28T17:49:59Z

Codecov Report

Merging #3038 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3038   +/-   ##
=======================================
  Coverage   87.82%   87.82%           
=======================================
  Files         213      213           
  Lines       42377    42377           
=======================================
  Hits        37219    37219           
  Misses       5158     5158

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 70f4e8a...db69143. Read the comment docs.

runtime/near-vm-logic/src/config.rs

MaksymZavershynskyi

Looks good overall. There are some comments.

MaksymZavershynskyi · 2020-07-30T01:11:46Z

runtime/near-vm-logic/src/gas_counter.rs

@@ -64,29 +75,81 @@ impl GasCounter {

    #[cfg(feature = "costs_counting")]
    #[inline]
-    fn inc_ext_costs_counter(&self, cost: ExtCosts, value: u64) {
+    fn inc_ext_costs_counter(&mut self, cost: ExtCosts, value: u64) {
        EXT_COSTS_COUNTER.with(|f| {


I thought you suggested to remove EXT_COSTS_COUNTER because it is an expensive thread-local tool. If we have profile that it looks like it should be easy to remove EXT_COSTS_COUNTER

Well, they have different semantics with profile (profile sums costs, EXT_COSTS_COUNTER counts invocations) and EXT_COSTS_COUNTER id being used in the cost estimator, so I decided to keep it.

MaksymZavershynskyi · 2020-07-30T01:16:25Z

runtime/near-vm-logic/src/gas_counter.rs

        self.deduct_gas(burn_gas, use_gas)
    }

    /// A helper function to pay base cost gas fee for batching an action.
    /// # Args:
    /// * `base_fee`: base fee for the action;
    /// * `sir`: whether the receiver_id is same as the current account ID;
-    pub fn pay_action_base(&mut self, base_fee: &Fee, sir: bool) -> Result<()> {
+    pub fn pay_action_base(&mut self, base_fee: &Fee, sir: bool, action: Actions) -> Result<()> {


We should just change pay_action_base and pay_acion_per_byte so that it takes action but does not take base_fee and then store RuntimeFeesConfig in GasCounter just like we store ExtCostsConfig. Then we would need to implement value() method for Actions just like we did it here: https://github.com/nearprotocol/nearcore/blob/f024a28d9b0f1b2238f04d288ddebc8ac12ff4d1/runtime/near-vm-logic/src/config.rs#L490

This is rather major change, do you believe we shall do that in this (somewhat unrelated) profiler change?

Let's create a tracking issue for it and fix it later.

MaksymZavershynskyi · 2020-07-30T01:17:58Z

runtime/near-vm-logic/src/config.rs

+        ExtCosts::validator_total_stake_base as usize + 1
+    }
+
+    pub fn name_of(index: usize) -> &'static str {


Let's implement Display trait instead.

I shall implement Display for which class? Note, that we have usize, not enum as an input, and usize -> ExtCosts conversion is not straightforward.

and usize -> ExtCosts conversion is not straightforward.

Why not? It should be a straightforward cast.

int cannot be casted to enum, as far, as I could see.

MaksymZavershynskyi · 2020-07-30T01:25:15Z

runtime/near-vm-logic/src/config.rs

@@ -541,4 +577,62 @@ impl ExtCosts {
            validator_total_stake_base => config.validator_total_stake_base,
        }
    }
+
+    pub fn count() -> usize {


Let's try to make it const: https://doc.rust-lang.org/reference/items/functions.html#const-functions

MaksymZavershynskyi · 2020-07-30T01:26:42Z

runtime/near-vm-logic/src/types.rs

@@ -35,3 +37,5 @@ pub enum PromiseResult {
    Successful(Vec<u8>),
    Failed,
 }
+
+pub type ProfileData = Rc<RefCell<Vec<u64>>>;


Could we try Rc<RefCell<[u8; Actions::count() + ExtCosts::count() + 1]>> once count() is const?

MaksymZavershynskyi · 2020-07-30T01:29:19Z

runtime/near-vm-logic/src/gas_counter.rs

@@ -22,6 +22,8 @@ pub struct GasCounter {
    prepaid_gas: Gas,
    is_view: bool,
    ext_costs_config: ExtCostsConfig,
+    /// Where to store profile data, if needed.


If I understood you correctly @olonho you said that it creates a negligible overhead if so then we can try to drop costs_counting flag once we get rid of the thread local static variable.

Not sure if we could, see comment above. It's used in https://github.com/nearprotocol/nearcore/blob/f024a28d9b0f1b2238f04d288ddebc8ac12ff4d1/runtime/runtime-params-estimator/src/stats.rs#L29.

MaksymZavershynskyi · 2020-07-30T01:31:01Z

Cargo.lock

 "serde",
 "serde_json",
+ "strum",


It does not look like strum is used anymore.

MaksymZavershynskyi · 2020-07-30T01:31:30Z

Cargo.lock

 "serde",
 "serde_json",
+ "strum",


It does not look like strum is used anymore.

olonho · 2020-07-30T15:15:57Z

Looks good, maybe more helpful to output number as space seperate columns at each line begining, so unix sort command can list cost from max to minimum easier

If sorting is needed, we'd better do that in code, but I guess it's not too big so that we could see relevant parts w/o sorting.

MaksymZavershynskyi

Approving, let's create a refactoring cleanup issue for tracking.

MaksymZavershynskyi · 2020-07-31T00:52:21Z

runtime/near-vm-logic/src/config.rs

+        ExtCosts::validator_total_stake_base as usize + 1
+    }
+
+    pub fn name_of(index: usize) -> &'static str {


and usize -> ExtCosts conversion is not straightforward.

Why not? It should be a straightforward cast.

ilblackdragon · 2020-08-02T05:03:23Z

runtime/near-vm-logic/src/logic.rs

@@ -864,7 +867,7 @@ impl<'a> VMLogic<'a> {
                .ok_or(HostError::IntegerOverflow)?;
        }
        use_gas = use_gas.checked_add(burn_gas).ok_or(HostError::IntegerOverflow)?;
-        self.gas_counter.deduct_gas(burn_gas, use_gas)
+        self.gas_counter.pay_action_accumulated(burn_gas, use_gas, Actions::new_receipt)


Hm, I might be wrong, but doesn't this has all the unused gas as well?
E.g. if we attach 100Tgas and use 20Tgas, this will be 80% usage because it will record 80 Tgas?

it only records burnt gas in the profiler.

evgenykuzyakov · 2020-08-03T16:12:49Z

runtime/near-vm-logic/src/config.rs

+        write!(
+            f,
+            "{}",
+            vec![


Is it possible to reuse name_of?

evgenykuzyakov · 2020-08-03T16:13:43Z

runtime/near-vm-logic/src/config.rs

+        write!(
+            f,
+            "{}",
+            vec![


Maybe also reuse name_of

evgenykuzyakov · 2020-08-03T16:15:05Z

runtime/near-vm-logic/src/config.rs

+// Type of an action, used in fees logic.
+#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug, PartialOrd, Ord)]
+#[allow(non_camel_case_types)]
+pub enum Actions {


I don't like the name of it, because it's confusing to Action. Is it possible to rename it to ActionCosts or ActionProfiler or something similar to indicate it's not an Action

Also it doesn't have to be public. Can be pub(crate)

It's exported to standalone runner, so not sure if could be crate pub.

This type is used outside of the crate, namely in standalone launcher.

evgenykuzyakov · 2020-08-03T16:19:14Z

runtime/near-vm-logic/src/types.rs

@@ -35,3 +38,6 @@ pub enum PromiseResult {
    Successful(Vec<u8>),
    Failed,
 }
+
+/// Profile of gas consumption, +1 for Wasm bytecode execution cost.
+pub type ProfileData = Rc<RefCell<[u64; Actions::count() + ExtCosts::count() + 1]>>;


Can we hide it behind the feature as well? It seems like this introduces RefCell and Rc towards the vm-logic from std, so this creates the dependencies.

As this type is used in signatures, it will clutter all code using it with the feature. Original code didn't use RefCell / Rc but had explicit lifetimes, and Max requested to rewrite it. What would we consider least evil?

evgenykuzyakov · 2020-08-03T16:20:52Z

runtime/near-vm-logic/src/gas_counter.rs

@@ -22,6 +22,8 @@ pub struct GasCounter {
    prepaid_gas: Gas,
    is_view: bool,
    ext_costs_config: ExtCostsConfig,
+    /// Where to store profile data, if needed.
+    profile: Option<ProfileData>,


Instead of put it into the constructor, can you put it using setProfile method. This way it can be behind a feature and doesn't affect existing implementations.

But is it really correct for the cost breakdown be behind the flag?

willemneal · 2020-08-04T13:33:28Z

runtime/near-vm-runner-standalone/src/main.rs

+            action_gas += profile_data[e as usize + ExtCosts::count()];
+        }
+
+        let wasm_gas = profile_data[Actions::count() + ExtCosts::count()];


After some thought it might be good if we had the option to write to JSON file.

Well, maybe, but not sure what usecase would be.

Well currently in near-sdk-as, it uses the rust binary and expects the output to be the JSON of the outcome. It is possible to work around this, but it would be a lot simpler if the only output is the outcome.

olonho · 2020-08-05T18:09:34Z

PTAL

evgenykuzyakov · 2020-08-17T17:52:08Z

runtime/near-vm-logic/src/gas_counter.rs

@@ -108,21 +172,41 @@ impl GasCounter {
                num_bytes.checked_mul(per_byte_fee.exec_fee()).ok_or(HostError::IntegerOverflow)?,
            )
            .ok_or(HostError::IntegerOverflow)?;
-
+        self.update_profile_action(action, burn_gas);
        self.deduct_gas(burn_gas, use_gas)
    }

    /// A helper function to pay base cost gas fee for batching an action.
    /// # Args:
    /// * `base_fee`: base fee for the action;
    /// * `sir`: whether the receiver_id is same as the current account ID;


Please update comments

evgenykuzyakov · 2020-08-17T17:52:17Z

runtime/near-vm-logic/src/gas_counter.rs

+        self.deduct_gas(burn_gas, use_gas)
+    }
+
+    pub fn pay_action_accumulated(


Add a comment

evgenykuzyakov · 2020-08-17T17:52:58Z

runtime/near-vm-logic/src/gas_counter.rs

+    fn update_profile_wasm(&mut self, _value: u64) {}
+
+    pub fn pay_wasm_gas(&mut self, value: u64) -> Result<()> {
+        self.update_profile_wasm(value);


We should remove this and use difference from total

olonho requested review from evgenykuzyakov and MaksymZavershynskyi July 25, 2020 14:48

olonho requested a review from ailisp as a code owner July 25, 2020 14:48

MaksymZavershynskyi suggested changes Jul 25, 2020

View reviewed changes

olonho force-pushed the gas_profiler branch from 3afc8f4 to e53c80c Compare July 27, 2020 13:02

ailisp approved these changes Jul 27, 2020

View reviewed changes

olonho force-pushed the gas_profiler branch from 9252309 to a4073fb Compare July 28, 2020 14:46

olonho requested a review from MaksymZavershynskyi July 28, 2020 14:48

olonho force-pushed the gas_profiler branch from a4073fb to 56dbe08 Compare July 28, 2020 15:25

evgenykuzyakov reviewed Jul 29, 2020

View reviewed changes

runtime/near-vm-logic/src/config.rs Outdated Show resolved Hide resolved

MaksymZavershynskyi suggested changes Jul 30, 2020

View reviewed changes

olonho requested a review from MaksymZavershynskyi July 30, 2020 15:40

MaksymZavershynskyi approved these changes Jul 31, 2020

View reviewed changes

weekly-digest bot mentioned this pull request Jul 31, 2020

Weekly Digest (24 July, 2020 - 31 July, 2020) #3062

Closed

ilblackdragon reviewed Aug 2, 2020

View reviewed changes

evgenykuzyakov reviewed Aug 3, 2020

View reviewed changes

willemneal reviewed Aug 4, 2020

View reviewed changes

olonho force-pushed the gas_profiler branch from 543ec54 to bce4024 Compare August 5, 2020 16:37

weekly-digest bot mentioned this pull request Aug 7, 2020

Weekly Digest (31 July, 2020 - 7 August, 2020) #3100

Closed

ilblackdragon mentioned this pull request Aug 17, 2020

Run profiler with remote state and contract near/near-cli#501

Closed

olonho force-pushed the gas_profiler branch from def5f92 to c358cd9 Compare August 17, 2020 17:49

evgenykuzyakov requested changes Aug 17, 2020

View reviewed changes

olonho added 4 commits August 18, 2020 12:21

Implement gas profiler.

3f04549

Fix typo

215321c

Review feedback.

8866c3d

Fix formatting.

4d2d5c7

olonho added 2 commits August 18, 2020 12:21

Review feedback.

d46caa7

Fix review.

24bbfed

olonho force-pushed the gas_profiler branch from e9d3329 to 24bbfed Compare August 18, 2020 09:21

olonho requested a review from evgenykuzyakov August 18, 2020 09:22

olonho mentioned this pull request Aug 18, 2020

[perf] Runtime: Optimize gas counter by splitting burn_gas and used_gas #3071

Closed

evgenykuzyakov approved these changes Aug 18, 2020

View reviewed changes

Merge branch 'master' into gas_profiler

db69143

olonho merged commit 1db01be into master Aug 18, 2020

olonho deleted the gas_profiler branch August 18, 2020 17:31

silca3553 mentioned this pull request Jul 30, 2022

Research gas model postech-dao/near#35

Open

lrettig mentioned this pull request Jan 10, 2023

Investigate gas metering in other protocols spacemeshos/pm#210

Closed

feat: implement gas profiler. #3038

feat: implement gas profiler. #3038

Conversation

olonho commented Jul 25, 2020 • edited Loading

gitpod-io bot commented Jul 25, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ailisp left a comment

Choose a reason for hiding this comment

codecov bot commented Jul 28, 2020 • edited Loading

Codecov Report

MaksymZavershynskyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olonho commented Jul 30, 2020

MaksymZavershynskyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olonho commented Aug 5, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olonho commented Jul 25, 2020 •

edited

Loading

gitpod-io bot commented Jul 25, 2020 •

edited

Loading

codecov bot commented Jul 28, 2020 •

edited

Loading