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

Possible performance optimizations #57

Open
alice-i-cecile opened this issue Mar 5, 2021 · 6 comments
Open

Possible performance optimizations #57

alice-i-cecile opened this issue Mar 5, 2021 · 6 comments

Comments

@alice-i-cecile
Copy link

  1. fixedbitset could be allocation free for small block counts (store blocks in a SmallVec)
  2. fixedbitset could have a const constructor

From Bevy's ECS rework notes. Figured I'd pass the thoughts on here in case there's appetite to improve this.

@AlvinKuruvilla
Copy link

@alice-i-cecile could you elaborate on your first point. Fair warning I am still fairly new to rust so this might be a noob question. Are you suggesting that the struct FixedBitSet take a SmallVec of Blocks rather than a Vec?

@alice-i-cecile
Copy link
Author

That's correct :) It looks like swapping to a SmallVec

data: Vec<Block>,
should be a seamless replacement.

I would probably gate this behind a feature flag to allow users who don't want the extra dependency to opt out.

@AlvinKuruvilla
Copy link

Awesome I will get do my best to work on it

@grovesNL
Copy link

@james7132 was the SmallVec in #74 problematic somehow? I'm interested in removing the Vec for small block counts and wondering if there's any particular direction we should explore next

@james7132
Copy link
Collaborator

@james7132 was the SmallVec in #74 problematic somehow? I'm interested in removing the Vec for small block counts and wondering if there's any particular direction we should explore next

It had a strong regression for contains as it always needed to do the heap check every time. Definitely not ideal for use cases that heavily relied on contains checks.

@james7132
Copy link
Collaborator

FixedBitset::new is now const so one of the two parts of this issue have been resolved.

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

No branches or pull requests

4 participants