-
Notifications
You must be signed in to change notification settings - Fork 467
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tighten synchronization scope specification #1843
base: main
Are you sure you want to change the base?
Conversation
which should resolve confusions about set intersections and dependency chaining
Thanks @krOoze - I like the predicate logic approach. Will have a look at this in more detail when I get a chance (probably in the next few days) |
* Let *A'* be the set of operations satisfying *A~S~*. | ||
* Let *B'* be the set of operations satisfying *B~S~*. | ||
* Submitting *S* for execution will result in an execution dependency | ||
*E* between *A'* and *B'*. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the use of predicate logic here is great. This seems intuitive and also more precise than the current spec.
I'm wondering what you think about this potential adjustment, which uses some more of the current wording:
* Let *A* and *B* be separate sets of operations.
* Let *S* be a synchronization command.
* Let *A~S~* be the first synchronization scope of *S*.
* Let *B~S~* be the second synchronization scope of *S*.
* Let *A'* be the set of operations in *A* satisfying *A~S~*.
* Let *B'* be the set of operations in *B* satisfying *B~S~*.
* Submitting *A*, *S* and *B* for execution, in that order, will result in
execution dependency *E* between *A'* and *B'*.
The reasoning being that, at least to me, this makes it a bit more clear that *B'*
can include operations that are submitted later. The note you have below clarifies this, which is great, but I think it would be nice if we could have this be really clear just from the main text of the spec, so that the note is less essential.
Arguably the update I'm suggesting is a bit redundant, because the predicates already "select" the operations, but I do think it reduces the need for the extra note. (My understanding is notes are meant to be more of an optional extra, and not essential for understanding the spec).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for late answer. Yea it is somewhat annoying B is a temporal set. One has to go outside spacetime to see the full extent of B.
Notes\comments are always desperate measures. Make no mistake, I would do everyting humanely possible to try to eliminate them.
What you suggest is there originally, and what I wanted to avoid, because it makes several inaccurate preassumptions. For example, let's say S is a vkWaitForFences
. What does it even mean "submitting vkWaitForFences
in order with B"? There's not even any queue to establish order. The thing that actually says what is covered by the synchronization, is the synchronization scope, not the order in which operations are submitted. For some hypothetical new S command, A should even be submitted after S, because why not (it could be sync command designed to establish dependency between two things in the future that are yet to be submitted).
Dammit, this got forgotten. Gotta rebase this... |
limited and evolved version of #1833
fix #1815
Tighten the definition of what "synchronization scope" actually is.
That should resolve confusions about using set intersections in the specification, and confusions about execution dependency chaining, supplemented by an example.
ping @williamih, @Tobski, @stonesthrow for input