Skip to content

Commit

Permalink
chore(estimator): remove TTN read estimation (#9307)
Browse files Browse the repository at this point in the history
Since we have flat storage for reads, we no longer charge for touched trie nodes (TTN) on reads.
Remove the gas estimation for it.

More specifically, we used to estimate TTN cost as `max(read_ttn, write_ttn)` and therefore had 3 numbers reported. (read, write, combined).
Now we only need a single number reported.

The removed code (read TTN estimation) also didn't work anymore, as it didn't actually touch any trie nodes, and hence an assertion was triggered.

```
thread 'main' panicked at 'assertion failed: nodes_touched_delta as usize >= 2 * final_key_len - 10', runtime/runtime-params-estimator/src/trie.rs:118:5
stack backtrace:
   0: rust_begin_unwind
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:64:14
   2: core::panicking::panic
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:114:5
   3: runtime_params_estimator::touching_trie_node_read
   4: runtime_params_estimator::touching_trie_node
   5: runtime_params_estimator::run_estimation
   6: runtime_params_estimator::main
```

We "fix" it by removing the code.
  • Loading branch information
jakmeier authored Jul 14, 2023
1 parent f7e4b47 commit 4cf8141
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 99 deletions.
20 changes: 4 additions & 16 deletions runtime/runtime-params-estimator/src/cost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,8 +546,10 @@ pub enum Cost {
/// `storage_write` or `storage_remove`. The fee is paid once for each
/// unique trie node accessed.
///
/// Estimation: Take the maximum of estimations for `TouchingTrieNodeRead`
/// and `TouchingTrieNodeWrite`
/// Estimation: Prepare an account that has many keys stored that are
/// prefixes from each other. Then measure write cost for the shortest and
/// the longest key. The gas estimation difference is divided by the
/// difference of actually touched nodes.
TouchingTrieNode,
/// It is similar to `TouchingTrieNode`, but it is charged instead of this
/// cost when we can guarantee that trie node is cached in memory, which
Expand All @@ -562,20 +564,6 @@ pub enum Cost {
/// for this are a bit involved but roughly speaking, it just forces values
/// out of CPU caches so that they are always read from memory.
ReadCachedTrieNode,
/// Helper estimation for `TouchingTrieNode`
///
/// Estimation: Prepare an account that has many keys stored that are
/// prefixes from each other. Then measure access cost for the shortest and
/// the longest key. The gas estimation difference is divided by the
/// difference of actually touched nodes.
TouchingTrieNodeRead,
/// Helper estimation for `TouchingTrieNode`
///
/// Estimation: Prepare an account that has many keys stored that are
/// prefixes from each other. Then measure write cost for the shortest and
/// the longest key. The gas estimation difference is divided by the
/// difference of actually touched nodes.
TouchingTrieNodeWrite,
/// Estimates `promise_and_base` which is charged for every call to
/// `promise_and`. This should cover the base cost for creating receipt
/// dependencies.
Expand Down
1 change: 0 additions & 1 deletion runtime/runtime-params-estimator/src/estimator_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ pub(crate) struct CachedCosts {
pub(crate) compile_cost_base_per_byte_v2: Option<(GasCost, GasCost)>,
pub(crate) gas_metering_cost_base_per_op: Option<(GasCost, GasCost)>,
pub(crate) apply_block: Option<GasCost>,
pub(crate) touching_trie_node_read: Option<GasCost>,
pub(crate) touching_trie_node_write: Option<GasCost>,
pub(crate) ed25519_verify_base: Option<GasCost>,
}
Expand Down
23 changes: 3 additions & 20 deletions runtime/runtime-params-estimator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,6 @@ static ALL_COSTS: &[(Cost, fn(&mut EstimatorContext) -> GasCost)] = &[
(Cost::StorageRemoveRetValueByte, storage_remove_ret_value_byte),
(Cost::TouchingTrieNode, touching_trie_node),
(Cost::ReadCachedTrieNode, read_cached_trie_node),
(Cost::TouchingTrieNodeRead, touching_trie_node_read),
(Cost::TouchingTrieNodeWrite, touching_trie_node_write),
(Cost::ApplyBlock, apply_block_cost),
(Cost::ContractCompileBase, contract_compile_base),
(Cost::ContractCompileBytes, contract_compile_bytes),
Expand Down Expand Up @@ -1168,24 +1166,9 @@ fn storage_remove_ret_value_byte(ctx: &mut EstimatorContext) -> GasCost {
}

fn touching_trie_node(ctx: &mut EstimatorContext) -> GasCost {
let read = touching_trie_node_read(ctx);
let write = touching_trie_node_write(ctx);
return std::cmp::max(read, write);
}

fn touching_trie_node_read(ctx: &mut EstimatorContext) -> GasCost {
if let Some(cost) = ctx.cached.touching_trie_node_read.clone() {
return cost;
}
let warmup_iters = ctx.config.warmup_iters_per_block;
let measured_iters = ctx.config.iter_per_block;
// Number of bytes in the final key. Will create 2x that many nodes.
// Picked somewhat arbitrarily, balancing estimation time vs accuracy.
let final_key_len = 1000;
let cost = trie::read_node_from_db(ctx, warmup_iters, measured_iters, final_key_len);

ctx.cached.touching_trie_node_read = Some(cost.clone());
cost
// TTN write cost = TTN cost because we no longer charge it on reads since
// flat storage for reads was introduced
touching_trie_node_write(ctx)
}

fn touching_trie_node_write(ctx: &mut EstimatorContext) -> GasCost {
Expand Down
8 changes: 0 additions & 8 deletions runtime/runtime-params-estimator/src/transaction_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,6 @@ impl TransactionBuilder {
self.transaction_from_function_call(account, "account_storage_insert_key", arg)
}

/// Transaction that checks existence of a given key under an account.
/// The account must have the test contract deployed.
pub(crate) fn account_has_key(&mut self, account: AccountId, key: &str) -> SignedTransaction {
let arg = (key.len() as u64).to_le_bytes().into_iter().chain(key.bytes()).collect();

self.transaction_from_function_call(account, "account_storage_has_key", arg)
}

pub(crate) fn rng(&mut self) -> ThreadRng {
rand::thread_rng()
}
Expand Down
54 changes: 0 additions & 54 deletions runtime/runtime-params-estimator/src/trie.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use near_primitives::hash::hash;
use near_primitives::types::TrieCacheMode;
use near_store::{TrieCachingStorage, TrieStorage};
use near_vm_runner::logic::ExtCosts;
use std::iter;
use std::sync::atomic::{AtomicUsize, Ordering};

static SINK: AtomicUsize = AtomicUsize::new(0);
Expand Down Expand Up @@ -69,59 +68,6 @@ pub(crate) fn write_node(
cost
}

pub(crate) fn read_node_from_db(
ctx: &mut EstimatorContext,
warmup_iters: usize,
measured_iters: usize,
final_key_len: usize,
) -> GasCost {
let block_latency = 0;
let overhead = overhead_per_measured_block(ctx, block_latency);
let mut testbed = ctx.testbed();
let tb = testbed.transaction_builder();
// Prepare a long chain in the trie
let signer = tb.random_account();
let key = "j".repeat(final_key_len);
let mut setup_block = Vec::new();
for key_len in 0..final_key_len {
let key = &key.as_bytes()[..key_len];
let value = b"0";
setup_block.push(tb.account_insert_key(signer.clone(), key, value));
}
let mut blocks = Vec::with_capacity(1 + 2 * warmup_iters + 2 * measured_iters);
blocks.push(setup_block);
blocks.extend(
iter::repeat_with(|| vec![tb.account_has_key(signer.clone(), &key[0..1])])
.take(measured_iters + warmup_iters),
);
blocks.extend(
iter::repeat_with(|| vec![tb.account_has_key(signer.clone(), &key)])
.take(measured_iters + warmup_iters),
);
let results = &testbed.measure_blocks(blocks, block_latency)[1..];
let (short_key_results, long_key_results) = results.split_at(measured_iters + warmup_iters);
let (cost_short_key, ext_cost_short_key) = aggregate_per_block_measurements(
1,
short_key_results[warmup_iters..].to_vec(),
Some(overhead.clone()),
);
let (cost_long_key, ext_cost_long_key) = aggregate_per_block_measurements(
1,
long_key_results[warmup_iters..].to_vec(),
Some(overhead),
);
let nodes_touched_delta = ext_cost_long_key[&ExtCosts::touching_trie_node]
- ext_cost_short_key[&ExtCosts::touching_trie_node];
// The exact number of touched nodes is a implementation that we don't want
// to test here but it should be close to 2*final_key_len
assert!(nodes_touched_delta as usize <= 2 * final_key_len + 10);
assert!(nodes_touched_delta as usize >= 2 * final_key_len - 10);
let cost_delta =
cost_long_key.saturating_sub(&cost_short_key, &NonNegativeTolerance::PER_MILLE);
let cost = cost_delta / nodes_touched_delta;
cost
}

pub(crate) fn read_node_from_chunk_cache(testbed: &mut Testbed) -> GasCost {
let debug = testbed.config.debug;
let iters = 200;
Expand Down

0 comments on commit 4cf8141

Please sign in to comment.