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

On poll_flush timing of RPC client and server #384

Closed
0x5459 opened this issue Nov 9, 2022 · 3 comments
Closed

On poll_flush timing of RPC client and server #384

0x5459 opened this issue Nov 9, 2022 · 3 comments

Comments

@0x5459
Copy link

0x5459 commented Nov 9, 2022

Hi,

Big fan of this repo.

I found that each time a write request to transport in RPC client, the poll_flush method will be called by the pump_write of RequestDispatch. This approach may not be cache friendly. I wrote a quick demo using start_send in a loop to mimic multiple requests and then pull_flush all these requests in one try.

In my demo, pump_write no longer writes one request at a time but it's writing requests all the time. Same with pump_read. If you think this is the right approach, I'm more than happy to submit a PR for this.

@tikue
Copy link
Collaborator

tikue commented Nov 9, 2022

Hi! Thanks for the kind words 😊

Thank you for pointing this out! I think it's a great suggestion to flush less frequently. Please send a PR!

@tikue
Copy link
Collaborator

tikue commented Nov 9, 2022

Looking more closely, I think this is the problem line: https://github.com/0x5459/ext-proc-channel/blob/85a8f275d0e0ff3e517d1a96bef5ab3ac27a89e4/src/client.rs#L269 because it doesn't check if the transport is ready for a request to be buffered.

start_send returns an Err when the write buffer is full, which means we need to flush, right? Otherwise the channel could spuriously fail.

The ensure_writeable fn doesn't always flush; it tries to only flush when the transport's buffer is full.

@0x5459
Copy link
Author

0x5459 commented Nov 10, 2022

Looking more closely, I think this is the problem line: https://github.com/0x5459/ext-proc-channel/blob/85a8f275d0e0ff3e517d1a96bef5ab3ac27a89e4/src/client.rs#L269 because it doesn't check if the transport is ready for a request to be buffered.

start_send returns an Err when the write buffer is full, which means we need to flush, right? Otherwise the channel could spuriously fail.

The ensure_writeable fn doesn't always flush; it tries to only flush when the transport's buffer is full.

I'm sorry. You're right. Great code!

@0x5459 0x5459 closed this as completed Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants