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

Remove "visited" set from inhabitedness checking (fix perf regression) #57033

Merged
merged 1 commit into from Dec 22, 2018

Conversation

Projects
None yet
5 participants
@nikic
Copy link
Contributor

nikic commented Dec 21, 2018

Now that references are no longer recursively checked, this should no longer be necessary, and it's a major performance bottleneck.

This should fix #57028.

r? @varkor

Remove "visited" set from inhabitedness checking
Now that references are no longer recursively checked, this should
no longer be necessary, and it's a major performance bottleneck.
@varkor

This comment has been minimized.

Copy link
Member

varkor commented Dec 21, 2018

Thanks for catching this!

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 21, 2018

📌 Commit a8babed has been approved by varkor

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Dec 21, 2018

@bors: p=1

fixing a pretty large perf regression

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 21, 2018

⌛️ Testing commit a8babed with merge a9ff135...

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

Auto merge of #57033 - nikic:inhabitedness-perf, r=varkor
Remove "visited" set from inhabitedness checking (fix perf regression)

Now that references are no longer recursively checked, this should no longer be necessary, and it's a major performance bottleneck.

This should fix #57028.

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 a9ff135 to master...

@bors bors merged commit a8babed 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