Precompute all knowable product graph node dependencies before executing the node #3960
Conversation
This file contains 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
When test_engine's assert_engine fails, produce a trace of the failing roots
This follows up https://rbcommons.com/s/twitter/r/4251/, removing the perf todos. It passes through the full graph construction's intermediate states so that they can short circuit subsequent subgraph constructions. It also moves transitive unfulfillable rule elimination so that it happens once for both full graph and subgraph instead of once per construction. There's also a bug fix in RuleEdges matching, which was caused by matching being overly broad.
add raw_root_rules to graph, temporarily add a failing test for how root rules should look clean up tests that were affected by the extraction add some temporary debug logging to execution request construction
There's a few bits in here that are naive. But it works
…e_graph_with_scheduler
fixes a refactor move issue in test_isolated_process, gets engine tests running again
…r the selector nodes
…n't working correctly--now it is
baroquebobcat
changed the title
baroquebobcat
added a commit
that referenced
this pull request
Dec 7, 2016
…d of a set (#4125) ### Problem I'm working on porting my patch for #3580 to the native engine, (#3960). First, I'm updating the graph construction code with the changes there. It hadn't covered Snapshot processes, and it modeled root rules as a set rather than a graph, which caused problems. ### Solution Include snapshot process' behavior in the rule graph construction used by the validator. Update tests to expect the new visualization format. ### List of changes * rename NodeBuilder to RuleIndex * rename RootRule to RootRuleGraphEntry * support SnapshotProcess in validator * Name snapshot process funcs so they are printable * Include subject and product types in intrinsic rule repr * Include graph generation methods that can construct new graphs from an existing one--these are currently unused * Raise an error if a declared rule is not reachable from the declared root types. * Unify naming for input selector for projection selectors * If field of a SelectDependencies is the default value, don't include it in the repr * Include a map of root rule to the rules that would fulfill it.
|
@baroquebobcat - closing this since it fails CI, has merge conflicts and is >6 months old. Feel free to re-open if you pick work back up on it. |
lenucksi
pushed a commit
to lenucksi/pants
that referenced
this pull request
Apr 25, 2017
…d of a set (pantsbuild#4125) ### Problem I'm working on porting my patch for pantsbuild#3580 to the native engine, (pantsbuild#3960). First, I'm updating the graph construction code with the changes there. It hadn't covered Snapshot processes, and it modeled root rules as a set rather than a graph, which caused problems. ### Solution Include snapshot process' behavior in the rule graph construction used by the validator. Update tests to expect the new visualization format. ### List of changes * rename NodeBuilder to RuleIndex * rename RootRule to RootRuleGraphEntry * support SnapshotProcess in validator * Name snapshot process funcs so they are printable * Include subject and product types in intrinsic rule repr * Include graph generation methods that can construct new graphs from an existing one--these are currently unused * Raise an error if a declared rule is not reachable from the declared root types. * Unify naming for input selector for projection selectors * If field of a SelectDependencies is the default value, don't include it in the repr * Include a map of root rule to the rules that would fulfill it.
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.
We now have a limited static analysis that we're currently using for validation.
This work takes that static analysis and uses it to construct the node dependencies so that we don't do so dynamically. The benefit of doing it this way is that we can eliminate trees in the graph that will ultimately noop in a statically analyzable way earlier and not even execute them.
For
./pants list ::in the pants repo, this reduces the number of nodes in the product graph by half.The other effect is to remove SelectNode, DependenciesNode and ProjectionNode because the scheduler manages their effects.
This work is for #3580