RFC: run loop finish forward progress - #2217
Open
pointbazaar wants to merge 2 commits into
Open
Conversation
g++ test/run_loop_finish_repro.cpp -Iinclude running it under valgrind easily results in 2+ minutes of runtime. The results are unreliable, sometimes it is faster, sometimes slower. valgrind ./a.out but with fair scheduling it takes around 2 seconds reliably. valgrind --fair-sched=yes ./a.out This indicates the implementation may depend on specific scheduling behavior which creates at least 60x difference in runtime. Not super sure about where to put this test, grepping for 'valgrind' does not find anything in this repo, and the results are not 100% reproducible, therefore i did not integrate it with the existing tests for now. The issue i am trying to reproduce here is related to unit tests running under valgrind in other projects which rely on stdexec. Large unpredictable runtime differences on finishing the run loop can make their tests flaky since sometimes timeouts are exceeded. Next patch contains a fix attempt. Co-Authored-By: GPT 5.6 Sol Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>
The run loop drains queued work after finish() has been requested and also waits for __task_count_ to reach zero. If the queue becomes empty while the count is still non-zero, the current implementation busy-spins by repeatedly calling __execute_all(). This can starve another thread that still needs to make progress in order to decrement __task_count_. One concrete sequence is finish() incrementing the count by two, setting __finishing_, and queueing the noop task. The worker can then execute the noop, leaving the count at one, and immediately enter the tight drain loop. The caller still needs to execute its final fetch_sub() to bring the count to zero, but an unfair scheduler may keep running the spinning worker instead. This was reproducible under Valgrind's default scheduler as an intermittent hang during run_loop shutdown. Using Valgrind's fair scheduler avoided the hang, which pointed at a forward-progress issue rather than missing work. Yielding when there is no queued work but tasks are still in flight allows the thread responsible for completing those tasks to run. With this change, the reproducer completed 700 consecutive runs under Valgrind without --fair-sched=yes. Co-Authored-By: GPT 5.6 Sol Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>
|
Tested. Fix the issue. LGTM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When running tests using
sdbusplus::async::context(which usesexecution::run_loop) i experienced occasional timeouts under valgrind's default unfair scheduler.This RFC PR contains a stdexec-only reproducer and fix attempt.
Looking for your feedback on my use-case and fix attempt 😃