Skip to content

test(network2): add new tcp-level benches#736

Merged
scarmuega merged 1 commit intomainfrom
test/benches-network2
Apr 1, 2026
Merged

test(network2): add new tcp-level benches#736
scarmuega merged 1 commit intomainfrom
test/benches-network2

Conversation

@scarmuega
Copy link
Copy Markdown
Member

@scarmuega scarmuega commented Apr 1, 2026

Summary by CodeRabbit

  • Tests
    • Added benchmarking infrastructure for the protocol module
    • Implemented performance benchmarks for handshake, chain synchronization, and block fetching operations

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 1, 2026

📝 Walkthrough

Walkthrough

Introduces Criterion benchmarking infrastructure for pallas-network2 protocol behavior. Adds dev dependencies and benchmark target configuration in Cargo.toml. Creates benchmark harness modules with MockChain, ResponderNode, and InitiatorNode for testing protocol interactions, along with three benchmark tests measuring handshake, chainsync, and blockfetch operations.

Changes

Cohort / File(s) Summary
Cargo Configuration
Cargo.toml
Added criterion (0.5) with async_tokio feature to dev-dependencies and configured custom [[bench]] target named protocol with harness = false.
Benchmark Harness Modules
benches/harness/mod.rs, benches/harness/chain.rs, benches/harness/node.rs
Created benchmark helper infrastructure: MockChain struct for generating protocol-compatible test data with slot tracking; ResponderNode for simulating responder behavior with TCP listener and event collection; InitiatorNode for driving initiator requests with async wait helper methods (wait_for_handshake, wait_for_intersection, wait_for_header, wait_for_block).
Benchmark Tests
benches/protocol.rs
Implemented three Criterion benchmarks (handshake, chainsync_10_headers, blockfetch) that create paired protocol nodes, drive specific protocol flows, and measure performance using the harness infrastructure.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 With whiskers twitching and benchmarks in hand,
I measure each hop through this protocol land,
Responders and initiators dance to the beat,
As criterion counts every async heartbeat! 💓⚡

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding new TCP-level benchmarks for the network2 module using Criterion. It's concise, specific, and clearly summarizes the primary purpose of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/benches-network2

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pallas-network2/benches/harness/node.rs`:
- Around line 42-79: The event loop currently never terminates because
manager.poll_next().await returns None is ignored and
ResponderEvent::PeerDisconnected is a no-op; change the None branch to
break/return so the loop exits when the stream ends and handle
ResponderEvent::PeerDisconnected by breaking (or returning) as well so the task
can complete and return the collected events vector; update the logic around the
loop that collects events (the events variable and any return at the end of the
function) so after breaking you return or propagate the events, referencing
manager.poll_next(), ResponderEvent::PeerDisconnected, and the events Vec to
locate where to modify.

In `@pallas-network2/benches/protocol.rs`:
- Around line 11-18: The responder task started by
ResponderNode::bind().await.spawn() is being aborted (responder.abort()) but not
awaited, causing benchmark jitter; change the code to keep the JoinHandle
returned by spawn(), call responder.abort() and then await the join handle
(e.g., await the JoinHandle returned by spawn()) to ensure the task has fully
shut down before the next iteration; apply the same fix wherever
responder.abort() is used (the occurrences near InitiatorNode::connect_to(addr)
/ wait_for_handshake() and the other two locations referenced) so each aborted
responder is awaited to completion.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ca98967f-c326-470a-b2fa-a1bd96346baa

📥 Commits

Reviewing files that changed from the base of the PR and between 9065695 and e41abbd.

📒 Files selected for processing (5)
  • pallas-network2/Cargo.toml
  • pallas-network2/benches/harness/chain.rs
  • pallas-network2/benches/harness/mod.rs
  • pallas-network2/benches/harness/node.rs
  • pallas-network2/benches/protocol.rs

Comment thread pallas-network2/benches/harness/node.rs
Comment thread pallas-network2/benches/protocol.rs
@scarmuega scarmuega merged commit 3951639 into main Apr 1, 2026
15 checks passed
@scarmuega scarmuega deleted the test/benches-network2 branch April 1, 2026 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant