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

Short-circuit DefIdForest::intersection() #57060

Merged
merged 1 commit into from Dec 22, 2018

Conversation

Projects
None yet
4 participants
@nikic
Copy link
Contributor

nikic commented Dec 22, 2018

If the forest is already empty, there is no point in intersecting further.

Also handle the first element separately, so we don't compute an unnecessary intersection between the full forest and the first element, which is always equal to the first element.

This is the second try at fixing #57028, as the previous attempt only recovered part of the regression. I checked locally that this drops time spent in ty::inhabitedness for syn-check a lot, though not to zero.

r? @varkor

Short-circuit DefIdForest::intersection()
If the forest is already empty, there is no point in intersecting
further.

Also handle the first element separately, so we don't compute an
unnecessary intersection between the full forest and the first
element, which is always equal to the first element.
@varkor

This comment has been minimized.

Copy link
Member

varkor commented Dec 22, 2018

@bors r+ p=1

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 22, 2018

📌 Commit f93cbf6 has been approved by varkor

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 22, 2018

⌛️ Testing commit f93cbf6 with merge fa922ab...

bors added a commit that referenced this pull request Dec 22, 2018

Auto merge of #57060 - nikic:inhabit-perf-2, r=varkor
Short-circuit DefIdForest::intersection()

If the forest is already empty, there is no point in intersecting further.

Also handle the first element separately, so we don't compute an unnecessary intersection between the full forest and the first element, which is always equal to the first element.

This is the second try at fixing #57028, as the previous attempt only recovered part of the regression. I checked locally that this drops time spent in ty::inhabitedness for syn-check a lot, though not to zero.

r? @varkor
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 22, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: varkor
Pushing fa922ab to master...

@bors bors merged commit f93cbf6 into rust-lang:master Dec 22, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment