Skip to content
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

[engine] generate concrete subgraphs of nodes using the rule graph #4304

Closed
baroquebobcat opened this issue Mar 2, 2017 · 2 comments
Closed

Comments

@baroquebobcat
Copy link
Contributor

With the rule graph, we can generate subgraphs until we need to switch subjects. Instead of generating dependencies one set at a time, generate and insert nodes into the graph until reaching an edge to a rule graph entry that can't be turned into a node yet.

@stuhood stuhood added the engine label Mar 2, 2017
@stuhood stuhood added this to the post-v2 cleanup milestone Mar 2, 2017
baroquebobcat added a commit that referenced this issue Mar 7, 2017
### Problem

Printing rule graphs has not been converted to Rust. This can make it hard to visualize what the rule sets look like. Also, the Python rule graph construction is effectively dead code already.

### Solution

Port rule graph visualization and visualization based testing to the rust side of the engine implementation.

### Result

Now we're setup to use the RuleGraph from the Rust side for functions as described in #4303 and #4304.
@stuhood stuhood modified the milestones: 1.4.0, post-v2 cleanup Apr 7, 2017
lenucksi pushed a commit to lenucksi/pants that referenced this issue Apr 25, 2017
…ld#4295)

### Problem

Printing rule graphs has not been converted to Rust. This can make it hard to visualize what the rule sets look like. Also, the Python rule graph construction is effectively dead code already.

### Solution

Port rule graph visualization and visualization based testing to the rust side of the engine implementation.

### Result

Now we're setup to use the RuleGraph from the Rust side for functions as described in pantsbuild#4303 and pantsbuild#4304.
@stuhood
Copy link
Sponsor Member

stuhood commented Jan 12, 2018

In the context of Futures, I'm not sure that this is any more efficient. But there are definite advantages to the static RuleGraph: #4020 is particularly important! Will close this one out.

@stuhood stuhood closed this as completed Jan 12, 2018
@stuhood stuhood reopened this Jan 12, 2018
@stuhood
Copy link
Sponsor Member

stuhood commented Jan 12, 2018

Er... wait a second. There are definitely advantages, as you can launch all of the Nodes for a subgraph in parallel. Reopened.

@stuhood stuhood modified the milestones: 1.4.x, 1.5.x Jan 12, 2018
@stuhood stuhood removed this from the 1.5.x milestone Feb 6, 2018
stuhood pushed a commit that referenced this issue Sep 20, 2018
### Problem

As described in #5788: `@rules` need a way to rely on values that are provided at request time, but which do not necessarily participate in the signatures of all `@rules` between the root and the consuming `@rule`. This is related to the existing concept of "variants", but requires an implementation that does not introduce variants into `Node` identities in subgraphs where they are not required.

Additionally, as described a while back in #4304, it should be possible to generate concrete subgraphs by removing ambiguity from the `RuleGraph`... but ambiguity is currently a "feature" required for composability of `@rule`s that do not know about one another.

### Solution

This change merges `Variants` and "subjects" into `Params`, and statically determines which `Params` are required in each subgraph. In order to handle cases where multiple providers of an `@rule` type dependency are available with different required input `Params`, the change "monomorphizes" (duplicates) `RuleGraph` entries per used parameter set. This allows us to remove runtime `Noop`s, because every `RuleGraph` entry (and thus `Node`) has exactly one `@rule` provider for each of its declared dependencies.

### Result

Lays groundwork for #4020 and #5788. Fixes #4304 by monomorphizing `RuleGraph` entries and removing `Noop`. Fixes #4027 by... deleting that code.

This change does not yet expose any sort of UX for providing more than one `Param` in a `Get` or root request, but it was already way too large, so I've opened #6478 for followup.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants