-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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 experiment: compute SCCs instead of iterative region solving #51987
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
@bors try |
[WIP] nll experiment: compute SCCs instead of iterative region solving This is an attempt to speed up region solving by replacing the current iterative dataflow with a SCC computation. The idea is to detect cycles (SCCs) amongst region constraints and then compute just one value per cycle. The graph with all cycles removed is of course a DAG, so we can then solve constraints "bottom up" once the liveness values are known. I kinda ran out of time this morning so the last commit is a bit sloppy but I wanted to get this posted, let travis run on it, and maybe do a perf run, before I clean it up.
Perf has been queued, should complete in around 7 hours. |
Perf URL for when data becomes available: |
☀️ Test successful - status-travis |
That link does not seem to be working. |
@Eh2406 yeah something went wrong with perf :( probably we will have results in about 2.5 hours from now |
OK, apparently the perf results are available at least partially now. This doesn't appear to be a big win. I'm not sure why, I'd like to dig a little more (I'd also like to land at least some of these refactorings regardless...). |
The link still is not working for me. How do you find the perf results you referred to? |
Link is now working, numbers look pretty good. double digit nll improvements in |
For some reason, scc generation takes 10% of clap-rs time. |
Er, wait, that may be incorrect. |
OK, I think I found the problem -- I wasn't checking for duplicates. I am rebasing now, but local measurements suggest that this wipes region solving off the profile. |
8b38e6f
to
05d4880
Compare
@bors try |
[WIP] nll experiment: compute SCCs instead of iterative region solving This is an attempt to speed up region solving by replacing the current iterative dataflow with a SCC computation. The idea is to detect cycles (SCCs) amongst region constraints and then compute just one value per cycle. The graph with all cycles removed is of course a DAG, so we can then solve constraints "bottom up" once the liveness values are known. I kinda ran out of time this morning so the last commit is a bit sloppy but I wanted to get this posted, let travis run on it, and maybe do a perf run, before I clean it up.
☀️ Test successful - status-travis |
Perf queued -- will be quite a while. |
So the perf results for this branch are available, but not the ideal comparison point. |
OK, so, this is now a massive win for inflate (45%) without any real downside (but not much benefit either). It also simplifies the code quite a bit, in terms of reasoning about inference, so I think we ought to land it. |
I don't understand your last comment, how is it "not much benefit either"? |
05d4880
to
028b312
Compare
e334a1c
to
6918c17
Compare
@bors r=pnkfelix |
📌 Commit 6918c17 has been approved by |
⌛ Testing commit 6918c17 with merge a4ed489b139480e6428239ce8ec2e5d29dce8f31... |
💔 Test failed - status-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
1 similar comment
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors retry I'm gonna hazard a guess that this is something transient:
cc @rust-lang/infra |
Yeah, it's a known problem -- #50887 |
nll experiment: compute SCCs instead of iterative region solving This is an attempt to speed up region solving by replacing the current iterative dataflow with a SCC computation. The idea is to detect cycles (SCCs) amongst region constraints and then compute just one value per cycle. The graph with all cycles removed is of course a DAG, so we can then solve constraints "bottom up" once the liveness values are known. I kinda ran out of time this morning so the last commit is a bit sloppy but I wanted to get this posted, let travis run on it, and maybe do a perf run, before I clean it up.
☀️ Test successful - status-appveyor, status-travis |
This was a 38% NLL win for @nikomatsakis: Was that expected? |
It also improved |
WIP: html5ever in the rustc-perf repository is memory-intensive Part of #52028. Rebased atop of #51987. r? @nikomatsakis
html5ever in the rustc-perf repository is memory-intensive Part of #52028. Rebased atop of #51987. r? @nikomatsakis
This is an attempt to speed up region solving by replacing the current iterative dataflow with a SCC computation. The idea is to detect cycles (SCCs) amongst region constraints and then compute just one value per cycle. The graph with all cycles removed is of course a DAG, so we can then solve constraints "bottom up" once the liveness values are known.
I kinda ran out of time this morning so the last commit is a bit sloppy but I wanted to get this posted, let travis run on it, and maybe do a perf run, before I clean it up.