[subsystem-benchmarks] Add statement-distribution benchmarks #3863

AndreiEres · 2024-03-27T14:42:31Z

Adds a subsystem benchmark for statements-distribution subsystem.

Results in CI (reference hw):

$ cargo bench -p polkadot-statement-distribution --bench statement-distribution-regression-bench --features subsystem-benchmarks

[Sent to peers] standart_deviation 0.07%
[Received from peers] standart_deviation 0.00%
[statement-distribution] standart_deviation 0.97%
[test-environment] standart_deviation 1.03%

Network usage, KiB                     total   per block
Received from peers                1088.0000    108.8000
Sent to peers                      1238.1800    123.8180

CPU usage, seconds                     total   per block
statement-distribution                0.3897      0.0390
test-environment                      0.4715      0.0472

alexggh

Overall, looks good to me, good job!

Left you some nit comments.

alexggh · 2024-05-23T08:36:58Z

polkadot/node/network/statement-distribution/benches/statement-distribution-regression-bench.rs

+		("Received from peers", 108.8000, 0.001),
+		("Sent to peers", 123.8180, 0.001),
+	]));
+	messages.extend(average_usage.check_cpu_usage(&[("statement-distribution", 0.0390, 0.1)]));


So, unless we are missing something here, can we concur that statement-distribution is not a subsystem with high resource(CPU + networking) demands ?

Yeah, it feels so, at least the signature checks seem damn fast compared to approval vote signatures :)

At least now we know about it

Yeah, it feels so, at least the signature checks seem damn fast compared to approval vote signatures :)

It is actually way less signature checks. For each candidate only the backing group would generate a statement so for polkadot you get 5 signature per candidate and with 100 candidates you check just 500 signatures in total.

In approval voting you need at least 30 votes(in practice more than that). per candidate so 30 signature per candidate and with 100 candidates you actually have to check 100 * 30 = 3000 signatures, that's around 6 times more signatures.

At least now we know about it

Yes, it was meant as a good news :D.

alexggh · 2024-05-23T08:42:01Z

polkadot/node/subsystem-bench/src/lib/network.rs

 	/// Generates peer_connected messages for all peers in `test_authorities`
-	pub fn generate_peer_connected(&self) -> Vec<AllMessages> {
+	pub fn generate_approval_distribution_peer_connected(&self) -> Vec<AllMessages> {


Nit: To not repeat these 2 functions you can name it something like

generate_subsystem_peer_connected(&self, lambda_to_transform_peer_connected_subsystem_specific_message)

and then call the lambda function in the map.

alexggh · 2024-05-23T08:44:59Z

polkadot/node/subsystem-bench/src/lib/mock/candidate_backing.rs

+
+const LOG_TARGET: &str = "subsystem-bench::candidate-backing-mock";
+
+pub struct MockCandidateBacking {


Nit: I would go with keeping the current convention where clearly define a CandidateBackingState structure, that gets included here.

alexggh · 2024-05-23T08:55:15Z

polkadot/node/subsystem-bench/src/lib/statement/mod.rs

+			.chain(two_hop_y_peers_and_groups)
+		{
+			let messages_sent_count = messages_tracker.get_mut(group_index).unwrap();
+			if *messages_sent_count {


I'm not too sure about this, my understanding is that a node could receive a statement originating in a group multiple times from different nodes, so I think we should let this send that message as well.

We need to get the messages same as in real network, so all have to go.

sandreim · 2024-05-24T11:43:00Z

polkadot/node/network/statement-distribution/benches/statement-distribution-regression-bench.rs

+		("Received from peers", 108.8000, 0.001),
+		("Sent to peers", 123.8180, 0.001),
+	]));
+	messages.extend(average_usage.check_cpu_usage(&[("statement-distribution", 0.0390, 0.1)]));


Yeah, it feels so, at least the signature checks seem damn fast compared to approval vote signatures :)

sandreim · 2024-05-24T11:45:31Z

polkadot/node/subsystem-bench/src/lib/mock/candidate_backing.rs

+				orchestra::FromOrchestra::Communication { msg } => {
+					gum::trace!(target: LOG_TARGET, msg=?msg, "recv message");
+
+					match msg {


I'd extract this to a function to help with readbility.

sandreim · 2024-05-24T11:48:58Z

polkadot/node/subsystem-bench/src/lib/mock/runtime_api.rs

+							_block_hash,
+							RuntimeApiRequest::AsyncBackingParams(sender),
+						) => {
+							let _ = sender.send(Ok(AsyncBackingParams {


these are not the production parameters that we use, but I guess doesn't really matter for this bench

Yeah, but better to set the production ones

sandreim · 2024-05-24T19:54:17Z

polkadot/node/subsystem-bench/src/lib/statement/mod.rs

+			.chain(two_hop_y_peers_and_groups)
+		{
+			let messages_sent_count = messages_tracker.get_mut(group_index).unwrap();
+			if *messages_sent_count {


We need to get the messages same as in real network, so all have to go.

alindima · 2024-05-27T11:32:59Z

polkadot/node/subsystem-bench/src/lib/mock/runtime_api.rs

+							}));
+						},
+						RuntimeApiMessage::Request(_parent, RuntimeApiRequest::Version(tx)) => {
+							tx.send(Ok(RuntimeApiRequest::DISABLED_VALIDATORS_RUNTIME_REQUIREMENT))


This needs to return the latest version, right?

At least a version that supports DisabledValidators

sandreim

Nice work! Thank you @AndreiEres

alindima

Left an idea but overall looks good, nice work!

alindima · 2024-05-27T13:02:15Z

polkadot/node/subsystem-bench/src/lib/statement/mod.rs

+		loop {
+			let manifests_count = state
+				.manifests_tracker
+				.values()
+				.filter(|v| v.load(Ordering::SeqCst))
+				.collect::<Vec<_>>()
+				.len();
+			gum::debug!(target: LOG_TARGET, "{}/{} manifest exchanges", manifests_count, candidates_advertised);
+
+			if manifests_count == candidates_advertised {
+				break;
+			}
+			tokio::time::sleep(Duration::from_millis(50)).await;


If I'm reading the code correctly, the manifests we are advertising already contain statements from all members of the backing group.
Therefore the only gossip that is emulated is the manifest/acknowledgement exchange. The statement gossip is not really stressing the subsystem.
In reality every backing group could continue sending Statement messages even after the backed candidate is fetched. (and we care to record these so that they can be punished in case of a dispute).
Maybe a manifest is advertised with only 2 backing votes but the backing group keeps issuing more votes even after that.

However, the code is already quite complicated and we can see that the CPU consumption of the subsystem is tiny, so I don't think implementing this will yield any benefit right now

Thanks @alindima ! A ticket describing this improvement should do justice!

Decided with @alindima, that in this specific case it doesn't make a big difference

* master: (93 commits) Fix broken windows build (#4636) Beefy client generic on aduthority Id (#1816) pallet-staking: Put tests behind `cfg(debug_assertions)` (#4620) Broker new price adapter (#4521) Change `XcmDryRunApi::dry_run_extrinsic` to take a call instead (#4621) Update README.md (#4623) Publish `chain-spec-builder` (#4518) Add omni bencher & chain-spec-builder bins to release (#4557) Moves runtime macro out of experimental flag (#4249) Filter workspace dependencies in the templates (#4599) parachain-inherent: Make `para_id` more prominent (#4555) Add metric to measure the time it takes to gather enough assignments (#4587) Improve On_demand_assigner events (#4339) Conditional `required` checks (#4544) [CI] Deny adding git deps (#4572) [subsytem-bench] Remove redundant banchmark_name param (#4540) Add availability-recovery from systematic chunks (#1644) Remove workspace lints from templates (#4598) `sc-chain-spec`: deprecated code removed (#4410) [subsystem-benchmarks] Add statement-distribution benchmarks (#3863) ...

…ech#3863) Fixes paritytech#3748 Adds a subsystem benchmark for statements-distribution subsystem. Results in CI (reference hw): ``` $ cargo bench -p polkadot-statement-distribution --bench statement-distribution-regression-bench --features subsystem-benchmarks [Sent to peers] standart_deviation 0.07% [Received from peers] standart_deviation 0.00% [statement-distribution] standart_deviation 0.97% [test-environment] standart_deviation 1.03% Network usage, KiB total per block Received from peers 1088.0000 108.8000 Sent to peers 1238.1800 123.8180 CPU usage, seconds total per block statement-distribution 0.3897 0.0390 test-environment 0.4715 0.0472 ```

Init

e25bd4a

AndreiEres added R0-silent Changes should not be mentioned in any release notes T12-benchmarks This PR/Issue is related to benchmarking and weights. labels Mar 27, 2024

AndreiEres added 3 commits April 11, 2024 08:47

Merge branch 'master' into AndreiEres/statement-distr-subsystem-bench

4208c5e

Add a skeleton

42f2a55

wip

3b4f88e

AndreiEres force-pushed the AndreiEres/statement-distr-subsystem-bench branch from 0976b23 to 3b4f88e Compare April 26, 2024 09:59

AndreiEres added 23 commits April 26, 2024 12:00

Merge branch 'master' into AndreiEres/statement-distr-subsystem-bench

c507c53

Add first iteration

0e474ce

Merge branch 'master' into AndreiEres/statement-distr-subsystem-bench

6780cf5

update

4ff06ab

update

c190589

update

9151571

remove redundant

0ec77eb

Generate candidates for all cores

d4abb60

Don't rely on full connectivity

99890cd

Receive manifests from neighbors

6cf9f78

Optimize neighbors messages

e7fb543

Polish

dcba6d0

Update

8ccf3cc

Reset trackers

560a5d5

Add benchmarks

daf901f

Update test state

4685d9f

Remove unused vars

011a7e7

Fix manifest missing

1512ff8

Move validator pairs to test authorities

cc683e3

Make HandleNetworkMessage async

ea1fd83

Request a candidate from own backing group

e29320f

Update var names

b8ac639

Use context to send messages

45a86ce

AndreiEres added 2 commits May 17, 2024 14:48

Remove clippy rule

e212386

Update baselines

c5119d5

AndreiEres requested review from alvicsam, alexggh and sandreim May 17, 2024 13:15

alvicsam approved these changes May 17, 2024

View reviewed changes

paritytech-review-bot bot requested a review from a team May 17, 2024 14:31

alexggh approved these changes May 23, 2024

View reviewed changes

alindima self-requested a review May 24, 2024 14:41

sandreim reviewed May 24, 2024

View reviewed changes

AndreiEres added 4 commits May 27, 2024 12:07

Use generic peer connected generation

74d477f

Use MockCandidateBackingState

0405d9d

Use AsyncBackingParams from Kusama

8775d70

Extract handle_statement

c7abc34

alindima reviewed May 27, 2024

View reviewed changes

Send more than 1 message

bd285d5

AndreiEres requested review from sandreim and alexggh May 27, 2024 12:09

Update baseline

a80d2f1

sandreim approved these changes May 27, 2024

View reviewed changes

alindima approved these changes May 27, 2024

View reviewed changes

AndreiEres added this pull request to the merge queue May 27, 2024

Merged via the queue into master with commit a709768 May 27, 2024
155 of 156 checks passed

AndreiEres deleted the AndreiEres/statement-distr-subsystem-bench branch May 27, 2024 19:49

This was referenced Aug 21, 2024

Update polkadot-sdk from v1.11.0 to stable2407 moondance-labs/tanssi#659

Closed

Update polkadot-sdk from v1.11.0 to stable2407 moonbeam-foundation/moonbeam#2912

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[subsystem-benchmarks] Add statement-distribution benchmarks #3863

[subsystem-benchmarks] Add statement-distribution benchmarks #3863

AndreiEres commented Mar 27, 2024 •

edited

Loading

alexggh left a comment

alexggh May 23, 2024

sandreim May 24, 2024

AndreiEres May 27, 2024

alexggh May 27, 2024

alexggh May 23, 2024

alexggh May 23, 2024

alexggh May 23, 2024 •

edited

Loading

sandreim May 24, 2024

sandreim May 24, 2024

sandreim May 24, 2024

sandreim May 24, 2024

AndreiEres May 27, 2024

sandreim May 24, 2024

alindima May 27, 2024

AndreiEres May 27, 2024

sandreim left a comment

alindima left a comment

alindima May 27, 2024

sandreim May 27, 2024

AndreiEres May 27, 2024


		const LOG_TARGET: &str = "subsystem-bench::candidate-backing-mock";

		pub struct MockCandidateBacking {

[subsystem-benchmarks] Add statement-distribution benchmarks #3863

[subsystem-benchmarks] Add statement-distribution benchmarks #3863

Conversation

AndreiEres commented Mar 27, 2024 • edited Loading

alexggh left a 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

alexggh May 23, 2024 • edited Loading

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

Choose a reason for hiding this comment

sandreim left a comment

Choose a reason for hiding this comment

alindima left a 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

AndreiEres commented Mar 27, 2024 •

edited

Loading

alexggh May 23, 2024 •

edited

Loading