Auto sync point before exclusive system despite *_ignore_deferred
#17874
Conversation
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
urben1680
changed the title
*_ignore_deferred
…having deferred parameters
urben1680
commented
Feb 15, 2025
Comment on lines
+2195
to
+2197
| insert_resource.before_ignore_deferred(contains_resource), | ||
| another_with_deferred.before_ignore_deferred(contains_resource), | ||
| contains_resource, |
There was a problem hiding this comment.
I guess I could make a chain of three anonymous systems but I wanted to express here that two nodes have edges with contains_resource, not one between the two nodes and one between the latter and the exclusive system.
|
I think this approach has too many flaws, I will make a new PR when I fixed some issues I noticed. |
urben1680
deleted the
auto-sync-before-exclusive-system-despite-ignore-deferred
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Objective
Fixes #17828
Exclusive systems are expected to see the effects of all precedent commands. This was previously not made sure of by the scheduler. This showed especially with
*_ignore_deferredconfigurations.Solution
Every edge that leads to an exclusive system will contain a sync point that are combined into one if needed.
There needs to be no extra sync point after the exclusive system because it can only queue commands in the global commands queue and that is flushed already in the
ExclusiveFunctionSystem::runimplementation.Testing
See the new unit tests.
I did not add mirrored tests of those in the
no_sync_edgesandno_sync_chainmodules below the new test because I think these are enough. Though I can add more if desired.Migration Guide
I am not sure if this needs any. The API did not change and I can think up no scenario where someone would rely on absent sync points before exclusive systems.