Ghali/benchmark data root #204

Leouarz · 2023-08-24T10:35:04Z

No description provided.

ToufeeqP

LGTM

codecov-commenter · 2023-08-24T13:06:47Z

Codecov Report

Patch coverage has no change and project coverage change: +0.62% 🎉

Comparison is base (a19d6e6) 50.05% compared to head (57d6fef) 50.67%.
Report is 3 commits behind head on develop.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #204      +/-   ##
===========================================
+ Coverage    50.05%   50.67%   +0.62%     
===========================================
  Files           71       71              
  Lines        10063    10170     +107     
===========================================
+ Hits          5037     5154     +117     
+ Misses        5026     5016      -10

Files Changed	Coverage Δ
pallets/dactr/src/weights.rs	`5.55% <0.00%> (-0.70%)`	⬇️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

fmiguelgarcia · 2023-08-30T12:38:12Z

pallets/dactr/src/benchmarking.rs

+	// This benchmark is not directly used by extrinsic.
+	// It is mostly used to check that the weight is lower or approximately equal the `data_root` benchmark
+	data_root_batch {
+		let i in 0..(2 * 1024 * 1024);


i should be greater than max_tx_size, and we should use the max_block_len (not sure about this name)

Suggested change

let i in 0..(2 * 1024 * 1024);

let i in max_tx_size..max_block_len;

It would be nice if the step is max_tx_size too

Here, i go from 0 to 2mb et make n transactions from it, so it will benchmark the creation of dataroot until the maximum that we can put in a block.

Why should we go from max_tx_size to max_block_len which is basically the same from 512kb to 2mb ?

Also do we have somewhere this max_block_len ? I'm not sure we have something equivalent in the code...

fmiguelgarcia · 2023-08-30T12:42:33Z

pallets/dactr/src/benchmarking.rs

+		let nb_additional_tx = if remaining_size > 0 { 1 } else { 0 };
+		let mut calls = Vec::with_capacity(nb_full_tx as usize + nb_additional_tx as usize);


nit:

Suggested change

let nb_additional_tx = if remaining_size > 0 { 1 } else { 0 };

let mut calls = Vec::with_capacity(nb_full_tx as usize + nb_additional_tx as usize);

let mut calls = Vec::with_capacity(nb_full_tx as usize + 1usize);

fmiguelgarcia · 2023-08-30T12:44:50Z

pallets/dactr/src/weights.rs

-			// Standard Error: 12
-			.saturating_add(Weight::from_ref_time(4_321_u64).saturating_mul(i as u64))
+		// Minimum execution time: 1_457 nanoseconds.
+		Weight::from_ref_time(1_487_000_u64)


Do we know if this increase is related with the change of the hashing algorithm on data root calculation?

Yes i was confirmed that it's not from the changes in the algorithm.

Also benchmarking like this is quite not stable meaning we can get different result on retry.

fmiguelgarcia · 2023-08-30T12:47:46Z

pallets/dactr/src/weights.rs

+	/// The range of component `i` is `[0, 2097152]`.
+	fn data_root_batch(i: u32, ) -> Weight {
+		// Minimum execution time: 778 nanoseconds.
+		Weight::from_ref_time(825_000_u64)


It is weird that one data_root takes 1_487 as minimum but several data_root takes 824 (less).
I mean It should be equal or greater, right?

I guess when the range is greater the results are smoother.

* Updated rust toolchain * Incorporated upstream changes into frame-system * Updated packages to use latest support versions * Removed uncles & deprecated GenesisConfig * wip: weights v2 & fixed system pallet tests * Updated mocks with new frame-system configs * wip: Added ExtendedBlock with frame & da headers * Updated weights to v2 * Updated da control mocks to fix tests * Updated nomad-updater-manager mocks to fix tests * Updated nomad-home mocks * Fixed system benchmarkings * fmt & refactor * Added frame-executive as local dep to patch * fmt fix * Updated pallet configs * Added code comment for rust bug * commented nomad-da-bridge tests * Removed unintended std feature gate on imports * Updated runtime configs * pallets: implement for in * pallets: implement `Default` for `GenesisConfig` in `no_std` * Added missing configs & functions to runtime * Temp: removed da-bridge from runtime * Fixed runtime tests * Fixed kate-rpc's * Fixed compile errors of node service * Updated kate-rpc to use default HeaderExtension * Added header_builder hosted function back * clippy fixes * Temporarily removed frame-executive tests * Fixed frame-system try-runtime feature * Fixed various features of the workspace * add bug report template, PR template, feature request template, chang… (#201) * add bug report template, PR template, feature request template, changelog, code of conduct, contributing guide * add missing point in checklist * Update CODE_OF_CONDUCT.md * Ghali/remove warnings (#205) * remove warnings * remove useless comment * Add tx index from block mapping to data index. (#198) * Add tx index from block mapping to data index. * Simplify code. * Fix data transactions mapping, make code more readable. * Clean unused imports. * Update docs. * Use safe conversion. * Use safe conversion. * Add unit test for testing data proof generation. * tidy. * Add leaf index check. * Update examples to match rpc endpoint. * add --unsafe-da-sync flag to quickly sync the chain * refactoring * Moved (de)serialization of BlockLength to no_std * Updated beefy-merkle-tree name * Changed Runtime from enum to struct everywhere * Re-introduced da-bridge to runtime: welcome back:) * Fixed da-bridge pallet benchmarks * Ghali/benchmark data root (#204) * add data root benchmark in case of max block * add new weights for data_root * nit * Updated RPC port * Added troubleshooting to readme * Commented block_length_proposal from CI e2e * fmt: weights to v2 * code cleaning & clippy fixes * Updated multiplier to check for empty block * Updated api_dev of avail-subxt * Enabled submit_block_length_proposal test * camel_casing fields of BlockLength * updated api_dev in avail-subxt * bumped spec_version to 12 * Added FRAME pallet migrations * apply review suggestions * feat: Added identity pallet to our module list * feat: Added mandate pallet * Updated avail-core branch reference to main * Upgrade/polkadot v1.0 (#218) * camel_casing fields of BlockLength * updated api_dev in avail-subxt * feat: Added identity pallet to our module list * feat: Added mandate pallet * Updated avail-core branch reference to main * fix migrations --------- Co-authored-by: Toufeeq Pasha <toufeeq@sovereignwallet.network> Co-authored-by: Marko Petrlic <petrlic.marko@gmail.com> * merge upstream, generate subxt metadata, add block to scheduler migration * remove extra sneaky lines * format * Switch commitment gen to new api * Try updating deps in cargo lock * Move avail-subxt to workspace to fix dep desync * Pin deps to fix example build issues * Something broke in the build... It seems like now avail-core is including a v21 of sp-core, but I can't figure out where it's getting it! * rebase fix * Added OnceLock TODO * Moved from nightly version to latest stable * fmt * Reverted formatting * Changed the CI toolchain * update to weightv2, remove warning, use weights folder in runtime like polkadot-sdk --------- Co-authored-by: Toufeeq Pasha <toufeeq@sovereignwallet.network> Co-authored-by: Saša Pršić <93726535+0xSasaPrsic@users.noreply.github.com> Co-authored-by: Marko Petrlic <petrlic.marko@gmail.com> Co-authored-by: William Arnold <will748@gmail.com>

Leouarz added 2 commits August 24, 2023 10:11

add data root benchmark in case of max block

2057fe7

add new weights for data_root

da1c596

Leouarz requested review from fmiguelgarcia and ToufeeqP August 24, 2023 10:35

ToufeeqP approved these changes Aug 24, 2023

View reviewed changes

fmiguelgarcia reviewed Aug 30, 2023

View reviewed changes

fmiguelgarcia approved these changes Aug 30, 2023

View reviewed changes

nit

57d6fef

Leouarz merged commit 5f83153 into develop Sep 1, 2023
4 checks passed

Leouarz deleted the ghali/benchmark-data-root branch September 13, 2023 13:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ghali/benchmark data root #204

Ghali/benchmark data root #204

Leouarz commented Aug 24, 2023

ToufeeqP left a comment

codecov-commenter commented Aug 24, 2023 •

edited

Loading

fmiguelgarcia Aug 30, 2023

Leouarz Aug 30, 2023

Leouarz Aug 30, 2023

fmiguelgarcia Aug 30, 2023

fmiguelgarcia Aug 30, 2023

Leouarz Aug 30, 2023

fmiguelgarcia Aug 30, 2023

Leouarz Aug 30, 2023

	let i in 0..(2 * 1024 * 1024);
	let i in max_tx_size..max_block_len;

		let nb_additional_tx = if remaining_size > 0 { 1 } else { 0 };
		let mut calls = Vec::with_capacity(nb_full_tx as usize + nb_additional_tx as usize);

Ghali/benchmark data root #204

Ghali/benchmark data root #204

Conversation

Leouarz commented Aug 24, 2023

ToufeeqP left a comment

Choose a reason for hiding this comment

codecov-commenter commented Aug 24, 2023 • edited Loading

Codecov Report

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

codecov-commenter commented Aug 24, 2023 •

edited

Loading