-
Notifications
You must be signed in to change notification settings - Fork 53
Description
I spent the last few days trying to diagnose why reading 2 MiB over a USB serial port took just over 2 minutes on Windows (compared to 7.3 seconds on macOS for the same device and workload). 02:04.765 on my last test, in fact.
While troubleshooting, I discovered that the GetQueuedCompletionStatusEx call in mio takes 30 ms on average to return any data for a read, regardless of number of bytes returned.
It turns out the problem lies in the way mio-serial overrides the timeouts configured by serialport:
Lines 809 to 826 in 49b1c56
| pub(crate) fn override_comm_timeouts(handle: RawHandle) -> StdIoResult<()> { | |
| let mut timeouts = COMMTIMEOUTS { | |
| // wait at most 1ms between two bytes (0 means no timeout) | |
| ReadIntervalTimeout: 1, | |
| // disable "total" timeout to wait at least 1 byte forever | |
| ReadTotalTimeoutMultiplier: 0, | |
| ReadTotalTimeoutConstant: 0, | |
| // write timeouts are just copied from serialport-rs | |
| WriteTotalTimeoutMultiplier: 0, | |
| WriteTotalTimeoutConstant: 0, | |
| }; | |
| let r = unsafe { SetCommTimeouts(handle, &mut timeouts) }; | |
| if r == 0 { | |
| return Err(io::Error::last_os_error()); | |
| } | |
| Ok(()) | |
| } |
Or at least, fixing the long 30 ms delays is a matter of commenting all of that out. The complete 2 MiB read process completes on Windows in about 7.8 seconds, on par with macOS.
A side effect of removing this override is that I get spurious 0-byte reads. But I honestly prefer that to 1,600x slower reads! The cost of checking if the length is zero is basically free in comparison.
I'm patching this out in my local builds, but I don't know what you want to do about this ... feature ... for the crate. Making it configurable might be an OK solution, but then I would have to teach tokio-serial to configure it properly. IMHO, ditch this override thing completely.
Relevant discussions:
- Set Windows timeouts to enforce non-blocking read serialport/serialport-rs#79
- bufReader read_until not handled correctly on Windows serialport/serialport-rs#73 (comment)
- Set Windows timeouts to enforce non-blocking read #38
Quoting the latter comment, "extremely cursed" sounds pretty accurate.