make scheduled/deferred task order deterministic#927
Closed
robnee wants to merge 1 commit intosolvespace:masterfrom
robnee:refreshTimer
Closed
make scheduled/deferred task order deterministic#927robnee wants to merge 1 commit intosolvespace:masterfrom robnee:refreshTimer
robnee wants to merge 1 commit intosolvespace:masterfrom
robnee:refreshTimer
Conversation
Member
|
Will look, 2-3 days. |
Contributor
Author
|
@ruevs thanks. To be clear, I am not proposing this for 3.0 (maybe slap the 4.0 label on it) so I think we can take our time looking at this. |
ruevs
pushed a commit
that referenced
this pull request
Jun 14, 2022
Fixes #920 #1143 Explanation from @robnee on Feb 7, 2021 in pull request #927 Solvespace uses two timers (generateAllTimer and showTWTimer) to defer tasks until the event loop processing finishes. This helps coalesce multiple calls into one. You can call scheduleGenerateAll multiple times while processing UI messages but only trigger one GenerateAll. scheduleGenerateAll and scheduleShowTW do their scheuduling by setting timers with durations of zero. These timers fire (at least on Linux and Windows) some time after all other events in the message queue have been processed. This works fine when scheduling either one of these tasks. However, there is no guarantee in what order the timers will fire (at least on Windows) regardless of which order the scheduling calls are made. It's pretty easy to demonstrate (on some platforms) by adding logging to the scheduling calls and timer callbacks. In many cases TextWindow::Show depends on generateAll happening first. This causes UI glitches where displays don't update and their contents are stale. Since this behavior is not deterministic it's easy to imagine how this problem could make certain bug reports difficult to reproduce and diagnose. #920 is a good example. It also makes syncing up UI behavior across all platforms a challenge. Solving this in the platform domain is tricky. This is PR endeavors to make the ordering of deferred calls to TextWindow::Show and generateAll deterministic. It does this by replacing generateAllTimer and showTWTimer with a single refreshTimer. Calls to scheduleGenerateAll and scheduleShowTW set flags to note the requested operations and schedule the refreshTimer. A new callback function SolveSpaceUI::Refresh can then check the flags and ensure that generateAll happens first. It fixes #920. Moreover, this PR makes it easy to observe and reproduce this problem reliably and across all platforms by simply reordering the calls in the Refresh callback. It's pretty clear that the ordering is important so some solution is needed, if for no other reason than the sanity of the devs. I think this is a pretty good solution as it spells out the ordering. If nothing else this PR is helpful in further investigations. @ruevs @phkahler I'd like to hear your thoughts.
phkahler
pushed a commit
that referenced
this pull request
Jun 14, 2022
Fixes #920 #1143 Explanation from @robnee on Feb 7, 2021 in pull request #927 Solvespace uses two timers (generateAllTimer and showTWTimer) to defer tasks until the event loop processing finishes. This helps coalesce multiple calls into one. You can call scheduleGenerateAll multiple times while processing UI messages but only trigger one GenerateAll. scheduleGenerateAll and scheduleShowTW do their scheuduling by setting timers with durations of zero. These timers fire (at least on Linux and Windows) some time after all other events in the message queue have been processed. This works fine when scheduling either one of these tasks. However, there is no guarantee in what order the timers will fire (at least on Windows) regardless of which order the scheduling calls are made. It's pretty easy to demonstrate (on some platforms) by adding logging to the scheduling calls and timer callbacks. In many cases TextWindow::Show depends on generateAll happening first. This causes UI glitches where displays don't update and their contents are stale. Since this behavior is not deterministic it's easy to imagine how this problem could make certain bug reports difficult to reproduce and diagnose. #920 is a good example. It also makes syncing up UI behavior across all platforms a challenge. Solving this in the platform domain is tricky. This is PR endeavors to make the ordering of deferred calls to TextWindow::Show and generateAll deterministic. It does this by replacing generateAllTimer and showTWTimer with a single refreshTimer. Calls to scheduleGenerateAll and scheduleShowTW set flags to note the requested operations and schedule the refreshTimer. A new callback function SolveSpaceUI::Refresh can then check the flags and ensure that generateAll happens first. It fixes #920. Moreover, this PR makes it easy to observe and reproduce this problem reliably and across all platforms by simply reordering the calls in the Refresh callback. It's pretty clear that the ordering is important so some solution is needed, if for no other reason than the sanity of the devs. I think this is a pretty good solution as it spells out the ordering. If nothing else this PR is helpful in further investigations. @ruevs @phkahler I'd like to hear your thoughts.
ruevs
added a commit
to ruevs/solvespace
that referenced
this pull request
Nov 6, 2024
devin-ai-integration bot
pushed a commit
to erkinalp/solvespace
that referenced
this pull request
Apr 3, 2025
Fixes solvespace#920 solvespace#1143 Explanation from @robnee on Feb 7, 2021 in pull request solvespace#927 Solvespace uses two timers (generateAllTimer and showTWTimer) to defer tasks until the event loop processing finishes. This helps coalesce multiple calls into one. You can call scheduleGenerateAll multiple times while processing UI messages but only trigger one GenerateAll. scheduleGenerateAll and scheduleShowTW do their scheuduling by setting timers with durations of zero. These timers fire (at least on Linux and Windows) some time after all other events in the message queue have been processed. This works fine when scheduling either one of these tasks. However, there is no guarantee in what order the timers will fire (at least on Windows) regardless of which order the scheduling calls are made. It's pretty easy to demonstrate (on some platforms) by adding logging to the scheduling calls and timer callbacks. In many cases TextWindow::Show depends on generateAll happening first. This causes UI glitches where displays don't update and their contents are stale. Since this behavior is not deterministic it's easy to imagine how this problem could make certain bug reports difficult to reproduce and diagnose. solvespace#920 is a good example. It also makes syncing up UI behavior across all platforms a challenge. Solving this in the platform domain is tricky. This is PR endeavors to make the ordering of deferred calls to TextWindow::Show and generateAll deterministic. It does this by replacing generateAllTimer and showTWTimer with a single refreshTimer. Calls to scheduleGenerateAll and scheduleShowTW set flags to note the requested operations and schedule the refreshTimer. A new callback function SolveSpaceUI::Refresh can then check the flags and ensure that generateAll happens first. It fixes solvespace#920. Moreover, this PR makes it easy to observe and reproduce this problem reliably and across all platforms by simply reordering the calls in the Refresh callback. It's pretty clear that the ordering is important so some solution is needed, if for no other reason than the sanity of the devs. I think this is a pretty good solution as it spells out the ordering. If nothing else this PR is helpful in further investigations. @ruevs @phkahler I'd like to hear your thoughts.
dennisyangji
pushed a commit
to Orthogonalpub/ode_solvespace
that referenced
this pull request
Nov 25, 2025
Fixes solvespace#920 solvespace#1143 Explanation from @robnee on Feb 7, 2021 in pull request solvespace#927 Solvespace uses two timers (generateAllTimer and showTWTimer) to defer tasks until the event loop processing finishes. This helps coalesce multiple calls into one. You can call scheduleGenerateAll multiple times while processing UI messages but only trigger one GenerateAll. scheduleGenerateAll and scheduleShowTW do their scheuduling by setting timers with durations of zero. These timers fire (at least on Linux and Windows) some time after all other events in the message queue have been processed. This works fine when scheduling either one of these tasks. However, there is no guarantee in what order the timers will fire (at least on Windows) regardless of which order the scheduling calls are made. It's pretty easy to demonstrate (on some platforms) by adding logging to the scheduling calls and timer callbacks. In many cases TextWindow::Show depends on generateAll happening first. This causes UI glitches where displays don't update and their contents are stale. Since this behavior is not deterministic it's easy to imagine how this problem could make certain bug reports difficult to reproduce and diagnose. solvespace#920 is a good example. It also makes syncing up UI behavior across all platforms a challenge. Solving this in the platform domain is tricky. This is PR endeavors to make the ordering of deferred calls to TextWindow::Show and generateAll deterministic. It does this by replacing generateAllTimer and showTWTimer with a single refreshTimer. Calls to scheduleGenerateAll and scheduleShowTW set flags to note the requested operations and schedule the refreshTimer. A new callback function SolveSpaceUI::Refresh can then check the flags and ensure that generateAll happens first. It fixes solvespace#920. Moreover, this PR makes it easy to observe and reproduce this problem reliably and across all platforms by simply reordering the calls in the Refresh callback. It's pretty clear that the ordering is important so some solution is needed, if for no other reason than the sanity of the devs. I think this is a pretty good solution as it spells out the ordering. If nothing else this PR is helpful in further investigations. @ruevs @phkahler I'd like to hear your thoughts.
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.
Solvespace uses two timers (
generateAllTimerandshowTWTimer) to defer tasks until the event loop processing finishes. This helps coalesce multiple calls into one. You can callscheduleGenerateAllmultiple times while processing UI messages but only trigger oneGenerateAll.scheduleGenerateAllandscheduleShowTWdo their scheuduling by setting timers with durations of zero. These timers fire (at least on Linux and Windows) some time after all other events in the message queue have been processed. This works fine when scheduling either one of these tasks. However, there is no guarantee in what order the timers will fire (at least on Windows) regardless of which order the scheduling calls are made. It's pretty easy to demonstrate (on some platforms) by adding logging to the scheduling calls and timer callbacks.In many cases
TextWindow::Showdepends ongenerateAllhappening first. This causes UI glitches where displays don't update and their contents are stale. Since this behavior is not deterministic it's easy to imagine how this problem could make certain bug reports difficult to reproduce and diagnose. #920 is a good example.It also makes syncing up UI behavior across all platforms a challenge.
Solving this in the platform domain is tricky. This is PR endeavors to make the ordering of deferred calls to
TextWindow::ShowandgenerateAlldeterministic. It does this by replacinggenerateAllTimerandshowTWTimerwith a singlerefreshTimer. Calls toscheduleGenerateAllandscheduleShowTWset flags to note the requested operations and schedule the refreshTimer. A new callback functionSolveSpaceUI::Refreshcan then check the flags and ensure thatgenerateAllhappens first. It fixes #920. Moreover, this PR makes it easy to observe and reproduce this problem reliably and across all platforms by simply reordering the calls in theRefreshcallback.It's pretty clear that the ordering is important so some solution is needed, if for no other reason than the sanity of the devs. I think this is a pretty good solution as it spells out the ordering. If nothing else this PR is helpful in further investigations.
@ruevs @phkahler I'd like to hear your thoughts.