Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
22b8699
validation extension in sp_io
cheme Jan 27, 2021
4de9f7a
need paths
cheme Jan 27, 2021
f0ea2af
arc impl
cheme Jan 27, 2021
71202a9
missing host function in executor
cheme Jan 28, 2021
f848b44
io to pkdot
cheme Feb 1, 2021
1979b6a
decode function.
cheme Feb 1, 2021
927a096
encode primitive.
cheme Feb 1, 2021
b034ec0
trailing tab
cheme Feb 1, 2021
757ff30
multiple patch
cheme Feb 1, 2021
7dba01b
fix child trie logic
cheme Feb 2, 2021
1a2a637
Merge branch 'master' into validation_io, and revert skip value specific
cheme Apr 8, 2021
c948254
Merge branch 'master' into validation_io
cheme Apr 8, 2021
7ef5a41
restore master versionning
cheme Apr 8, 2021
360c324
bench compact proof size
cheme Apr 8, 2021
4edc38a
trie-db 22.3 is needed
cheme Apr 8, 2021
6f899c0
line width
cheme Apr 8, 2021
d3eb91c
split line
cheme Apr 8, 2021
4475edd
fixes for bench (additional root may not be needed as original issue was
cheme Apr 9, 2021
e6e4682
Merge branch 'master' into validation_io
cheme May 3, 2021
dba397c
Merge branch 'validation_io' of github.com:cheme/substrate into valid…
cheme May 3, 2021
c0fc8fd
revert compact from block size calculation.
cheme May 3, 2021
8f353dd
Merge branch 'master' into validation_io
cheme May 28, 2021
7ebb54d
Merge branch 'master' into validation_io
cheme Jun 4, 2021
bf91187
New error type for compression.
cheme Jun 4, 2021
c37046e
Adding test (incomplete (failing)).
cheme Jun 4, 2021
7daabb3
Merge branch 'master' into validation_io
cheme Jun 4, 2021
cfd923a
There is currently no proof recording utility in sp_trie, removing
cheme Jun 4, 2021
f25fc34
small test of child root in proof without a child proof.
cheme Jun 4, 2021
19c132e
remove empty test.
cheme Jun 6, 2021
515c2fd
remove non compact proof size
cheme Jun 6, 2021
8aadbf1
Merge branch 'master' into validation_io
cheme Jun 6, 2021
f867d7f
Missing revert.
cheme Jun 6, 2021
d639940
proof method to encode decode.
cheme Jun 7, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
bench compact proof size
  • Loading branch information
cheme committed Apr 8, 2021
commit 360c3240a55861e70d67d95b09813ce5ec013411
12 changes: 9 additions & 3 deletions client/db/src/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use std::cell::{Cell, RefCell};
use std::collections::HashMap;

use hash_db::{Prefix, Hasher};
use sp_trie::{MemoryDB, prefixed_key, StorageProof};
use sp_trie::{MemoryDB, prefixed_key, StorageProof, encode_compact};
use sp_core::{
storage::{ChildInfo, TrackedStorageKey},
hexdisplay::HexDisplay
Expand Down Expand Up @@ -517,14 +517,20 @@ impl<B: BlockT> StateBackend<HashFor<B>> for BenchmarkingState<B> {
self.state.borrow().as_ref().map_or(sp_state_machine::UsageInfo::empty(), |s| s.usage_info())
}

fn proof_size(&self) -> Option<u32> {
fn proof_size(&self) -> Option<(u32, u32)> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we return here both?

What is the rational behind this?

If we "compress" always, we are not interested in the uncompressed size? (And we actually should not call this compress, maybe something different)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point was to use this to check the size delta from batch.

At this point we 'compress' never, but if 'proof_size' in bench is only here for PoV, then keeping only one value could be fine.
(I don't know if we want to compress 'always' the parachain could choose, even if there is no real point in not doing it).

So yes I can remove it, just it does not really cost much to keep it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it is relevant to have both the compressed and uncompressed size for analysis.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is that relevant?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This question was also not answered yet.

I don't see any reason to return 2 values here. What is the benefit of this? We will use the compact one only anyway.

self.proof_recorder.as_ref().map(|recorder| {
let proof = StorageProof::new(recorder
.read()
.iter()
.filter_map(|(_k, v)| v.as_ref().map(|v| v.to_vec()))
.collect());
proof.encoded_size() as u32
let proof_size = proof.encoded_size() as u32;
let compact_proof = encode_compact::<sp_trie::Layout<HashFor<B>>>(
proof,
self.root.get(),
).unwrap();

(proof_size, compact_proof.encoded_size() as u32)
})
}
}
Expand Down
5 changes: 5 additions & 0 deletions frame/benchmarking/src/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pub enum BenchmarkSelector {
Reads,
Writes,
ProofSize,
CompactProofSize,
}

#[derive(Debug)]
Expand Down Expand Up @@ -88,6 +89,7 @@ impl Analysis {
BenchmarkSelector::Reads => result.reads.into(),
BenchmarkSelector::Writes => result.writes.into(),
BenchmarkSelector::ProofSize => result.proof_size.into(),
BenchmarkSelector::CompactProofSize => result.compact_proof_size.into(),
}
).collect();

Expand Down Expand Up @@ -129,6 +131,7 @@ impl Analysis {
BenchmarkSelector::Reads => result.reads.into(),
BenchmarkSelector::Writes => result.writes.into(),
BenchmarkSelector::ProofSize => result.proof_size.into(),
BenchmarkSelector::CompactProofSize => result.compact_proof_size.into(),
};
(result.components[i].1, data)
})
Expand Down Expand Up @@ -194,6 +197,7 @@ impl Analysis {
BenchmarkSelector::Reads => result.reads.into(),
BenchmarkSelector::Writes => result.writes.into(),
BenchmarkSelector::ProofSize => result.proof_size.into(),
BenchmarkSelector::CompactProofSize => result.compact_proof_size.into(),
})
}

Expand Down Expand Up @@ -375,6 +379,7 @@ mod tests {
writes,
repeat_writes: 0,
proof_size: 0,
compact_proof_size: 0,
}
}

Expand Down
8 changes: 6 additions & 2 deletions frame/benchmarking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -774,8 +774,11 @@ macro_rules! impl_benchmark {

// Calculate the diff caused by the benchmark.
let elapsed_extrinsic = finish_extrinsic.saturating_sub(start_extrinsic);
let diff_pov = match (start_pov, end_pov) {
(Some(start), Some(end)) => end.saturating_sub(start),
let (diff_pov, compact_diff_pov) = match (start_pov, end_pov) {
(Some((start, compact_start)), Some((end, compact_end))) => (
end.saturating_sub(start),
compact_end.saturating_sub(compact_start),
),
_ => Default::default(),
};

Expand Down Expand Up @@ -806,6 +809,7 @@ macro_rules! impl_benchmark {
writes: read_write_count.2,
repeat_writes: read_write_count.3,
proof_size: diff_pov,
compact_proof_size: compact_diff_pov,
});
}

Expand Down
3 changes: 2 additions & 1 deletion frame/benchmarking/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ pub struct BenchmarkResults {
pub writes: u32,
pub repeat_writes: u32,
pub proof_size: u32,
pub compact_proof_size: u32,
}

/// Configuration used to setup and run runtime benchmarks.
Expand Down Expand Up @@ -165,7 +166,7 @@ pub trait Benchmarking {
}

/// Get current estimated proof size.
fn proof_size(&self) -> Option<u32> {
fn proof_size(&self) -> Option<(u32, u32)> {
self.proof_size()
}
}
Expand Down
2 changes: 1 addition & 1 deletion primitives/externalities/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ pub trait Externalities: ExtensionStore {
///
/// Returns estimated proof size for the state queries so far.
/// Proof is reset on commit and wipe.
fn proof_size(&self) -> Option<u32> {
fn proof_size(&self) -> Option<(u32, u32)> {
None
}
}
Expand Down
2 changes: 1 addition & 1 deletion primitives/state-machine/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ pub trait Backend<H: Hasher>: sp_std::fmt::Debug {
fn set_whitelist(&self, _: Vec<TrackedStorageKey>) {}

/// Estimate proof size
fn proof_size(&self) -> Option<u32> {
fn proof_size(&self) -> Option<(u32, u32)> {
unimplemented!()
}
}
Expand Down
2 changes: 1 addition & 1 deletion primitives/state-machine/src/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,7 @@ where
self.backend.set_whitelist(new)
}

fn proof_size(&self) -> Option<u32> {
fn proof_size(&self) -> Option<(u32, u32)> {
self.backend.proof_size()
}
}
Expand Down
5 changes: 3 additions & 2 deletions utils/frame/benchmarking-cli/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,20 +126,21 @@ impl BenchmarkCmd {
// Print the table header
batch.results[0].components.iter().for_each(|param| print!("{:?},", param.0));

print!("extrinsic_time_ns,storage_root_time_ns,reads,repeat_reads,writes,repeat_writes,proof_size_bytes\n");
print!("extrinsic_time_ns,storage_root_time_ns,reads,repeat_reads,writes,repeat_writes,proof_size_bytes,compact_proof_size_bytes\n");
// Print the values
batch.results.iter().for_each(|result| {
let parameters = &result.components;
parameters.iter().for_each(|param| print!("{:?},", param.1));
// Print extrinsic time and storage root time
print!("{:?},{:?},{:?},{:?},{:?},{:?},{:?}\n",
print!("{:?},{:?},{:?},{:?},{:?},{:?},{:?},{:?}\n",
result.extrinsic_time,
result.storage_root_time,
result.reads,
result.repeat_reads,
result.writes,
result.repeat_writes,
result.proof_size,
result.compact_proof_size,
);
});

Expand Down
1 change: 1 addition & 0 deletions utils/frame/benchmarking-cli/src/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ mod test {
writes: (base + slope * i).into(),
repeat_writes: 0,
proof_size: 0,
compact_proof_size: 0,
}
)
}
Expand Down