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

[nll] teach SCC about 'static #53178

Closed
nikomatsakis opened this issue Aug 7, 2018 · 3 comments
Closed

[nll] teach SCC about 'static #53178

nikomatsakis opened this issue Aug 7, 2018 · 3 comments
Assignees
Labels
A-NLL Area: Non-lexical lifetimes (NLL) NLL-performant Working towards the "performance is good" goal T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@nikomatsakis
Copy link
Contributor

Right now, if you have a region R where R: 'static, the SCC computation in NLL is not aware that this means that R = 'static. If we taught it that 'static: R for all R, though, it would put them in the same SCC. This might be a big win for things like html5ever, as it would collapse almost all the regions into one big SCC. (I was doing some profiling on html5ever using the branch from #53177, and it seemed like this would help further drop the overhead.)

@nikomatsakis nikomatsakis added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-NLL Area: Non-lexical lifetimes (NLL) WG-compiler-nll NLL-performant Working towards the "performance is good" goal labels Aug 7, 2018
@nikomatsakis nikomatsakis added this to the Rust 2018 RC milestone Aug 8, 2018
@nikomatsakis
Copy link
Contributor Author

nikomatsakis commented Aug 8, 2018

@wesleywiser is going to take a look at this

@nikomatsakis
Copy link
Contributor Author

Here are some relevant links.

This is where we compute the SCCs:

let constraint_sccs = Rc::new(constraints.compute_sccs(&constraint_graph));

This is the definition of that method:

crate fn compute_sccs(
&self,
constraint_graph: &graph::ConstraintGraph,
) -> Sccs<RegionVid, ConstraintSccIndex> {
let region_graph = &graph::RegionGraph::new(self, constraint_graph);
Sccs::new(region_graph)
}

It takes a ConstraintSet:

#[derive(Clone, Default)]
crate struct ConstraintSet {
constraints: IndexVec<ConstraintIndex, OutlivesConstraint>,
}

as well as a ConstraintGraph, from which it builds a RegionGraph:

crate struct RegionGraph<'s> {
set: &'s ConstraintSet,
constraint_graph: &'s ConstraintGraph,
}

The RegionGraph has an edge R1 -> R2 if there is a constraint R1: R2. This is defined by this trait impl:

impl<'s> graph::WithSuccessors for RegionGraph<'s> {
fn successors<'graph>(
&'graph self,
node: Self::Node,
) -> <Self as graph::GraphSuccessors<'graph>>::Iter {
self.sub_regions(node)
}
}

To make the change, I think we want to supply an altered graph that adds additional edges. We know that (by definition) 'static: 'a for any region 'a. So basically we might extend region_graph so that, in the successors method, it checks if the region is 'static. If so, it just returns an iterator over every region. Otherwise, it returns the existing code.

Note that a lot of this code changes in my PR #53177 -- in particular, the graph is now parameterized so that R1: R2 can also mean an edge R1 <- R2 (a "reverse" edge). If we are in reverse, you would rather want to add 'static as an "extra successor" to every region. Alternatively, we could define an "augmented region graph" that only works for "normal" region graphs, since we only care for this extra edge when doing the SCC computation.

@nikomatsakis
Copy link
Contributor Author

wesleywiser added a commit to wesleywiser/rust that referenced this issue Aug 27, 2018
wesleywiser added a commit to wesleywiser/rust that referenced this issue Aug 28, 2018
wesleywiser added a commit to wesleywiser/rust that referenced this issue Sep 7, 2018
@bors bors closed this as completed in 4e706f5 Sep 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NLL Area: Non-lexical lifetimes (NLL) NLL-performant Working towards the "performance is good" goal T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants