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

exhaustiveness: speedup checking 3x-5x #987

Merged
merged 5 commits into from
Sep 19, 2023
Merged

exhaustiveness: speedup checking 3x-5x #987

merged 5 commits into from
Sep 19, 2023

Conversation

feds01
Copy link
Contributor

@feds01 feds01 commented Sep 19, 2023

This PR brings speedups of exhaustiveness from 3x-5x by the following changes:

This PR switches the internal exhaustiveness stores to just use Vecs over the Store interface. This avoids the locking mechanism for stores which never need to be locked.

Additionally, this also switches Fields to use a ThinVec over a Vec which leads to better performance and less memory use overall.

Finally, we don't need to store the bias on an IntRange directly, only when we need to convert back into normal values, so remove it from the IntRange struct.

In addition to performance improvements, the analysis phase now records the time taken for exhaustiveness to complete its operations, and reports them through the standard compiler metrics plumbing.

@feds01 feds01 self-assigned this Sep 19, 2023
@feds01 feds01 added the performance Issues and PRs related to performance label Sep 19, 2023
@kontheocharis
Copy link
Collaborator

What is ever the point of using Vec when ThinVec exists..?

@feds01
Copy link
Contributor Author

feds01 commented Sep 19, 2023

What is ever the point of using Vec when ThinVec exists..?

I'm not too sure, it's superior for memory use, but could it potentially have performance implications?

@feds01 feds01 merged commit fc291c9 into main Sep 19, 2023
@feds01 feds01 deleted the tc-metrics branch September 19, 2023 17:43
@kontheocharis
Copy link
Collaborator

kontheocharis commented Sep 19, 2023

I guess the disadvantage is that a zero-sized Vec does not require heap allocation where as a zero-sized ThinVec does. It seems the only disadvantage is ThinVec<T> can’t ever be zero-cost roundtripped to a Box<[T]>, String, or *mut T

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Issues and PRs related to performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants