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

Use Ulid instead of Guid for better PERF #16828

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Use Ulid instead of Guid for better PERF #16828

wants to merge 6 commits into from

Conversation

hishamco
Copy link
Member

@hishamco hishamco commented Oct 4, 2024

The benchmarks show that the Ulid is faster https://github.com/Cysharp/Ulid

FYI .NET 9.0 introduces Guid.CreateVersion7() but according to my findings Ulid is still the fastest

Method Mean Error StdDev
NewGuidV4 45.01 ns 0.119 ns 0.106 ns
NewGuidV7 82.69 ns 0.821 ns 0.768 ns
NewUlid 29.90 ns 0.098 ns 0.092 ns
NewUlidToGuid 31.12 ns 0.185 ns 0.164 ns

@hishamco hishamco added the perf label Oct 4, 2024
@hishamco hishamco requested a review from sebastienros October 4, 2024 10:24
@hishamco hishamco self-assigned this Oct 4, 2024
@hishamco
Copy link
Member Author

hishamco commented Oct 4, 2024

Also, we could introduce IGuidGenerator in OC.Infrastructure.Abstraction that has the default implementation, so anyone else can replace the default implementation easily without touch the OC code-base

@sebastienros
Copy link
Member

sebastienros commented Oct 4, 2024

For the sake of the reasoning, we are talking 50ns in the best case. Which is 20 times less than a micro second (not milli, micro). For usages which are not in hot paths. And I can only see drawbacks, like introducing another external dependency, with potentially security implications (is it really unique?), and serialization concerns (will STJ support it like standard dotnet Guids?), and deviating from the standard dotnet libs.

Note that the author is very well trusted and we already use ZString or MessagePack.

@hishamco
Copy link
Member Author

hishamco commented Oct 4, 2024

And I can only see drawbacks, like introducing another external dependency

Right, but we already used external dependency in many places for example using ZString

Note that the author is very well trusted and we already use ZString or MessagePack.

Exactly we already used ZString

I'm wondering about the STJ serialization, if we intend to use such a thing I might spend some time testing the serialization, but I think it's doable

Copy link
Contributor

This pull request has merge conflicts. Please resolve those before requesting a review.

Copy link
Contributor

This pull request has merge conflicts. Please resolve those before requesting a review.

Copy link
Contributor

github-actions bot commented Nov 9, 2024

This pull request has merge conflicts. Please resolve those before requesting a review.

Copy link
Contributor

This pull request has merge conflicts. Please resolve those before requesting a review.

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

Successfully merging this pull request may close these issues.

2 participants