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

Make UniqueStash generic over index type #9

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

spersson
Copy link
Contributor

One motivation for this change is to be able to have smaller indices.
As such, implement wrapping add for entry versions, for any number of bits
up to 64.

I have not updated documentation yet to mention the re-using of versions.

I am also considering to store the empty/full tag in most significant bit of the version field (and changing from enum to union as you suggested). Another thing is that I also want to create macros for making it easier for users to create their on index types (implementing the Index and UniqueIndex traits). But that's for later, curious if you are interested in this change first.
Open question about the conversion to/from string that is implemented for Tag already. If using custom index type then no such conversion is provided but I think that's OK for now...?

One motivation for this change is to be able to have smaller indices.
As such, implement wrapping add for entry versions, for any number of bits
up to 64.
src/index.rs Outdated
/// Create a new UniqueIndex.
///
/// This method should panic if `offset` is out of acceptable range.
fn new_index(offset: usize, version: u64) -> Self;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this should just be called "new".

@spersson
Copy link
Contributor Author

I tried updating the documentation now but I not very happy with it. Feel very free to change anything/everything... you won't hurt my feelings. :)

@Stebalien
Copy link
Owner

I'll try to review this as soon as I can but that probably won't be for a while.

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