jcurl / RJCP.DLL.SerialPortStream

SerialPortStream is an independent implementation of System.IO.Ports.SerialPort and SerialStream for better reliability and maintainability. Default branch is 2.x and now has support for Mono with help of a C library.
Microsoft Public License
614 stars 196 forks source link

Synchronous performance is slower than .NET SerialPort #103

Closed leocb closed 4 years ago

leocb commented 4 years ago

Hi, first of all, thanks for this lib, reliability is greater and CPU usage is lower, great work! However, i noticed that the performance when doing several small packets synchronously is slower than .NET default SerialPort

My setup is as follows:

  1. Program send 64 bytes to device and blocks the thread with a AutoresetEvent.WaitOne
  2. Device writes those 64 bytes to the flash memory and sends an ACK\n back to the program
  3. Program receive data on the serial using the DataReceived event, validates the ACK\n and unblocks the Thread by AutoresetEvent.Set()
  4. Repeat until done

Comparison - this is the same code, device and firmware on both cases, the only thing that's changed is the serial port: SerialPortStream: b15009e5-d70a-4858-8171-e90d03a02ec3

.NET SerialPort net

jcurl commented 4 years ago

Thanks for the report. I don't expect the SerialPortStream to be faster than Microsoft's solution. Microsoft write to the device driver direct using asynchronous I/O buffers. They're implementation is closer to the driver layer than SerialPortStream.

SerialPortStream implements a buffer on top, to hide away limitations of some drivers (e.g. FTDI allows for large buffers, but PL2303 at the time this library was written would fail). Because of that, there is already a layer in-between. When you write to the SerialPortStream, it writes to a local ring buffer. The I/O thread waits for data with its own ManualResetEvent and does an async operation to the underlying driver. When the Windows driver is finished, it indicates this to the main thread.

Given how the SerialPortStream works, you could just read and write on your own thread, and get rid of using an event. To speed up the transfer, you might consider modifying the sources to increase the priority of the I/O thread on Windows.

leocb commented 4 years ago

Thanks for the tips, I'll try some solutions and report back. From my testing and profiling, the biggest delay happens between the device send and the SerialPortStream detecting that data, it was 14ms in average. my best guess would be increasing the I/O thread priority

A colleague from where I work said he got faster results when he simply switched from SerialPort to your lib - from 0.5Mbps to 2.4Mbps over USB CDC (same as me). I'll take a look at his implementation too.

When you say

you could just read and write on your own thread

Could you elaborate on where I'd hook it to? Are you suggesting I create a pooling thread on SerialPortStream ReadByte?

Thanks again 😃

jcurl commented 4 years ago

Assuming that you're blocking on your own ManualResetEvent for an event to occur, you might want to block on the Read itself.

Your colleague may be getting higher rates as all read/write is just a buffer operation, and every i/o loop just looks at what is pending to read/write.

The delays you're seeing are probably thread changes.

leocb commented 4 years ago

Blocking directly on the Read() is at least twice as fast, but still slower than SerialPort. My colleague's protocol is fundamentally different from mine, he does bulk async send/receive, so there's always a lot of buffer available for him to process. I'm having a bad time trying to compile from source, is there instructions somewhere? I'd like to try a few things on it before deciding on reverting back to the SerialPort.

jcurl commented 4 years ago

I would expect you can just include the csproj into your solution file.

leocb commented 4 years ago

Oh yeah, nvm. Visual Studio 2019 was being dumb about the nuget package 😆

leocb commented 4 years ago

Just a report on my progress: By Disabling PL2303_WORKAROUNDS I stopped receiving DataReceived events (bug?) with no obvious errors. However, by leaving it enabled and changing the following settings on line 220 of CommOverlappedIo.cs

timeouts.ReadIntervalTimeout = System.Threading.Timeout.Infinite;
timeouts.ReadTotalTimeoutConstant = 0;
timeouts.ReadTotalTimeoutMultiplier = 0;

I'm able to get transfers speeds with events as fast as blocking the Thread on the Read() - But still not as fast as SerialPort

for now I'll be reverting my solution to SerialPort, but I'll keep analyzing the possibility to migrate to yours.

jcurl commented 4 years ago

I'll be closing this ticket, as the original problem is talking about performance. I would expect the original .NET implementation to be more performant, as it's using callbacks from the driver direct with Threaded I/O directly to the buffers the user provides.

The SerialPortStream implementation tries to abstract that, as different drivers behave differently depending on the sizes of the buffers given for read/write, and can block on small buffers. So less performance is a tradeoff for making the usage a little more generic.