fix(timeout, uucore): use sigwait and timers for waiting - #13237
Conversation
a44e1dc to
2fd27a1
Compare
Merging this PR will improve performance by 4.85%
Performance Changes
Tip Curious why this is faster? Comment Comparing Footnotes
|
|
GNU testsuite comparison: |
c7dfbe9 to
968dc25
Compare
968dc25 to
8563627
Compare
|
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. |
|
Should we have benchmark for |
That just |
|
By the way, CodSpeed ignores time of all syscalls. |
|
I'd guess that does not include blocked time by |
|
The implementation seems solid to me. Just one question, why not move away from nix here? Very little of nix is being used and even though rustix doesn't have sigset, the api layer is rather small and could be implemented here in about 50 lines or so. Seems like this change would be a good case to continue with removal of nix. The signal conversion would be simpler too if it got canned. |
|
That's fine to do in another patch/PR, but I wouldn't want to add even more complexity to this PR. Let me ask in the Discord if there's some interest in doing this, but ty for the idea! And yes, I will look at your PR today :) |
Does away with the current waiting loop for great good and profit.
Notes: I had to use the
nixcrate for signals (even if we're trying to move away from it) becauserustixhas no plans to support thesigsetAPIs. For the timers, I had to manually uselibcbecausenixhas 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, aslibcdoes 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.