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

SerialPort - high CPU usage #2379

Open
Tracked by #64596
michaldobrodenka opened this issue Jan 30, 2020 · 42 comments
Open
Tracked by #64596

SerialPort - high CPU usage #2379

michaldobrodenka opened this issue Jan 30, 2020 · 42 comments
Labels
area-System.IO.Ports enhancement Product code improvement that does NOT require public API changes/additions needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration os-linux Linux OS (any supported distro)
Milestone

Comments

@michaldobrodenka
Copy link

michaldobrodenka commented Jan 30, 2020

Tested on Linux. I have a embedded application where I have a separate thread with sync Read/Write operations in loop and after migrating from mono, this thread CPU usage went from 5% to 25%.

Typical use case for SerialPort like ModBus with small poll intervals, where serial port sends & recieive small packets many times per second make .NET Core Serial consume a lot of resources.

See:
https://stackoverflow.com/questions/54903630/net-core-3-0-system-io-ports-serialport-uses-5-10-cpu-on-rpi-all-the-time

@jkotas
Copy link
Member

jkotas commented Jan 30, 2020

cc @wfurt

@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Jan 30, 2020
@wfurt wfurt added the os-linux Linux OS (any supported distro) label Jan 30, 2020
@michaldobrodenka
Copy link
Author

When I look at the implementation, for every simple blocking read/write operation - multiple objects on heap are created: SerialStreamIORequest, CancellationTokenSource for timeout or ReadTask/WriteTasks.

When you consider high performance scenario, like modbus on 115200 line where you send and receive a few bytes like 50 times per second, pressure for GC and object creation is not really light.

Maybe we can improve code by checking in Read/Write if other IO operation is running and if not, complete synchronously. But then we still have to deal with timeout. I'm not an OS expert, but probably using linux 'select' would be more efficient than creating CancellationTokenSource object 100 times per second. But I understand that this is probably not a priority for now.

@JeremyKuhne JeremyKuhne removed the untriaged New issue has not been triaged by the area owner label Mar 5, 2020
@JeremyKuhne JeremyKuhne added this to the Future milestone Mar 5, 2020
@JeremyKuhne JeremyKuhne added the enhancement Product code improvement that does NOT require public API changes/additions label Mar 5, 2020
@krwq
Copy link
Member

krwq commented May 14, 2020

@michaldobrodenka it's not that simple, I agree we could reduce CTS creation here but I doubt this would reduce any high CPU usage or have any visible impact.

As for using select - we are currently using poll which in a sense is very similar but the problem is we cannot just wait until it timeouts because SerialPort was designed such that it also supports events which mess up the perf capabilities really bad (perhaps one solution would be to use different code when there are events vs when there are none). I suspect the error here is that we're waiting for data but we're not getting any and we're trading off quick response time with CPU usage here. I.e. if we slept here then OS will wait minimum of 10ms which would reduce any responsiveness, I suspect we should do some kind of SpinWait in a loop which would give us something in between looping and sleeping - this will need to be investigated a bit more to not regress any scenarios.

@michaldobrodenka
Copy link
Author

I solved this by extracting serial port from Mono. https://github.com/michaldobrodenka/System.IO.Ports.Mono

@jeroenwalter
Copy link

I'm using the serialport on a Rpi3 connected to an Arduino via usb. The arduino is sending continuous data to the serialport.
Even just reading the data without doing anything with it via the DataReceived event results in about 50 - 80 threads consumed by the serialport, all waiting for some other serialport thread. Data is being received seconds after they are sent.
The cpu load generated by this implementation is much too high for serious applications.
When I saw the implementation I was baffled by all the async/await stuff, even for reading a single byte.
Please consider completely rewriting this class, as now it is just a cpu hog and useless for serious data throughput use on rpi and similar boards.

@jeroenwalter
Copy link

jeroenwalter commented Jun 14, 2020

I solved this by extracting serial port from Mono. https://github.com/michaldobrodenka/System.IO.Ports.Mono

I've just implemented a DataReceived event for your implementation. cpu load is 2% on rpi vs 25% with the dotnet serialport implementation.
VM-size also doesn't keep growing, as it does with the dotnet implementation.

@janvorli
Copy link
Member

DataReceived event results in about 50 - 80 threads consumed by the serialport

This sounds pretty bad. One of the overheads (obviously not the only one) caused by this is that so many threads result in a lot of virtual memory consumption due to thread stacks. It is a problem especially on 32 bit systems where the virtual address space is very limited.
@krwq, @wfurt, do we understand why would so many threads be created?

@jeroenwalter
Copy link

jeroenwalter commented Jun 15, 2020

I can confirm that the virtual memory consumption of the application rose to about 1GB within a matter of minutes on the Rpi... using the dotnet serialport class that is.
The other implementation from michaldobrenka doesn't have this problem, VM size doesn't grow.

@jeroenwalter
Copy link

jeroenwalter commented Jun 15, 2020

@krwq, @wfurt, do we understand why would so many threads be created?

Just take a look at https://github.com/dotnet/runtime/blob/master/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Unix.cs

My guess is because every call to ReadByte(), which you typically do in a loop in the DataReceived event, results in a Task being created and awaited. This takes some time, in the meantime new data can arrive, which results in firing the DataReceived event via RaiseDataReceivedChars using a ThreadPool.QueueUserWorkItem.
Now, if you use a lock in the DataReceived handler, because you don't want to read the serialport from 2 threads simultaneously, then all other DataReceived events have to wait until you release the lock. This will make all those worker threads wait.

jeroenwalter added a commit to jeroenwalter/Arduino that referenced this issue Jul 10, 2020
- added FirmataSession class, this is a light version of ArduinoSession, that has better firmata message handling, doesn't use a queue. so can be used for Arduinos that constantly produce a shitload of messages without overflowing the queue.
- added System.IO.Ports.Mono, replaces the crappy Microsoft SerialPort implementation as that one uses too much cpu, threads and memory, see dotnet/runtime#2379
- extended the test program with some additional tests, like a raw dump of all incoming bytes for Arduinos that produce data autonomously. Also you can choose which serial port to use.
- code refactoring, replaced magic numbers with enums.
@sandervandegeijn
Copy link

sandervandegeijn commented Jan 31, 2021

I'm probably having the same issue, seeing 30-50% cpu = half a core on Linux. On Windows however with exactly the same code, cpu usage is doing next to nothing. There is a performance gap between my Linux box (nuc with 4 core celeron) and my Windows machine (I5 9600), but this is significant enough.

At first I suspected my own implementation due to many tasks, but refactored it, think that's okay now but still having issues. When I increase the load on the serial bus it gets worse.

I'm using easymodbustcp as the modbus lib for a .net5 console app, can anyone give me some pointers on how to swap out the default implementation withthe mono one?

Found another thread: https://stackoverflow.com/questions/59577676/c-sharp-serialport-multithread-send-receive-collisions

@sandervandegeijn
Copy link

sandervandegeijn commented Feb 2, 2021

Okay performance is definitely terrible on Linux... On Windows it's fine. How can we get this higher on the backlog, this is barely usable, even sending 3 modbusrequests / second at 9600bps is eating up a lot of cpu time while doing some basic string manipulations. I'm not capable of fixing this library myself.

@jeroenwalter
Copy link

Okay performance is definitely terrible on Linux... On Windows it's fine. How can we get this higher on the backlog, this is barely usable in, even sending 3 modbusrequests / second on 9600bps is eating up a lot of cpu time while doing some basic string manipulations. I'm not capable of fixing this library myself.

I ran into the same issues, also addressed them some posts back, but no feedback from the dotnet team unfortunately.
You may take a look in my solution in https://github.com/jeroenwalter/Arduino/tree/netcore30/System.IO.Ports.Mono
My Arduino solution uses an implementation that I found on https://github.com/michaldobrodenka/System.IO.Ports.Mono
It works well on the raspberry pi 3.

@janvorli
Copy link
Member

janvorli commented Feb 2, 2021

@wfurt is this something you can look into fixing?

@wfurt
Copy link
Member

wfurt commented Feb 2, 2021

possibly. I'm mostly focused on other areas right now but the work needed may not be too large. I'll chat with @krwq about plan.

It would be great if people could share simple projects so we have good baseline how this is used. I can make some guesses from the discussion but having some code easy to run and measure would be nice.

@hrocha1
Copy link

hrocha1 commented Feb 2, 2021

It seems like the common use case is Raspberry Pi communicating with microcontrollers. So the "minimum sample project" might probably be something like Arduino talking to Raspberry via serial? There is a decent chance that many people have that hardware available and are able to test potential changes.

@wfurt
Copy link
Member

wfurt commented Feb 2, 2021

I have few RPi and I used it with serial USB connected to my x64 VM. I'm mostly interested in coding patterns. For example ReadByte() can be greatly simplified but I'm not sure how much that matters.

@sandervandegeijn
Copy link

sandervandegeijn commented Feb 2, 2021

It seems like the common use case is Raspberry Pi communicating with microcontrollers. So the "minimum sample project" might probably be something like Arduino talking to Raspberry via serial? There is a decent chance that many people have that hardware available and are able to test potential changes.

Not only for those home/hobby projects. Serial is still relevant, especially for industrial applications. I'm using a Intel NUC as a modbus master with a CH340G/RS485 USB stick to communicate with multiple energy meters, solar inverters and charge controllers. Still in a home situation, but a lot of industrial appliances / sensors use Modbus RTU (which mainly runs over RS232/RS485).

@jeroenwalter
Copy link

jeroenwalter commented Feb 2, 2021

It seems like the common use case is Raspberry Pi communicating with microcontrollers. So the "minimum sample project" might probably be something like Arduino talking to Raspberry via serial? There is a decent chance that many people have that hardware available and are able to test potential changes.

I use my own .netcore fork of the SolidSoils Arduino library on a rpi3. The rpi is not sending commands to initiate data transfer. Instead the Arduino is constantly sending firmata messages to the rpi.
The SolidSoils code (my fork as well as the original SolidSoils lib) uses the ReadByte() method in the DataReceived event to parse firmata messages.
See the method SerialDataReceived() in https://github.com/jeroenwalter/Arduino/blob/netcore30/Solid.Arduino.Firmata/FirmataSession.cs
or in https://github.com/SolidSoils/Arduino/blob/master/Solid.Arduino/ArduinoSession.cs

For event driven serial communication like this, I think ReadByte() for a single byte should be as fast as possible, no async tasks or enqueued action or such, just read the byte. In this case you can only read 1 byte at once, as you don't know how many bytes are available and can't wait for extra bytes to arrive before processing a complete firmata message.

@sandervandegeijn
Copy link

sandervandegeijn commented Feb 7, 2021

Okay, on Windows with .net 5.0 it isn't fully stable either. Although I'm not noticing the high cpu usage on my Windows 10 boxes it does trigger a dirty error;

Unhandled exception. info: Modbus2Mqtt.Eventing.ModbusResult.ModbusRegisterResultHandler[0]
      Result for: eastron_test register: reactive power : 0
System.NullReferenceException: Object reference not set to an instance of an object.
   at System.IO.Ports.SerialStream.EventLoopRunner.CallReceiveEvents(Object state)
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at System.IO.Ports.SerialStream.EventLoopRunner.CallReceiveEvents(Object state)
   at System.Threading.ThreadPoolWorkQueue.Dispatch()

I can reproduce this quite easily.

@Priyantha
Copy link

Okay, on Windows with .net 5.0 it isn't fully stable either. Although I'm not noticing the high cpu usage on my Windows 10 boxes it does trigger a dirty error;

Unhandled exception. info: Modbus2Mqtt.Eventing.ModbusResult.ModbusRegisterResultHandler[0]
      Result for: eastron_test register: reactive power : 0
System.NullReferenceException: Object reference not set to an instance of an object.
   at System.IO.Ports.SerialStream.EventLoopRunner.CallReceiveEvents(Object state)
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at System.IO.Ports.SerialStream.EventLoopRunner.CallReceiveEvents(Object state)
   at System.Threading.ThreadPoolWorkQueue.Dispatch()

I can reproduce this quite easily.

During running the same piece of tooling, in this case something to communicate with Modbus, I am experiencing the same issue as @ict-one-nl.

@adamsitnik adamsitnik added this to the 7.0.0 milestone Jul 22, 2021
@dlukach
Copy link

dlukach commented Oct 5, 2021

is there any updates on this? This is really impacting a commercial product I'm developing. Tried to use the mono ports posted above but it is locking up on the reading of a serial line. The .net 5 implementation of ports is killing my RPI from functioning properly. If there is any way my team can assist in expediting this issue please let me know.

@ghost ghost added the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Oct 5, 2021
@dlukach
Copy link

dlukach commented Oct 5, 2021

So the issue with the mono port, it defaults to "\r\n" for new line when it should be "\n" like the .net implementation. I didn't look why since the default value is "\n" yet the private var doesn't have that set

@krwq
Copy link
Member

krwq commented Oct 5, 2021

@dlukach for \r\n issue please create a new issue so this won't get missed. As for updates... well I didn't really have time to pick this up 😞

@sandervandegeijn
Copy link

Now that .net6 has left the building (great job!) can you guys to spare some time to fix this? It would be more correct to simply drop serial port as it is now entirely until it's fixed than leaving as it is now.

@sandervandegeijn
Copy link

I really hope this gets fixed in 7, basically you could throw a notimplemented exception tbh ;)

@markus-fischbacher
Copy link

I'm facing the same bad performance on my Linux embedded system. I have to deal with two UARTs in parallel. CPU usage approx. 80%. On MacOS it's working better (maybe because of more CPU power). I will try the Mono implementation.

@sicode
Copy link

sicode commented Aug 23, 2022

Okay, on Windows with .net 5.0 it isn't fully stable either. Although I'm not noticing the high cpu usage on my Windows 10 boxes it does trigger a dirty error;

Unhandled exception. info: Modbus2Mqtt.Eventing.ModbusResult.ModbusRegisterResultHandler[0]
      Result for: eastron_test register: reactive power : 0
System.NullReferenceException: Object reference not set to an instance of an object.
   at System.IO.Ports.SerialStream.EventLoopRunner.CallReceiveEvents(Object state)
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at System.IO.Ports.SerialStream.EventLoopRunner.CallReceiveEvents(Object state)
   at System.Threading.ThreadPoolWorkQueue.Dispatch()

I can reproduce this quite easily.

Yes,When Cpu uage 90% ,100% crash

@Alex-111
Copy link

If there is no fix available in the near future, I suggest to include the mono code instead of the current one….

@kasperk81
Copy link
Contributor

@Alex-111 try the latest one https://www.nuget.org/packages/System.IO.Ports#versions-body-tab (8 preview3)

@Alex-111
Copy link

@kasperk81
Why do you suggest this? Can you explain, what has been changed to reduce CPU usage?

@jeroenwalter
Copy link

@kasperk81 Why do you suggest this? Can you explain, what has been changed to reduce CPU usage?

https://github.com/dotnet/runtime/blob/main/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Unix.cs

As far as I can see, the implementation didn't change much and is still using Tasks and queuing threads anytime a byte arrives on the serial port.
So my guess is that nothing has been improved performance-wise on a Raspberry Pi.
Would be nice if the .net team does some performance testing / benchmarking vs old Mono implementation on non-Windows platforms.

@sandervandegeijn
Copy link

I gave up on this, it's completely unusable. Throwing a not implemented exception would be cleaner than leaving this **** in.

@michaldobrodenka
Copy link
Author

If you want I can share mono serial port implementation rewritten to pure c# + libc dllimport. (so it's not dependent on monoposixhelper native libraries)

@schotime
Copy link

schotime commented Jun 5, 2023

@michaldobrodenka This would be fantastic. I'm struggling with deadlocks bigtime with the MS one.

@michaldobrodenka
Copy link
Author

@schotime this is source I use heavily on low end ARM machines (I made some namespace changes, hope it will work :) ) https://github.com/michaldobrodenka/Mono.IO.Ports-managed

@schotime
Copy link

schotime commented Jun 7, 2023

@michaldobrodenka It works!!!! Thankyou! I'm using it on ARM PI like devices. Hopefully these rotten errors can be gone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.IO.Ports enhancement Product code improvement that does NOT require public API changes/additions needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration os-linux Linux OS (any supported distro)
Projects
None yet
Development

No branches or pull requests