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

High CPU usage #2

Closed
Anessen97 opened this issue Sep 4, 2019 · 14 comments
Closed

High CPU usage #2

Anessen97 opened this issue Sep 4, 2019 · 14 comments
Labels
invalid This doesn't seem right

Comments

@Anessen97
Copy link

I started using SimpleCom in order to connect to my serial devices, and I've gotta say that it's really cool to be able to use it inside Windows Terminal.
The only problem I found is that it's using a lot of my CPU and I've no idea if it's a widespread issue.

@YaSuenag
Copy link
Owner

YaSuenag commented Sep 4, 2019

Can you share more details?

SimpleCom would consume a lot of CPU time when a lot of chars are shown. I want to know the issue is saw when idle state on serial line.

@Anessen97
Copy link
Author

When idle it uses up to 25% of CPU, but it usually hovers around 20%.
Thank you for the quick response though.

@YaSuenag
Copy link
Owner

YaSuenag commented Sep 6, 2019

Hmm... I could not reproduce it.

taskmgr

What device did you connect via SimpleCom? The traffic is streaming on serial line like a sensor data?

@Anessen97
Copy link
Author

Screenshot (17)
The serial line is currently idle and connected to a La Frite SBC.

@YaSuenag
Copy link
Owner

YaSuenag commented Sep 6, 2019

I've not yet reproduce this issue on my Raspberry Pi 2.

Can you share thread stack via Process Explorer?

  1. Find SimpleCom process
  2. Right click -> choose "Properties"
  3. Chose "Threads" tab
  4. Select thread name and click "Stack" button

Usually 2 threads are available in Threads tab. I want to check how SimpleCom works on your PC.
Of course, you can use / debug on Visual Studio.

@Anessen97
Copy link
Author

Screenshot (18)
Screenshot (19)
These are the threads that Process Explorer shows me.
Thank you so much for your time, let me know if I forgot something.

@YaSuenag YaSuenag added the invalid This doesn't seem right label Sep 6, 2019
@YaSuenag
Copy link
Owner

YaSuenag commented Sep 6, 2019

Thank you for sharing! But it is not a bug, so I close this issue.

Thread 2812 consumes a lot of CPU time. This thread is cause of high CPU usage.
This thread is for stdout redirector - transfer stdout on serial device to the console on PC. Serial device might send unprintable characters, e.g. escape sequence. So I think your La Frite (or OS on your SBC) might send many data on serial line, and it could not handle SimpleCom because they are unprintable.

Thus I close this issue - you need to investigate your La Frite or its OS.

@YaSuenag YaSuenag closed this as completed Sep 6, 2019
@ngbrown
Copy link
Contributor

ngbrown commented Mar 29, 2021

This is happening to me as well, even on a newly connected port, with no characters sent or received. The SimpleCom.exe always takes one CPU worth of processing (100%/8=12%).

Screenshot - 2021-03-29_15-02-26

I started it from within Windows Terminal. The same CPU usage happens when launched directly.

@YaSuenag
Copy link
Owner

I've not been able to reproduce it yet with my Raspberry Pi 4 (w/ Raspberry Pi OS for AArch64).

This is happening to me as well, even on a newly connected port, with no characters sent or received.

Can you prove it? e.g. insert debug code in SimpleCom.
What device did you use? I guess SimpleCom receives unprintable chars. (e.g. control chars)

@ngbrown
Copy link
Contributor

ngbrown commented Mar 31, 2021

Hello, at first I thought you were asking what remote device the serial port was connected to, and that doesn't make a difference. It doesn't have to be connected to anything. But then maybe you are actually asking what is the USB device that is providing the COM port?

The USB to serial cable is an FTDI chip based StarTech ICUSB2321F:

Device Friendly Name: USB Serial Port (COM3)
Device Instance ID: FTDIBUS\VID_0403+PID_6001+AH063JZ9A\0000
Device Manufacturer: FTDI
Provider Name: FTDI
Driver Date: 8-16-2017
Driver Version: 2.12.28.0

The same CPU usage does not occur when using Tera Term, PuTTY, or MTTTY.

@YaSuenag
Copy link
Owner

Hello, at first I thought you were asking what remote device the serial port was connected to, and that doesn't make a difference. It doesn't have to be connected to anything. But then maybe you are actually asking what is the USB device that is providing the COM port?

I want to know what device did you communicate on serial line because peripheral device (or its terminal) might not support VT100. It relates to #6 .
I tested SimpleCom with both Raspberry Pi 4 with USB OTG and Raspberry Pi 2 with TTL-232R-3V3 (FTDI FT232R), but I haven't seen similar issue.

If you can modify SimpleCom, I want to know what characters were sent from peripheral. I can help you to debug.

@ngbrown
Copy link
Contributor

ngbrown commented Apr 5, 2021

It started occurring again, and I profiled it:

Screenshot - 2021-04-05_13-30-18

In StdOutRedirector, ReadFile is immediately returning with a TRUE but with no bytes to read. So it's being rapidly called in a loop.

I've narrowed it down to not setting any COMMTIMEOUTS values. When I run GetCommTimeouts, it came back with a ReadIntervalTimeout=0xffffffff, which I think means return immediately, even if no bytes have been received (docs). Other serial port programs seems to reset that value when they exit. (At least Tera Term ran and exited before SimpleCom runs resets the value).

Also, there are a lot of Win32 API calls with no error checking or handling. For example the call SetupComm(hSerial, 1, 1) was actually erroring. A value of 256 instead of 1 seems to not error. I'm not sure how you want to handle lots of error checking, so in my next bit of code, I just send it to the debug console...

Adding something like this after SetupComm should fix this high CPU issue:

	COMMTIMEOUTS comm_timeouts;
	if (!GetCommTimeouts(hSerial, &comm_timeouts))
	{
		DWORD last_error = GetLastError();
		OutputDebugString(_T("GetCommTimeouts failed.\r\n"));
	}

	comm_timeouts.ReadIntervalTimeout = 1;
	comm_timeouts.ReadTotalTimeoutMultiplier = 0;
	comm_timeouts.ReadTotalTimeoutConstant = 10;
	comm_timeouts.WriteTotalTimeoutMultiplier = 0;
	comm_timeouts.WriteTotalTimeoutConstant = 0;
	
	if (!SetCommTimeouts(hSerial, &comm_timeouts))
	{
		DWORD last_error = GetLastError();
		OutputDebugString(_T("SetCommTimeouts failed.\r\n"));
	}

In StdOutRedirector, the use of a buffer (say 4KiB) would be more efficient when reading and writing bursts of characters. With the timeout, it would still be responsive with one character.

@YaSuenag
Copy link
Owner

YaSuenag commented Apr 6, 2021

Thanks @ngbrown for sharing details!

I've narrowed it down to not setting any COMMTIMEOUTS values. When I run GetCommTimeouts, it came back with a ReadIntervalTimeout=0xffffffff, which I think means return immediately, even if no bytes have been received (docs). Other serial port programs seems to reset that value when they exit. (At least Tera Term ran and exited before SimpleCom runs resets the value).

I wonder why my Raspberry Pi 4 does not happen it... The behavior might be different in each devices (it might not only peripheral - includes serial adaptor on Windows host).
Anyway to reset timeout value sounds good. Can you send PR?

Also, there are a lot of Win32 API calls with no error checking or handling. For example the call SetupComm(hSerial, 1, 1) was actually erroring. A value of 256 instead of 1 seems to not error. I'm not sure how you want to handle lots of error checking, so in my next bit of code, I just send it to the debug console...

Basically I want to throw WinAPIException when we get error from Windows API, then exit SimpleCom when the exception is caught.
To be honest I thought they (SetupComm() and so on) might not be failed, so I elided their error checks. I agree with you to add error checks to them of course.

Adding something like this after SetupComm should fix this high CPU issue:

	COMMTIMEOUTS comm_timeouts;
	if (!GetCommTimeouts(hSerial, &comm_timeouts))
	{
		DWORD last_error = GetLastError();
		OutputDebugString(_T("GetCommTimeouts failed.\r\n"));
	}

	comm_timeouts.ReadIntervalTimeout = 1;
	comm_timeouts.ReadTotalTimeoutMultiplier = 0;
	comm_timeouts.ReadTotalTimeoutConstant = 10;
	comm_timeouts.WriteTotalTimeoutMultiplier = 0;
	comm_timeouts.WriteTotalTimeoutConstant = 0;
	
	if (!SetCommTimeouts(hSerial, &comm_timeouts))
	{
		DWORD last_error = GetLastError();
		OutputDebugString(_T("SetCommTimeouts failed.\r\n"));
	}

I think ReadIntervalTimeout can be zero because we expect it behaves like stdout. Is it right?

In StdOutRedirector, the use of a buffer (say 4KiB) would be more efficient when reading and writing bursts of characters. With the timeout, it would still be responsive with one character.

I saw some incorrect behavior when I use buffer at there, but I cannot remember details.
It might relates multibyte chars (e.g. Kanji in Japanese, VT escape sequences). However, it sounds good as far as I can see now...

@YaSuenag
Copy link
Owner

YaSuenag commented May 8, 2021

This issue has been closed, and @ngbrown has suggested some improvements in this issue. So I separated them to #8, #9, #10. I want to discuss for them in each issues. Contributions are welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

3 participants