mio icon indicating copy to clipboard operation
mio copied to clipboard

Add NamedPipe::set_buffer_size

Open Thomasdezeeuw opened this issue 2 years ago • 3 comments

Allows the user to control the size of the buffers used.

@vadixidav, @mlsvrts coul you try this branch. Specifically can you try a 1 >= byte write, followed by a small read, because after reading the code a bit more I'm not sure this will solve your problem(s).

Thomasdezeeuw avatar Aug 14 '22 16:08 Thomasdezeeuw

@vadixidav, @mlsvrts either of you try this? Did it fix your issues?

Thomasdezeeuw avatar Sep 10 '22 16:09 Thomasdezeeuw

I can give it a try right now.

vadixidav avatar Sep 10 '22 17:09 vadixidav

I can confirm that this works @Thomasdezeeuw. I made this change to tokio: https://github.com/vadixidav/tokio/commit/798a71f3934b7c470bc1ff4594cb09d38440b069

This just sets every NamedPipe on windows to have a buffer size of 1.

I patched my local application and I immediately saw latencies go from 2 seconds down to imperceptible levels.

[patch.crates-io]
mio = { git = 'https://github.com/Thomasdezeeuw/mio.git', branch = 'issue#1582_named_pipe_set_buffer_size' }
tokio = { git = 'https://github.com/vadixidav/tokio.git', branch = 'mio#1608_hack' }

For clarification, I am only performing reads, no writes.

vadixidav avatar Sep 10 '22 20:09 vadixidav

I got around to testing this and can also improve performance with this change!

mlsvrts avatar Dec 07 '22 04:12 mlsvrts

Since this hasn't seen any activity and it's likely not the API we want I'm going to close this.

Thomasdezeeuw avatar Aug 29 '23 13:08 Thomasdezeeuw

@Thomasdezeeuw There is still a bug currently when using serial ports on windows due to this issue. I would still propose that we change the default buffer size to 1, otherwise async/await operations don't work properly on windows. Essentially, on windows, even though you have received data, it is not given to you until a waiting period that is very long, and this makes it unusable for all use cases I have used it for thus far. Should we open a new ticket to track this bug?

vadixidav avatar Aug 29 '23 20:08 vadixidav

There is another PR over here https://github.com/berkowski/mio-serial/pull/38. Perhaps this is a better solution to the problem, but I am not sure if it is the right place to fix the issue or not. Any thoughts?

vadixidav avatar Aug 29 '23 20:08 vadixidav

@Thomasdezeeuw There is still a bug currently when using serial ports on windows due to this issue. I would still propose that we change the default buffer size to 1, otherwise async/await operations don't work properly on windows. Essentially, on windows, even though you have received data, it is not given to you until a waiting period that is very long, and this makes it unusable for all use cases I have used it for thus far. Should we open a new ticket to track this bug?

I don't think a buffer size of 1 makes sense for other cases of NamedPipe. But indeed let's open a ticket to keep track of this.

There is another PR over here berkowski/mio-serial#38. Perhaps this is a better solution to the problem, but I am not sure if it is the right place to fix the issue or not. Any thoughts?

We can try something like that. I don't have enough experience with Windows's named pipes to provide any useful insights though.

Thomasdezeeuw avatar Aug 30 '23 07:08 Thomasdezeeuw