Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

networking-bench: Update benchmarks payload #7056

Merged
merged 7 commits into from
Jan 9, 2025

Conversation

AndreiEres
Copy link
Contributor

Description

  • Used 10 notifications and requests within the benchmarks. After moving the network workers' initialization out of the benchmarks, it is acceptable to use this small number without losing precision.
  • Removed the 128MB payload that consumed most of the execution time.

@AndreiEres 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 Jan 6, 2025
@AndreiEres AndreiEres requested review from a team as code owners January 6, 2025 16:45
@AndreiEres AndreiEres requested a review from alvicsam January 6, 2025 16:46
@paritytech-review-bot paritytech-review-bot bot requested a review from a team January 7, 2025 14:35
@@ -156,12 +153,19 @@ where
tokio::select! {
Some(event) = notification_service1.next_event() => {
if let NotificationEvent::NotificationStreamOpened { .. } = event {
break;
// Send a 32MB notification to preheat the network
notification_service1.send_async_notification(&peer_id2, vec![0; 2usize.pow(25)]).await.unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to do a similar thing with requests: before doing any actual measurements request 32MB to reach the maximum TCP speed. Might be needed to modify the default max response size, as AFAIR it is < 32MB by default.

@AndreiEres AndreiEres enabled auto-merge January 9, 2025 17:52
@AndreiEres AndreiEres added this pull request to the merge queue Jan 9, 2025
Merged via the queue into master with commit 6bfe452 Jan 9, 2025
203 of 204 checks passed
@AndreiEres AndreiEres deleted the AndreiEres/update-networking-charts branch January 9, 2025 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T12-benchmarks This PR/Issue is related to benchmarking and weights.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants