Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Commit 65ac8a5

Browse files
shawntabriziParity Botgui1117
authored
Include StorageInfo in Benchmarking Pipeline (#9090)
* extend storageinfo * extend_storage_info * use vec * add storage info to pipeline * get read and written keys * undo storageinfo move * refactor keytracker * return read / write count * playing with key matching * add basic `StorageInfo` constructor * add whitelisted to returned info * fix some test stuff * pipe comments into benchmark data * add_storage_comments * add comments to template * track only storage prefix * Update frame/benchmarking/src/lib.rs * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_balances --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/balances/src/weights.rs --template=./.maintain/frame-weight-template.hbs * fix test * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_balances --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/balances/src/weights.rs --template=./.maintain/frame-weight-template.hbs * remove test logs * add temp benchmark script * Apply suggestions from code review Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com> * remove keytracker and use trackedstoragekey * add comment for unknown keys * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_timestamp --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/timestamp/src/weights.rs --template=./.maintain/frame-weight-template.hbs * remove duplicate comments with unknown keys * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_timestamp --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/timestamp/src/weights.rs --template=./.maintain/frame-weight-template.hbs * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_balances --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/balances/src/weights.rs --template=./.maintain/frame-weight-template.hbs * refactor bench tracker, and fix results * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_balances --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/balances/src/weights.rs --template=./.maintain/frame-weight-template.hbs * fix child tries in new tracker * extra newline * fix unused warning * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_timestamp --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/timestamp/src/weights.rs --template=./.maintain/frame-weight-template.hbs * fix master merge * storage info usage refactor * remove now unused * fix refactor * use a vec for prefix * fix tests * also update writer to use vec * disable read and written keys for now * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=frame_system --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/system/src/weights.rs --template=./.maintain/frame-weight-template.hbs * Update frame/system/src/weights.rs * fix test * Delete weights.rs * reset weights Co-authored-by: Parity Bot <admin@parity.io> Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
1 parent 78da574 commit 65ac8a5

File tree

28 files changed

+552
-185
lines changed

28 files changed

+552
-185
lines changed

‎.maintain/frame-weight-template.hbs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ pub trait WeightInfo {
4747
pub struct SubstrateWeight<T>(PhantomData<T>);
4848
impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
4949
{{~#each benchmarks as |benchmark|}}
50+
{{~#each benchmark.comments as |comment|}}
51+
// {{comment}}
52+
{{~/each}}
5053
fn {{benchmark.name~}}
5154
(
5255
{{~#each benchmark.components as |c| ~}}
@@ -76,6 +79,9 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
7679
// For backwards compatibility and tests
7780
impl WeightInfo for () {
7881
{{~#each benchmarks as |benchmark|}}
82+
{{~#each benchmark.comments as |comment|}}
83+
// {{comment}}
84+
{{~/each}}
7985
fn {{benchmark.name~}}
8086
(
8187
{{~#each benchmark.components as |c| ~}}

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎bin/node-template/runtime/src/lib.rs‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ pub use pallet_balances::Call as BalancesCall;
3131
pub use sp_runtime::{Permill, Perbill};
3232
pub use frame_support::{
3333
construct_runtime, parameter_types, StorageValue,
34-
traits::{KeyOwnerProofSystem, Randomness},
34+
traits::{KeyOwnerProofSystem, Randomness, StorageInfo},
3535
weights::{
3636
Weight, IdentityFee,
3737
constants::{BlockExecutionWeight, ExtrinsicBaseWeight, RocksDbWeight, WEIGHT_PER_SECOND},
@@ -450,8 +450,12 @@ impl_runtime_apis! {
450450
impl frame_benchmarking::Benchmark<Block> for Runtime {
451451
fn dispatch_benchmark(
452452
config: frame_benchmarking::BenchmarkConfig
453-
) -> Result<Vec<frame_benchmarking::BenchmarkBatch>, sp_runtime::RuntimeString> {
453+
) -> Result<
454+
(Vec<frame_benchmarking::BenchmarkBatch>, Vec<StorageInfo>),
455+
sp_runtime::RuntimeString,
456+
> {
454457
use frame_benchmarking::{Benchmarking, BenchmarkBatch, add_benchmark, TrackedStorageKey};
458+
use frame_support::traits::StorageInfoTrait;
455459

456460
use frame_system_benchmarking::Pallet as SystemBench;
457461
impl frame_system_benchmarking::Config for Runtime {}
@@ -469,6 +473,8 @@ impl_runtime_apis! {
469473
hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef780d41e5e16056765bc8461851072c9d7").to_vec().into(),
470474
];
471475

476+
let storage_info = AllPalletsWithSystem::storage_info();
477+
472478
let mut batches = Vec::<BenchmarkBatch>::new();
473479
let params = (&config, &whitelist);
474480

@@ -478,7 +484,7 @@ impl_runtime_apis! {
478484
add_benchmark!(params, batches, pallet_template, TemplateModule);
479485

480486
if batches.is_empty() { return Err("Benchmark not found for this pallet.".into()) }
481-
Ok(batches)
487+
Ok((batches, storage_info))
482488
}
483489
}
484490
}

‎bin/node/runtime/src/lib.rs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1509,8 +1509,13 @@ impl_runtime_apis! {
15091509
impl frame_benchmarking::Benchmark<Block> for Runtime {
15101510
fn dispatch_benchmark(
15111511
config: frame_benchmarking::BenchmarkConfig
1512-
) -> Result<Vec<frame_benchmarking::BenchmarkBatch>, sp_runtime::RuntimeString> {
1512+
) -> Result<
1513+
(Vec<frame_benchmarking::BenchmarkBatch>, Vec<frame_support::traits::StorageInfo>),
1514+
sp_runtime::RuntimeString,
1515+
> {
15131516
use frame_benchmarking::{Benchmarking, BenchmarkBatch, add_benchmark, TrackedStorageKey};
1517+
use frame_support::traits::StorageInfoTrait;
1518+
15141519
// Trying to add benchmarks directly to the Session Pallet caused cyclic dependency
15151520
// issues. To get around that, we separated the Session benchmarks into its own crate,
15161521
// which is why we need these two lines below.
@@ -1537,6 +1542,8 @@ impl_runtime_apis! {
15371542
hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7b99d880ec681799c0cf30e8886371da95ecffd7b6c0f78751baa9d281e0bfa3a6d6f646c70792f74727372790000000000000000000000000000000000000000").to_vec().into(),
15381543
];
15391544

1545+
let storage_info = AllPalletsWithSystem::storage_info();
1546+
15401547
let mut batches = Vec::<BenchmarkBatch>::new();
15411548
let params = (&config, &whitelist);
15421549

@@ -1574,7 +1581,7 @@ impl_runtime_apis! {
15741581
add_benchmark!(params, batches, pallet_election_provider_multi_phase, ElectionProviderMultiPhase);
15751582

15761583
if batches.is_empty() { return Err("Benchmark not found for this pallet.".into()) }
1577-
Ok(batches)
1584+
Ok((batches, storage_info))
15781585
}
15791586
}
15801587
}

‎client/db/src/bench.rs‎

Lines changed: 101 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -66,40 +66,6 @@ impl<Block: BlockT> sp_state_machine::Storage<HashFor<Block>> for StorageDb<Bloc
6666
}
6767
}
6868

69-
/// Track whether a specific key has already been read or written to.
70-
#[derive(Default, Clone, Copy)]
71-
pub struct KeyTracker {
72-
has_been_read: bool,
73-
has_been_written: bool,
74-
}
75-
76-
/// A simple object that counts the reads and writes at the key level to the underlying state db.
77-
#[derive(Default, Clone, Copy, Debug)]
78-
pub struct ReadWriteTracker {
79-
reads: u32,
80-
repeat_reads: u32,
81-
writes: u32,
82-
repeat_writes: u32,
83-
}
84-
85-
impl ReadWriteTracker {
86-
fn add_read(&mut self) {
87-
self.reads += 1;
88-
}
89-
90-
fn add_repeat_read(&mut self) {
91-
self.repeat_reads += 1;
92-
}
93-
94-
fn add_write(&mut self) {
95-
self.writes += 1;
96-
}
97-
98-
fn add_repeat_write(&mut self) {
99-
self.repeat_writes += 1;
100-
}
101-
}
102-
10369
/// State that manages the backend database reference. Allows runtime to control the database.
10470
pub struct BenchmarkingState<B: BlockT> {
10571
root: Cell<B::Hash>,
@@ -110,11 +76,14 @@ pub struct BenchmarkingState<B: BlockT> {
11076
record: Cell<Vec<Vec<u8>>>,
11177
shared_cache: SharedCache<B>, // shared cache is always empty
11278
/// Key tracker for keys in the main trie.
113-
main_key_tracker: RefCell<HashMap<Vec<u8>, KeyTracker>>,
79+
/// We track the total number of reads and writes to these keys,
80+
/// not de-duplicated for repeats.
81+
main_key_tracker: RefCell<HashMap<Vec<u8>, TrackedStorageKey>>,
11482
/// Key tracker for keys in a child trie.
11583
/// Child trie are identified by their storage key (i.e. `ChildInfo::storage_key()`)
116-
child_key_tracker: RefCell<HashMap<Vec<u8>, HashMap<Vec<u8>, KeyTracker>>>,
117-
read_write_tracker: RefCell<ReadWriteTracker>,
84+
/// We track the total number of reads and writes to these keys,
85+
/// not de-duplicated for repeats.
86+
child_key_tracker: RefCell<HashMap<Vec<u8>, HashMap<Vec<u8>, TrackedStorageKey>>>,
11887
whitelist: RefCell<Vec<TrackedStorageKey>>,
11988
proof_recorder: Option<ProofRecorder<B::Hash>>,
12089
proof_recorder_root: Cell<B::Hash>,
@@ -137,7 +106,6 @@ impl<B: BlockT> BenchmarkingState<B> {
137106
shared_cache: new_shared_cache(0, (1, 10)),
138107
main_key_tracker: Default::default(),
139108
child_key_tracker: Default::default(),
140-
read_write_tracker: Default::default(),
141109
whitelist: Default::default(),
142110
proof_recorder: record_proof.then(Default::default),
143111
proof_recorder_root: Cell::new(root.clone()),
@@ -191,10 +159,8 @@ impl<B: BlockT> BenchmarkingState<B> {
191159
let whitelist = self.whitelist.borrow();
192160

193161
whitelist.iter().for_each(|key| {
194-
let whitelisted = KeyTracker {
195-
has_been_read: key.has_been_read,
196-
has_been_written: key.has_been_written,
197-
};
162+
let mut whitelisted = TrackedStorageKey::new(key.key.clone());
163+
whitelisted.whitelist();
198164
main_key_tracker.insert(key.key.clone(), whitelisted);
199165
});
200166
}
@@ -203,12 +169,10 @@ impl<B: BlockT> BenchmarkingState<B> {
203169
*self.main_key_tracker.borrow_mut() = HashMap::new();
204170
*self.child_key_tracker.borrow_mut() = HashMap::new();
205171
self.add_whitelist_to_tracker();
206-
*self.read_write_tracker.borrow_mut() = Default::default();
207172
}
208173

209174
// Childtrie is identified by its storage key (i.e. `ChildInfo::storage_key`)
210175
fn add_read_key(&self, childtrie: Option<&[u8]>, key: &[u8]) {
211-
let mut read_write_tracker = self.read_write_tracker.borrow_mut();
212176
let mut child_key_tracker = self.child_key_tracker.borrow_mut();
213177
let mut main_key_tracker = self.main_key_tracker.borrow_mut();
214178

@@ -218,33 +182,21 @@ impl<B: BlockT> BenchmarkingState<B> {
218182
&mut main_key_tracker
219183
};
220184

221-
let read = match key_tracker.get(key) {
185+
let should_log = match key_tracker.get_mut(key) {
222186
None => {
223-
let has_been_read = KeyTracker {
224-
has_been_read: true,
225-
has_been_written: false,
226-
};
187+
let mut has_been_read = TrackedStorageKey::new(key.to_vec());
188+
has_been_read.add_read();
227189
key_tracker.insert(key.to_vec(), has_been_read);
228-
read_write_tracker.add_read();
229190
true
230191
},
231192
Some(tracker) => {
232-
if !tracker.has_been_read {
233-
let has_been_read = KeyTracker {
234-
has_been_read: true,
235-
has_been_written: tracker.has_been_written,
236-
};
237-
key_tracker.insert(key.to_vec(), has_been_read);
238-
read_write_tracker.add_read();
239-
true
240-
} else {
241-
read_write_tracker.add_repeat_read();
242-
false
243-
}
193+
let should_log = !tracker.has_been_read();
194+
tracker.add_read();
195+
should_log
244196
}
245197
};
246198

247-
if read {
199+
if should_log {
248200
if let Some(childtrie) = childtrie {
249201
log::trace!(
250202
target: "benchmark",
@@ -258,7 +210,6 @@ impl<B: BlockT> BenchmarkingState<B> {
258210

259211
// Childtrie is identified by its storage key (i.e. `ChildInfo::storage_key`)
260212
fn add_write_key(&self, childtrie: Option<&[u8]>, key: &[u8]) {
261-
let mut read_write_tracker = self.read_write_tracker.borrow_mut();
262213
let mut child_key_tracker = self.child_key_tracker.borrow_mut();
263214
let mut main_key_tracker = self.main_key_tracker.borrow_mut();
264215

@@ -269,30 +220,21 @@ impl<B: BlockT> BenchmarkingState<B> {
269220
};
270221

271222
// If we have written to the key, we also consider that we have read from it.
272-
let has_been_written = KeyTracker {
273-
has_been_read: true,
274-
has_been_written: true,
275-
};
276-
277-
let write = match key_tracker.get(key) {
223+
let should_log = match key_tracker.get_mut(key) {
278224
None => {
225+
let mut has_been_written = TrackedStorageKey::new(key.to_vec());
226+
has_been_written.add_write();
279227
key_tracker.insert(key.to_vec(), has_been_written);
280-
read_write_tracker.add_write();
281228
true
282229
},
283230
Some(tracker) => {
284-
if !tracker.has_been_written {
285-
key_tracker.insert(key.to_vec(), has_been_written);
286-
read_write_tracker.add_write();
287-
true
288-
} else {
289-
read_write_tracker.add_repeat_write();
290-
false
291-
}
231+
let should_log = !tracker.has_been_written();
232+
tracker.add_write();
233+
should_log
292234
}
293235
};
294236

295-
if write {
237+
if should_log {
296238
if let Some(childtrie) = childtrie {
297239
log::trace!(
298240
target: "benchmark",
@@ -303,6 +245,23 @@ impl<B: BlockT> BenchmarkingState<B> {
303245
}
304246
}
305247
}
248+
249+
// Return all the tracked storage keys among main and child trie.
250+
fn all_trackers(&self) -> Vec<TrackedStorageKey> {
251+
let mut all_trackers = Vec::new();
252+
253+
self.main_key_tracker.borrow().iter().for_each(|(_, tracker)| {
254+
all_trackers.push(tracker.clone());
255+
});
256+
257+
self.child_key_tracker.borrow().iter().for_each(|(_, child_tracker)| {
258+
child_tracker.iter().for_each(|(_, tracker)| {
259+
all_trackers.push(tracker.clone());
260+
});
261+
});
262+
263+
all_trackers
264+
}
306265
}
307266

308267
fn state_err() -> String {
@@ -507,9 +466,30 @@ impl<B: BlockT> StateBackend<HashFor<B>> for BenchmarkingState<B> {
507466
}
508467

509468
/// Get the key tracking information for the state db.
469+
/// 1. `reads` - Total number of DB reads.
470+
/// 2. `repeat_reads` - Total number of in-memory reads.
471+
/// 3. `writes` - Total number of DB writes.
472+
/// 4. `repeat_writes` - Total number of in-memory writes.
510473
fn read_write_count(&self) -> (u32, u32, u32, u32) {
511-
let count = *self.read_write_tracker.borrow_mut();
512-
(count.reads, count.repeat_reads, count.writes, count.repeat_writes)
474+
let mut reads = 0;
475+
let mut repeat_reads = 0;
476+
let mut writes = 0;
477+
let mut repeat_writes = 0;
478+
479+
self.all_trackers().iter().for_each(|tracker| {
480+
if !tracker.whitelisted {
481+
if tracker.reads > 0 {
482+
reads += 1;
483+
repeat_reads += tracker.reads - 1;
484+
}
485+
486+
if tracker.writes > 0 {
487+
writes += 1;
488+
repeat_writes += tracker.reads - 1;
489+
}
490+
}
491+
});
492+
(reads, repeat_reads, writes, repeat_writes)
513493
}
514494

515495
/// Reset the key tracking information for the state db.
@@ -525,6 +505,40 @@ impl<B: BlockT> StateBackend<HashFor<B>> for BenchmarkingState<B> {
525505
*self.whitelist.borrow_mut() = new;
526506
}
527507

508+
fn get_read_and_written_keys(&self) -> Vec<(Vec<u8>, u32, u32, bool)> {
509+
// We only track at the level of a key-prefix and not whitelisted for now for memory size.
510+
// TODO: Refactor to enable full storage key transparency, where we can remove the
511+
// `prefix_key_tracker`.
512+
let mut prefix_key_tracker = HashMap::<Vec<u8>, (u32, u32, bool)>::new();
513+
self.all_trackers().iter().for_each(|tracker| {
514+
if !tracker.whitelisted {
515+
let prefix_length = tracker.key.len().min(32);
516+
let prefix = tracker.key[0..prefix_length].to_vec();
517+
// each read / write of a specific key is counted at most one time, since
518+
// additional reads / writes happen in the memory overlay.
519+
let reads = tracker.reads.min(1);
520+
let writes = tracker.writes.min(1);
521+
if let Some(prefix_tracker) = prefix_key_tracker.get_mut(&prefix) {
522+
prefix_tracker.0 += reads;
523+
prefix_tracker.1 += writes;
524+
} else {
525+
prefix_key_tracker.insert(
526+
prefix,
527+
(
528+
reads,
529+
writes,
530+
tracker.whitelisted,
531+
),
532+
);
533+
}
534+
}
535+
});
536+
537+
prefix_key_tracker.iter().map(|(key, tracker)| -> (Vec<u8>, u32, u32, bool) {
538+
(key.to_vec(), tracker.0, tracker.1, tracker.2)
539+
}).collect::<Vec<_>>()
540+
}
541+
528542
fn register_overlay_stats(&self, stats: &sp_state_machine::StateMachineStats) {
529543
self.state.borrow_mut().as_mut().map(|s| s.register_overlay_stats(stats));
530544
}
@@ -597,11 +611,11 @@ mod test {
597611
]
598612
).unwrap();
599613

600-
let rw_tracker = bench_state.read_write_tracker.borrow();
601-
assert_eq!(rw_tracker.reads, 6);
602-
assert_eq!(rw_tracker.repeat_reads, 0);
603-
assert_eq!(rw_tracker.writes, 2);
604-
assert_eq!(rw_tracker.repeat_writes, 0);
614+
let rw_tracker = bench_state.read_write_count();
615+
assert_eq!(rw_tracker.0, 6);
616+
assert_eq!(rw_tracker.1, 0);
617+
assert_eq!(rw_tracker.2, 2);
618+
assert_eq!(rw_tracker.3, 0);
605619
drop(rw_tracker);
606620
bench_state.wipe().unwrap();
607621
}

0 commit comments

Comments
 (0)