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

Modifying the read request size of NamedPipe #1582

Closed
mlsvrts opened this issue Jun 16, 2022 · 8 comments
Closed

Modifying the read request size of NamedPipe #1582

mlsvrts opened this issue Jun 16, 2022 · 8 comments
Milestone

Comments

@mlsvrts
Copy link

mlsvrts commented Jun 16, 2022

I've been running into some performance issues reading intermittent byte streams from serial ports on windows. I have detailed the exact issue here:

berkowski/tokio-serial#55

I can fix the performance issue (for my use-case) by modiyfing named_pipe.rs, to force read requests of only 1-byte. This seems to work well, and emulates the behavior of PuTTy.

Is it possible to modify the read request size in some way that doesn't require modifying the library source? This would allow mio-serial and tokio-serial to nicely expose the same feature, which could lead to some serious performance improvements -- particularly if the end application is decoding packets of some fixed length.

@Thomasdezeeuw
Copy link
Collaborator

@mlsvrts am I correct in saying that the problem here is not necessarily the buffer size, but that the read is not completed the moment data is available? Perhaps their are flags we can use to return the moment any data is available.

@mlsvrts
Copy link
Author

mlsvrts commented Jun 17, 2022

@Thomasdezeeuw Yes, I agree that's a correct interpretation. Actually the performance would probably be better that way as well; in the case of USB to Serial adapters, for instance, I think you could receive entire blocks of data at the same time.

@vadixidav
Copy link

I am running up against this issue as well. I am trying to reasonably timestamp messages coming over a serial port, but with the current situation a whole buffer of messages comes in about once every half a second due to the buffering happening in mio. I think it would be an easy fix to just change the value to 1 to meet the expectations of the API while we wait for a more optimal solution. I am able to get something working using the synchronous serialport crate, but not with async currently due to this. For others, as a workaround I recommend switching to synchronous APIs and spawning a blocking thread for them. Make sure your BufReader has only 1 byte capacity or it wont work.

@Thomasdezeeuw
Copy link
Collaborator

@vadixidav, @mlsvrts would a NamedPipe::set_buffer_size function, changing the buffer size used in

self.pool.lock().unwrap().get(4 * 1024)
, fix this issue?

@mlsvrts
Copy link
Author

mlsvrts commented Aug 14, 2022

@Thomasdezeeuw Yes, I think that's the most straightforward solution; mio-serial and other libraries should be able to use that to control the buffering characteristics for read requests. In the default case, they can set the value to 1, and async applications will be notified whenever a new byte is ready.

I think dynamic modification also allows a common pattern where an application might read a header of some fixed length, and then read a body of length indicated by the header.

@Thomasdezeeuw
Copy link
Collaborator

Can anyone give #1608 a try?

@Thomasdezeeuw
Copy link
Collaborator

Closing this due to inactivity. It seems people want this, but no one is willing to put in the work.

Also see related pr #1778, which might help with larger buffers.

@vadixidav
Copy link

I still use my workaround fork for this. I might actually need this very soon, so I will reopen one if I make the proper changes.

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

No branches or pull requests

3 participants