fix(spin-common): resolve codependency scheduling gaps for nested pre/post-processors and their ordering constraints - #178
Merged
Conversation
…/post-processors and their ordering constraints
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
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.
Codependencies (
@PreProcess/@PostProcesstasks) never get their ownInstruction, so several kinds of scheduling information attached to them was being dropped on the floor. A codependency's own@Fromdependency was pulled into theProgramwithout a real graph edge tying it to the owning task, letting the owner (and its inline codependency) get dispatched concurrently with, rather than after, that dependency. A@Before/@Afterannotation declared directly on a codependency's ownTaskclass was silently ignored, since codependency resolution never went throughDefaultInstruction's ordering-constraint pass. And a codependency that itself declared@PreProcess/@PostProcesshad its own nested codependency dropped entirely, since resolution only ever looked one level deep.DefaultInstructionnow resolves codependencies transitively viaaddCodependenciesRecursively, discovering a nested pre/post-processor's own codependencies and preserving the correct execution order (a nested@PreProcesscodependency's dependencies are added before it; a nested@PostProcesscodependency's after). A newultimateOwnerhelper walks a codependency's@PreProcess/@PostProcesschain up to the top-levelTaskthat actually owns anInstruction, so@Before/@Afterdeclared anywhere in that chain gets folded into that owner's ordering-only dependencies instead of being lost.DefaultProgram's dependency-graph construction now also wires in edges forinstruction.codependencies().flatMap(Invocable::dependencies), so a codependency's own data dependencies become real scheduling constraints rather than just being included somewhere in theProgramwith no ordering tie to the owner.A known limitation is called out in the
addCodependenciesRecursivelyJavadoc: a@PreProcessnested under a@PostProcess(or vice versa) isn't reordered relative to its target, sinceDefaultProgram#runTaskexecutes all pre-processors as one flat group before the task and all post-processors as one flat group after.