Skip to content

fix(timeout, uucore): use sigwait and timers for waiting#13237

Open
Alonely0 wants to merge 1 commit into
uutils:mainfrom
Alonely0:fix-timeout-latency
Open

fix(timeout, uucore): use sigwait and timers for waiting#13237
Alonely0 wants to merge 1 commit into
uutils:mainfrom
Alonely0:fix-timeout-latency

Conversation

@Alonely0

@Alonely0 Alonely0 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Does away with the current waiting loop for great good and profit.

Notes: I had to use the nix crate for signals (even if we're trying to move away from it) because rustix has no plans to support the sigset APIs. For the timers, I had to manually use libc because nix has not caught up to Redox and does not allow for fine-grained platform control, although it's very self-contained and trivially safe; I also have had to add some bindings for Redox and Android, as libc does not fully support them (yet). Hence, I've done my best to encapsulate all the platform-dependent mess into something manageable and maintainable; I genuinely think it's as good as it gets.

Fixes: #9099, #11615 (and all other duplicates, if any). All other open timeout issues should be re-assessed since it's a big backend change.

@Alonely0 Alonely0 force-pushed the fix-timeout-latency branch 2 times, most recently from a44e1dc to 2fd27a1 Compare July 1, 2026 03:44
@codspeed-hq

codspeed-hq Bot commented Jul 1, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 4.85%

⚡ 1 improved benchmark
✅ 326 untouched benchmarks
⏩ 46 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation sort_ascii_utf8_locale 16.1 ms 15.4 ms +4.85%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing Alonely0:fix-timeout-latency (8563627) with main (e4781a1)

Open in CodSpeed

Footnotes

  1. 46 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/date/resolution (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/misc/io-errors (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/pid-pipe (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/dd/no-allocate is now passing!
Note: The gnu test tests/misc/write-errors was skipped on 'main' but is now failing.

@Alonely0 Alonely0 force-pushed the fix-timeout-latency branch 17 times, most recently from c7dfbe9 to 968dc25 Compare July 1, 2026 14:58
@Alonely0 Alonely0 force-pushed the fix-timeout-latency branch from 968dc25 to 8563627 Compare July 1, 2026 15:33
@Alonely0

Alonely0 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Well, after an 18-hour-long, caffeine- and hyperfocus-fueled frenzy, we got it! I shall rest (and eat), now.

I have to say this is the most stable and consistent any fixes for this issue have been, undoubtedly thanks to the efforts of the contributors who fixed SIGPIPE and other shenanigans. This PR also manages to keep/bring most of the quirks from GNU, but I'll check my notes in case there's something we can improve (read: unfix) in a hypothetical subsequent PR.

@Alonely0 Alonely0 marked this pull request as ready for review July 1, 2026 16:12
@oech3

oech3 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Should we have benchmark for timeout 0?

@Alonely0

Alonely0 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Should we have benchmark for timeout 0?

That just waitpids and skips all the shenanigans here. We could have a latency sanity-check benchmark by using a small wait time, but considering Codspeed's variability, I'm not sure if it's going to be super useful. It's something I'd rather try to tackle in another PR tbh.

@oech3

oech3 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

By the way, CodSpeed ignores time of all syscalls.

@Alonely0

Alonely0 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

I'd guess that does not include blocked time by clock_nanosleep? It's what std::thread::sleep does internally (usually), and therefore what we've done until now. Maybe a regular test with a timeout hook would be more fit in that case?

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