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

Reduced allocations in hot methods #22

Merged
merged 1 commit into from
Dec 28, 2022

Conversation

gfoidl
Copy link
Contributor

@gfoidl gfoidl commented Dec 27, 2022

And used some low-hanging perf-tricks.

Compare especially the allocated column with the one from the ReadMe.


BenchmarkDotNet=v0.13.2, OS=Windows 10 (10.0.19045.2364)
Intel Core i7-7700HQ CPU 2.80GHz (Kaby Lake), 1 CPU, 8 logical and 4 physical cores
.NET SDK=7.0.101
  [Host]     : .NET 7.0.1 (7.0.122.56804), X64 RyuJIT AVX2
  DefaultJob : .NET 7.0.1 (7.0.122.56804), X64 RyuJIT AVX2

|                               Method |      Mean |    Error |    StdDev |    Median |   Gen0 | Allocated |
|------------------------------------- |----------:|---------:|----------:|----------:|-------:|----------:|
|                       Guid.NewGuid() |  90.99 ns | 1.783 ns |  1.392 ns |  90.81 ns |      - |         - |
|          Ulid.NewUlid(SimpleUlidRng) |  81.27 ns | 1.339 ns |  1.187 ns |  81.06 ns |      - |         - |
|              Ulid.NewUlid(CSUlidRng) | 183.88 ns | 3.715 ns |  7.997 ns | 184.12 ns |      - |         - |
| Ulid.NewUlid(SimpleMonotonicUlidRng) | 110.69 ns | 2.241 ns |  3.489 ns | 110.36 ns |      - |         - |
|     Ulid.NewUlid(CSMonotonicUlidRng) | 104.62 ns | 2.130 ns |  2.367 ns | 104.33 ns |      - |         - |
|                   Guid.Parse(string) | 297.19 ns | 5.884 ns |  8.248 ns | 294.01 ns | 0.0305 |      96 B |
|                   Ulid.Parse(string) | 383.53 ns | 7.719 ns | 16.113 ns | 379.30 ns | 0.0587 |     184 B |
|                      Guid.ToString() | 242.71 ns | 3.464 ns |  3.071 ns | 242.39 ns | 0.0305 |      96 B |
|                      Ulid.ToString() | 243.36 ns | 5.451 ns | 15.902 ns | 238.51 ns | 0.0253 |      80 B |
|                   `new Guid(byte[])` |  18.79 ns | 0.546 ns |  1.610 ns |  19.07 ns | 0.0127 |      40 B |
|                   `new Ulid(byte[])` |  22.31 ns | 0.566 ns |  1.668 ns |  22.64 ns | 0.0127 |      40 B |
|                   Guid.ToByteArray() | 107.64 ns | 2.221 ns |  4.332 ns | 107.81 ns | 0.0126 |      40 B |
|                   Ulid.ToByteArray() | 184.90 ns | 3.672 ns |  7.583 ns | 182.43 ns | 0.0126 |      40 B |
|                        Ulid.ToGuid() | 177.62 ns | 3.607 ns |  5.615 ns | 176.49 ns |      - |         - |
|                     `new Ulid(Guid)` | 113.43 ns | 1.943 ns |  1.817 ns | 113.40 ns | 0.0126 |      40 B |
|                   `new Ulid(Guid)`   | 99.74 ns  | 2.028 ns | 5.343 ns  | 97.46 ns  |      - |         - |

Some questions:

  • is this approach usable in this repo?
  • do you care about big-endian machines? If so I need to flip some byte-sequences.

Further more perf could be squeezed out (also for NS2.0), but I think the biggest improvement is by making some hot-methods (well definition of "hot" is here solely by me) allocation free.

And used some low-hanging perf-tricks.
@RobThree
Copy link
Owner

RobThree commented Dec 28, 2022

Hey,

First of all: thanks for the PR. Much appreciated! Looks good!

  • is this approach usable in this repo?

I guess. I don't see why not. Is there anything in particular you were referring to? Or the general PR?

  • do you care about big-endian machines? If so I need to flip some byte-sequences.

I don't think I care; though maybe an exception on BE machines would be a good idea to at least prevent some 'surprises' from happening. I don't know how much work it would be to also support BE and if it would be worth the effort. I would appreciate your thoughts on the matter too.

Finally; I do think there's a (minor) breaking change in this PR? Are there any others I may have missed?

Again, thank you very much, this all looks pretty good to me!

@RobThree RobThree merged commit f309291 into RobThree:master Dec 28, 2022
@gfoidl gfoidl deleted the allocation-less branch December 29, 2022 12:22
@gfoidl
Copy link
Contributor Author

gfoidl commented Dec 29, 2022

Is there anything in particular you were referring to? Or the general PR?

  • the '#if'defs. They are quite more than before.
  • adding new public methods (w/o kind of API review)
  • use of "unsafe" methods

BE machines

To support BE the easiest would be to re-use the NS2.0 code-path. But this is code bloat (which dead-code eliminate the JIT at runtime).
Optimized BE code could be created too (by reversing the vectors, span) but I don't think there's need for it.
Maybe we should wait for the outcome of #23, then before a release of this lib take measure for BE (either throw or use slow fallback -- I'm leaning towards throwing).

Breaking changes ... Are there any others I may have missed?

No, that's the only one. As it's not binary / source breaking I think that's OK. It's behaviorial breaking for that exception-type being thrown.

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