feat(bench): make benchmark inputs deterministic using fixed seed #13586

DeVikingMark · 2024-12-28T18:25:21Z

Description of Changes

Made benchmark inputs deterministic by using a fixed seed for all random number generators. This resolves the issue of noise in test results on codspeed.

The changes include:

Added a fixed seed (12345) for all TestRunner instances.
Used the ChaCha algorithm for RNG, ensuring deterministic results.
Updated the following benchmarks:
- crates/trie/sparse/benches/rlp_node.rs
- crates/trie/sparse/benches/root.rs
- crates/trie/trie/benches/trie_root.rs
- crates/transaction-pool/benches/priority.rs

Testing

Ran benchmarks multiple times to verify reproducibility of results.
Confirmed that results remain stable across runs.

Additional Notes

This change does not affect the functionality of the code itself but ensures that benchmark results are more stable and reproducible.

Rjected

thanks! I have some suggestions on reducing some TestRunner setup lines

Rjected · 2024-12-30T17:18:17Z

crates/transaction-pool/benches/priority.rs

+fn criterion_benchmark(c: &mut Criterion) {
+    let mut group = c.benchmark_group("priority");
+    group.sample_size(10);
+
+    let config = ProptestConfig {
+        rng_algorithm: prop::test_runner::RngAlgorithm::ChaCha,
+        seed: 12345,
+        ..Default::default()
+    };
+
+    let mut group = c.benchmark_group("Blob priority calculation");
+    let fee_jump_input = generate_test_data_fee_delta();
+
+    // Unstable sorting of unsorted collection
+    fee_jump_bench(&mut group, "BenchmarkDynamicFeeJumpCalculation", fee_jump_input);
+
+    let blob_priority_input = generate_test_data_priority();
+
+    // BinaryHeap that is resorted on each update
+    priority_bench(&mut group, "BenchmarkPriorityCalculation", blob_priority_input);
+}
+


we should be making the existing benches deterministic, instead of adding a new bench

Rjected · 2024-12-30T17:20:13Z

crates/trie/sparse/benches/rlp_node.rs

+        let mut runner = TestRunner::new(ProptestConfig {
+            rng_algorithm: prop::test_runner::RngAlgorithm::ChaCha,
+            seed: 12345,
+            ..Default::default()
+        });


this should be using let mut runner = TestRunner::deterministic(); instead

Rjected · 2024-12-30T17:20:25Z

crates/trie/trie/benches/trie_root.rs

@@ -30,7 +30,11 @@ pub fn trie_root_benchmark(c: &mut Criterion) {

 fn generate_test_data(size: usize) -> Vec<ReceiptWithBloom<Receipt>> {
    prop::collection::vec(arb::<ReceiptWithBloom<Receipt>>(), size)
-        .new_tree(&mut TestRunner::new(ProptestConfig::default()))
+        .new_tree(&mut TestRunner::new(ProptestConfig {


same nit re: using the deterministic runner

Rjected · 2024-12-30T17:20:29Z

crates/trie/sparse/benches/root.rs

+    let mut runner = TestRunner::new(ProptestConfig {
+        rng_algorithm: prop::test_runner::RngAlgorithm::ChaCha,
+        seed: 12345,
+        ..Default::default()
+    });


same nit re: using the deterministic runner

DeVikingMark · 2024-12-30T18:51:41Z

@Rjected I made some changes, could you pls review them? tnx

Rjected

thanks! almost there, looks like compile error is caused by an artifact of older changes?

Rjected · 2024-12-30T18:58:30Z

crates/transaction-pool/benches/priority.rs

+    let mut group = c.benchmark_group("priority");
+    group.sample_size(10);
+


seems like this needs to be removed?

seems like this needs to be removed?

you're right, I changed, it seems to be that this one is what we need, or am I trippin'?

Rjected

lgtm, thanks!

codspeed-hq · 2024-12-30T20:56:58Z

CodSpeed Performance Report

Merging #13586 will improve performances by ×3.5

_{Comparing DeVikingMark:potuzhniy (a698607) with main (0b135a2)}

Summary

⚡ 22 improvements
✅ 55 untouched benchmarks

Benchmarks breakdown

	Benchmark	`main`	`DeVikingMark:potuzhniy`	Change
⚡	`hash builder[init size 1000	update size 100	num updates 10]`	31.5 ms
⚡	`hash builder[init size 1000	update size 100	num updates 5]`	14.4 ms
⚡	`hash builder[init size 1000	update size 1000	num updates 10]`	196.1 ms
⚡	`hash builder[init size 1000	update size 1000	num updates 1]`	9.8 ms
⚡	`hash builder[init size 1000	update size 1000	num updates 3]`	39.2 ms
⚡	`hash builder[init size 1000	update size 1000	num updates 5]`	78 ms
⚡	`hash builder[init size 10000	update size 1000	num updates 10]`	310.4 ms
⚡	`hash builder[init size 10000	update size 1000	num updates 5]`	141.1 ms
⚡	`sparse trie[init size 1000	update size 100	num updates 10]`	15.6 ms
⚡	`sparse trie[init size 1000	update size 100	num updates 1]`	1.6 ms
⚡	`sparse trie[init size 1000	update size 100	num updates 3]`	4.3 ms
⚡	`sparse trie[init size 1000	update size 100	num updates 5]`	7.7 ms
⚡	`sparse trie[init size 1000	update size 1000	num updates 10]`	122.3 ms
⚡	`sparse trie[init size 1000	update size 1000	num updates 1]`	8.7 ms
⚡	`sparse trie[init size 1000	update size 1000	num updates 3]`	29 ms
⚡	`sparse trie[init size 1000	update size 1000	num updates 5]`	53 ms
⚡	`sparse trie[init size 10000	update size 100	num updates 10]`	30.9 ms
⚡	`sparse trie[init size 10000	update size 100	num updates 5]`	17.9 ms
⚡	`sparse trie[init size 10000	update size 1000	num updates 10]`	156.8 ms
⚡	`sparse trie[init size 10000	update size 1000	num updates 1]`	18.8 ms
...	...	...	...	...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

DeVikingMark · 2024-12-30T21:19:36Z

lgtm, thanks!

It was a pleasure to help you 🫡 Could you pls merge it? :)

DeVikingMark added 5 commits December 28, 2024 19:19

Update rlp_node.rs

285d63b

Update root.rs

e0dcfe8

Update trie_root.rs

ffe91aa

Update priority.rs

b400269

Update priority.rs

6c5cbf8

DeVikingMark requested review from rkrasiuk, Rjected, shekhirin and mattsse as code owners December 28, 2024 18:25

Rjected requested changes Dec 30, 2024

View reviewed changes

DeVikingMark added 4 commits December 30, 2024 19:42

Update rlp_node.rs

058ccb8

Update priority.rs

9fb2654

Update trie_root.rs

ed89bf6

Update root.rs

182b7f6

Rjected requested changes Dec 30, 2024

View reviewed changes

Update priority.rs

c884f8d

Rjected approved these changes Dec 30, 2024

View reviewed changes

fix: add rng_with_seed helper

a698607

Rjected requested a review from gakonst as a code owner December 30, 2024 20:48

Rjected added this pull request to the merge queue Dec 30, 2024

Merged via the queue into paradigmxyz:main with commit 5629ba0 Dec 30, 2024
43 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(bench): make benchmark inputs deterministic using fixed seed #13586

feat(bench): make benchmark inputs deterministic using fixed seed #13586

DeVikingMark commented Dec 28, 2024 •

edited by Rjected

Loading

Rjected left a comment

Rjected Dec 30, 2024

Rjected Dec 30, 2024

Rjected Dec 30, 2024

Rjected Dec 30, 2024

DeVikingMark commented Dec 30, 2024

Rjected left a comment

Rjected Dec 30, 2024

DeVikingMark Dec 30, 2024

Rjected left a comment

codspeed-hq bot commented Dec 30, 2024

DeVikingMark commented Dec 30, 2024

		let mut group = c.benchmark_group("priority");
		group.sample_size(10);

feat(bench): make benchmark inputs deterministic using fixed seed #13586

feat(bench): make benchmark inputs deterministic using fixed seed #13586

Conversation

DeVikingMark commented Dec 28, 2024 • edited by Rjected Loading

Description of Changes

Testing

Additional Notes

Rjected left a comment

Choose a reason for hiding this comment

Rjected Dec 30, 2024

Choose a reason for hiding this comment

Rjected Dec 30, 2024

Choose a reason for hiding this comment

Rjected Dec 30, 2024

Choose a reason for hiding this comment

Rjected Dec 30, 2024

Choose a reason for hiding this comment

DeVikingMark commented Dec 30, 2024

Rjected left a comment

Choose a reason for hiding this comment

Rjected Dec 30, 2024

Choose a reason for hiding this comment

DeVikingMark Dec 30, 2024

Choose a reason for hiding this comment

Rjected left a comment

Choose a reason for hiding this comment

codspeed-hq bot commented Dec 30, 2024

CodSpeed Performance Report

Merging #13586 will improve performances by ×3.5

Summary

Benchmarks breakdown

DeVikingMark commented Dec 30, 2024

DeVikingMark commented Dec 28, 2024 •

edited by Rjected

Loading