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] optimize places_conflict... somehow #51820

Closed
nikomatsakis opened this issue Jun 26, 2018 · 1 comment
Closed

[nll] optimize places_conflict... somehow #51820

nikomatsakis opened this issue Jun 26, 2018 · 1 comment
Assignees
Labels
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.

Comments

@nikomatsakis
Copy link
Contributor

places_conflict currently accounts for 8% of MIR borrowck time. Most of that time, based on perf annotate results, seems to be spent in and around the SmallVec operations (e.g., push and into_iter). The tricky part here is that the Place data structures we are using are setup as a kind of tree, so that x.a.b is sort of like this:

  • Projection of b from:
    • Projection of a from:
      • Local variable x

But we want to iterate "bottom up", so x, then a, then b. This doesn't lend itself to a simple iterator so we currently build up a SmallVec of results.

It might be a win instead to build up a (stack-allocated) linked list by recursively "unrolling" the paths for both sides. Or perhaps dong other experiments, such as sharing a Vec buffer (that seemed a bit tricky from a type perspective when I looked into it).

Longer term, I'd like to move away from using Place here (and perhaps change how Place is setup in MIR) but it seems worth trying to tackle this in a targeted way.

@nikomatsakis nikomatsakis added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-compiler-nll NLL-performant Working towards the "performance is good" goal labels Jun 26, 2018
@nikomatsakis nikomatsakis changed the title optimize places_conflict [nll] optimize places_conflict... somehow Jun 26, 2018
@nikomatsakis
Copy link
Contributor Author

I'm going to take a stab at doing this, since I think my description will be nigh incomprehensible to anyone else.

@nikomatsakis nikomatsakis self-assigned this Jun 27, 2018
bors added a commit that referenced this issue Jun 27, 2018
optimize `places_conflict` to avoid complex vectors etc

Fixes #51820

I've not measured the perf here, we should.
@nikomatsakis nikomatsakis added this to the Rust 2018 Preview 2 milestone Jun 29, 2018
bors added a commit that referenced this issue Jun 30, 2018
optimize `places_conflict` to avoid complex vectors etc

Fixes #51820
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

1 participant