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

handle return value of read() correctly #44

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ishiguroJSK
Copy link
Contributor

I found out this node is using 100% of 1 thread, and then I found this section has the cause of that.

dynpick_driver/src/main.cpp

Lines 100 to 114 in 507cdab

bool readCharFromSocket(const int& fdc, const int& length, char* reply){
int len = 0;
int c = 0;
while ( len < length ) {
c = read(fdc, reply+len, length-len);
if (c >= 0) {
len += c;
} else {
ROS_DEBUG("=== need to read more data ... n = %d (%d) ===", c, len);
continue;
}
}
// This could actually check if data was received and may return on timeout with false
return true;
}

Because this socket is opened as NONBLOCK, and read() returns "0" when there is no new received data (-1 for some error), this section is spinning so rapid unnecessarily like below (I inserted debug print).

[ INFO] [1566116808.497431092]: c=0, len=0, length=27
[ INFO] [1566116808.497445567]: c=0, len=0, length=27
[ INFO] [1566116808.497458347]: c=0, len=0, length=27
[ INFO] [1566116808.497474636]: c=27, len=27, length=27
[ INFO] [1566116808.497504326]: c=0, len=0, length=27
[ INFO] [1566116808.497519655]: c=0, len=0, length=27
[ INFO] [1566116808.497534684]: c=0, len=0, length=27
[ INFO] [1566116808.497550536]: c=0, len=0, length=27
[ INFO] [1566116808.497565303]: c=0, len=0, length=27
[ INFO] [1566116808.497578473]: c=0, len=0, length=27
[ INFO] [1566116808.497593122]: c=0, len=0, length=27
[ INFO] [1566116808.497605761]: c=0, len=0, length=27
[ INFO] [1566116808.497622420]: c=0, len=0, length=27
[ INFO] [1566116808.497635082]: c=0, len=0, length=27
[ INFO] [1566116808.497649813]: c=0, len=0, length=27
[ INFO] [1566116808.497664944]: c=0, len=0, length=27
[ INFO] [1566116808.497679948]: c=0, len=0, length=27
[ INFO] [1566116808.497694495]: c=0, len=0, length=27
[ INFO] [1566116808.497708893]: c=0, len=0, length=27
[ INFO] [1566116808.497723325]: c=0, len=0, length=27
[ INFO] [1566116808.497737710]: c=0, len=0, length=27
[ INFO] [1566116808.497751946]: c=0, len=0, length=27
[ INFO] [1566116808.497766212]: c=0, len=0, length=27
[ INFO] [1566116808.497780605]: c=0, len=0, length=27
[ INFO] [1566116808.497794989]: c=0, len=0, length=27
[ INFO] [1566116808.497809211]: c=0, len=0, length=27
[ INFO] [1566116808.497823536]: c=0, len=0, length=27
[ INFO] [1566116808.497838065]: c=0, len=0, length=27
[ INFO] [1566116808.497852595]: c=0, len=0, length=27
[ INFO] [1566116808.497865277]: c=0, len=0, length=27
[ INFO] [1566116808.497879729]: c=0, len=0, length=27
[ INFO] [1566116808.497894314]: c=0, len=0, length=27
[ INFO] [1566116808.497908938]: c=0, len=0, length=27
[ INFO] [1566116808.497923482]: c=0, len=0, length=27
[ INFO] [1566116808.497937884]: c=0, len=0, length=27
[ INFO] [1566116808.497952361]: c=0, len=0, length=27
[ INFO] [1566116808.497966956]: c=0, len=0, length=27
[ INFO] [1566116808.497981353]: c=0, len=0, length=27
[ INFO] [1566116808.497995635]: c=0, len=0, length=27
[ INFO] [1566116808.498009962]: c=0, len=0, length=27
[ INFO] [1566116808.498024313]: c=0, len=0, length=27
[ INFO] [1566116808.498038630]: c=0, len=0, length=27
[ INFO] [1566116808.498052902]: c=0, len=0, length=27
[ INFO] [1566116808.498067295]: c=0, len=0, length=27
[ INFO] [1566116808.498080885]: c=0, len=0, length=27
[ INFO] [1566116808.498095530]: c=0, len=0, length=27
[ INFO] [1566116808.498108360]: c=0, len=0, length=27
[ INFO] [1566116808.498122606]: c=0, len=0, length=27
[ INFO] [1566116808.498137254]: c=0, len=0, length=27
[ INFO] [1566116808.498151487]: c=0, len=0, length=27
[ INFO] [1566116808.498164403]: c=0, len=0, length=27
[ INFO] [1566116808.498178861]: c=0, len=0, length=27
[ INFO] [1566116808.498194524]: c=0, len=0, length=27
[ INFO] [1566116808.498210608]: c=0, len=0, length=27
[ INFO] [1566116808.498226475]: c=0, len=0, length=27
[ INFO] [1566116808.498239336]: c=0, len=0, length=27
[ INFO] [1566116808.498253562]: c=0, len=0, length=27
[ INFO] [1566116808.498271000]: c=0, len=0, length=27
[ INFO] [1566116808.498283778]: c=0, len=0, length=27
[ INFO] [1566116808.498298645]: c=0, len=0, length=27
[ INFO] [1566116808.498316287]: c=0, len=0, length=27
[ INFO] [1566116808.498331088]: c=0, len=0, length=27
[ INFO] [1566116808.498345495]: c=0, len=0, length=27
[ INFO] [1566116808.498359940]: c=0, len=0, length=27
[ INFO] [1566116808.498375037]: c=0, len=0, length=27
[ INFO] [1566116808.498389499]: c=0, len=0, length=27
[ INFO] [1566116808.498403975]: c=0, len=0, length=27
[ INFO] [1566116808.498418343]: c=0, len=0, length=27
[ INFO] [1566116808.498431096]: c=0, len=0, length=27
[ INFO] [1566116808.498445626]: c=0, len=0, length=27
[ INFO] [1566116808.498459254]: c=0, len=0, length=27
[ INFO] [1566116808.498475007]: c=27, len=27, length=27
[ INFO] [1566116808.498505095]: c=0, len=0, length=27

I think polling at 10kHz is enough to run dynpick at 1kHz.
With this PR, this node is running with 10~30% of single thread usage.
How about like this implement?

@k-okada
Copy link
Member

k-okada commented Dec 21, 2021

@SoichiTaniguchi if you have real hardware device and free time, please check if this works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants