Skip to content

perf(uucore): fix bottleneck on hot loop#9101

Closed
Alonely0 wants to merge 1 commit intouutils:mainfrom
Alonely0:opt_timeout
Closed

perf(uucore): fix bottleneck on hot loop#9101
Alonely0 wants to merge 1 commit intouutils:mainfrom
Alonely0:opt_timeout

Conversation

@Alonely0
Copy link
Contributor

Fixes #9099 by replacing a sleep() on a spin loop waiting on a process with a yield to the OS' scheduler.

Fixes uutils#9099 by replacing a sleep() on a spin loop waiting on a process with a yield to the OS' scheduler.
@Alonely0
Copy link
Contributor Author

Ty @depesz for the bug report and the pointer on the culprit!

@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)

@Alonely0
Copy link
Contributor Author

I don't think those CI fails are because of the changes on this PR :)

@scx1332
Copy link

scx1332 commented Oct 31, 2025

This fix will cause to 100% CPU Thread usage during wait which is definately not good.

@scx1332
Copy link

scx1332 commented Oct 31, 2025

In my opinion Sleep(1) //1ms would be great option, because you reduce waiting time to around 1ms and do not create unnecessary 100%CPU load. If you want even greater response then there is usleep() function (in unix)

@Alonely0
Copy link
Contributor Author

Alonely0 commented Oct 31, 2025

This fix will cause to 100% CPU Thread usage during wait which is definately not good.

Can't say I can reproduce this. Ideally yield_now() avoids this; of course, if the system has absolutely nothing better to do, it's going to keep checking... but that's an edge case. However, we can combine it with spin_loop to signal the CPU to go low-power mode.

Screen.Recording.2025-10-31.172505.mp4

@Alonely0
Copy link
Contributor Author

Alonely0 commented Oct 31, 2025

Just remembered that OS timers exist. On POSIX there's setitimer(2), we could just set that up with a signal handler, which if executes, handles the code path for failure; if it does not, it's because the process finished first and therefore also timeout (even so, we should probably disarm the timer just in case). I think this is quite preferable to the yield_now() or sleep(), so unless someone opposes I'll change the code to use this.

However, this is more work than I can handle at the moment; I can take it up from here on Wednesday after my exams are over, though. The yield_now() solution seems to work well enough and is much more simpler than setting up OS timers and signal handlers (and handling all different platforms), even if it is the most "correct" option.

@Alonely0
Copy link
Contributor Author

Alonely0 commented Nov 3, 2025

Superseded by #9123.

@Alonely0 Alonely0 closed this Nov 3, 2025
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.

timeout is slow

2 participants