WIP: Generalize sequence incrementing and only increment sequences when time doesn't move forwards #136
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Rename `UuidV1Context` to `DefaultUuidV1Context` - add a `UuidV1Context` trait that has a `generate` method that accepts the current time - Store the last generated time in `DefaultUuidV1Context` Still not actually considering the previous/current times, though.
radix
requested review from
Dylan-DPC-zz,
kinggoesgaming and
KodrAus
as code owners
It's actually not as bad as I expected: the new mutex-using context takes roughly twice as much time to generate a Uuid compared to the old context which only used a single AtomicUsize and always incremented it.
I think we need to think this one through a little more, there should be an option available when a user is in |
|
@kinggoesgaming I have some other discussion points back on the ticket at #106. |
kinggoesgaming
changed the title
kinggoesgaming
changed the title
|
superseded by #137 |
Dylan-DPC-zz
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a big backwards-incompatible change, and WORK-IN-PROGRESS.
UuidV1Contextwas renamed toDefaultUuidV1ContextUuidV1Contextis added. This trait has a method for getting a u16 sequence number given the current time.DefaultUuidV1Contextnow has a Mutex in it for storing and updating the last time that a sequence number was requested. This means thatDefaultUuidV1Contextis only available when thestdfeature is enabled.Is there any way we can avoid the context? I'm sure it probably slows down new_v1 a lot (though I plan on adding some benchmarks for this).
Also, while #106 proposed changing
UuidV1Contextto a trait, I think this might be unnecessary churn. A trait is a good idea, but it should probably be called something else so thatUuidV1Contextcan remain backwards compatible.