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

Switch to using smallvec over tinyvec #100

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

james7132
Copy link

smallvec seems to be in much higher use than tinyvec, so adding tinyvec tends to increase the amount of work while using lewton as a dependency in a typical build. This PR replaces tinyvec with smallvec as a drop-in replacement.

@est31
Copy link
Member

est31 commented Jun 4, 2022

See the reasons why tinyvec was adopted: #75 . It's 100% safe. But yeah 52 dependents vs 759 dependents...

@james7132
Copy link
Author

I'm not too familiar with the operations being done with the type, but it does seem like it's collecting some kind of stream of bools into a temporary buffer of some kind.

Is the number of elements here bounded? If so, I could switch this to use a on-stack bitset instead, eliminating the dependency entirely.

Otherwise, I'm still going to insist on using smallvec here. I'm trying to minimize the dependency tree of Bevy, which depends transitively on this crate through rodio. This is the only instance of tinyvec in the entire dependency tree, while there are at least 6 uses of smallvec elsewhere.

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

Successfully merging this pull request may close these issues.

2 participants