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

perf(net): improve overall multiplex performance for rlpx satellite stream #13861

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

hoank101
Copy link
Contributor

Closes #13856

@hoank101 hoank101 marked this pull request as draft January 18, 2025 16:01
@hoank101 hoank101 marked this pull request as ready for review January 18, 2025 16:24
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

cool, thanks for picking this up.

this already looks good.

still have some nits, and we can now remove the poll_ready + send calls from the stream impl entirely

crates/net/eth-wire/src/multiplex.rs Outdated Show resolved Hide resolved
crates/net/eth-wire/src/multiplex.rs Outdated Show resolved Hide resolved
crates/net/eth-wire/src/multiplex.rs Outdated Show resolved Hide resolved
@mattsse mattsse added C-perf A change motivated by improving speed, memory usage or disk footprint A-networking Related to networking in general labels Jan 18, 2025
@hoank101
Copy link
Contributor Author

thank @mattsse for review, i just updated

@hoank101 hoank101 requested a review from mattsse January 19, 2025 02:38
@hoank101
Copy link
Contributor Author

also help me review this PR @mattsse

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

no that we do this on the sink impl, we can remove

loop {
match this.inner.conn.poll_ready_unpin(cx) {
Poll::Ready(Ok(())) => {
if let Some(msg) = this.inner.out_buffer.pop_front() {
if let Err(err) = this.inner.conn.start_send_unpin(msg) {
return Poll::Ready(Some(Err(err.into())))
}
} else {
break
}
}
Poll::Ready(Err(err)) => {
if let Err(disconnect_err) =
this.inner.conn.start_disconnect(DisconnectReason::DisconnectRequested)
{
return Poll::Ready(Some(Err(disconnect_err.into())))
}
return Poll::Ready(Some(Err(err.into())))
}
Poll::Pending => {
conn_ready = false;
break
}
}
}

crates/net/eth-wire/src/multiplex.rs Show resolved Hide resolved
@hoank101 hoank101 requested a review from mattsse January 22, 2025 14:35
@hoank101 hoank101 changed the title perf: improve overall multiplex performance for rlpx satellite stream perf(net): improve overall multiplex performance for rlpx satellite stream Jan 30, 2025
@hoank101
Copy link
Contributor Author

seem look good, please help me review @mattsse

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-networking Related to networking in general C-perf A change motivated by improving speed, memory usage or disk footprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve overall multiplex performance for rlpx satellite stream
2 participants