Thanks, interesting suggestion; can you please run our benchmarks and post before and after results here?

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.54%. Comparing base (b843130) to head (fed8a76).
Report is 6 commits behind head on develop.

@@             Coverage Diff             @@
##           develop    #1414      +/-   ##
===========================================
+ Coverage    78.48%   78.54%   +0.06%     
===========================================
  Files          200      201       +1     
  Lines         5703     5720      +17     
  Branches      1168     1168              
===========================================
+ Hits          4476     4493      +17     
  Misses         714      714              
  Partials       513      513              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Benchmark:

| Method  | Mean      | Error     | StdDev    |
|-------- |----------:|----------:|----------:|
| Default | 34.354 ns | 0.2249 ns | 0.2104 ns |
| Fast    |  8.337 ns | 0.0624 ns | 0.0553 ns |

    [Benchmark]
    public Guid Default() => Guid.NewGuid();

    [Benchmark]
    public Guid Fast() => FastGuid.NewGuid();

Tested on Windows, dotnet 8.
For different OS it could be different. In mentioned dotnet runtime issue they say ~900ns on Linux. On Linux Guid.NewGuid() uses /dev/urandom as a source for randomness. /dev/urandom serve as cryptographically secure pseudorandom number generators. They allow access to a CSPRNG that is seeded with entropy from environmental noise, collected from device drivers and other sources. It's known fact, that it have limited trougput, especially considering that in Kubernetes many containers share one physical machine and possible same /dev/urandom stream.

Ok, can you compare the existing Autofac benchmarks as well, the ones in our "bench" folder (https://github.com/autofac/Autofac/tree/develop/bench/Autofac.Benchmarks)?

Want to understand the overall impact of the change on Autofac, based on how many guids we generate.

What are specific Benchmarks which depends on .NewGuid() ?

I ran the entire set Before & After this change and don't see performance improvement.
It looks even worse, and I cannot explain why.

But the benchmarks mostly test Resolve*() methods, not registrations where GUID may be generated

Environment: x64, Linux
Example:
Before:

After:

Interesting that it's coming out worse...have you benchmarked dictionary lookups when Guids from FastGuid are used as a key? I'm wondering (without evidence) if the GetHashCode distribution of the guids are not spread out in the same way as with Guid.NewGuid?

I have tried to mix bits of Guid:

    public static Guid NewGuid()
    {
        var v = Interlocked.Increment(ref variablePart);
        var h = (int)v * 521;               // one-to-one function to improve Guid's hash distribution
        return new(h, (short)(v >> 32), (short)(v >> 48), BasePart);
    }

But it does not show visible improvement.
Guid.GetHashCode() XORs all 32-bit parts of Guid, so the distribution should be uniform

Thanks to @snaumenko-st for further improvement of this PR: 88272de

Benchmark results look not worse than before.

Trying to figure out the state of this so I can get a 0.1.0 release out for #1415. I may just release that one without this, regardless.

However, looking at the chain here, it sounds like:

• The first run of benchmarks showed that things actually got worse.
• Subsequent runs (not shown) indicate things aren't worse, but also aren't any better.
• The primary use case here is to improve registration time, which happens effectively once per application lifecycle.

It's not interesting to add an optimization that doesn't do anything, so I'm not sure where that leaves us.

If it's something where we can show some really marked improvement like "it used to take 10 seconds to start this app and now it takes half a second" or whatever, I'm all for it. But I'm not super interested in optimizing the "we told Autofac to scan and register every type in the entire app and all assemblies" use case - folks shouldn't be doing that, and the pain may be a forcing factor to tell them they're doing something wrong, especially if we're still supporting AnyConcreteTypeNotAlreadyRegisteredSource.

• The primary use case here is to improve registration time, which happens effectively once per application lifecycle.

In our case most of restrations happen at App start, but a few additional happen at every Web Request.
That is why we are interested in this improvement.

I think I still need to see some sort of improvement, which means this PR possibly needs to include adding some benchmarks that show the affected path and can show that there's improvement. It'd just be nice to see that it's not, like, "2ns improvement" or something basically meaningless. I saw the benchmark of raw GUID creation, but it'd be good to benchmark it in the context of the whole execution path.

Added a RegistrationBenchmark:

Before:

After:

Let me verify I'm reading this right:

After adding the fast GUID optimization, across six registrations you'll save about four microseconds (0.7us per registration) but allocate 1KB more memory to get that.

Does that sound right?

Yes.

Second run of this benchmark confirms.
Probably new Guids have not so random distribution and Dictionary allocates more buckets for them.

Hmmm. Maybe @alistairjevans feels differently, but... I... I have to admit, I kind of lean against this given the trade-off between the microseconds vs. allocations doesn't add up.

Yeah, I have to agree, the extra code complexity doesn't seem worth the tiny time gains, and we also don't understand the dictionary bucket impacts enough to know that any gain would be consistent anyway.

We see following hotspot on NewGuid() during profiling

So it looks not-insignificant

Was that profiling session captured on windows or Linux?

Linux Debian x64 .NET8

Ah; is that also where you've been running benchmarks?

Ah; is that also where you've been running benchmarks?

Not exactly
I was running benchmarks on Ubuntu x64 on real PC
but the App profiling was under Docker container in the Azure cloud

May be .NewGuid() implementation differs in that environment.
It is based on /dev/urandom, and VM Hypervisor may have slow implementation of this virtual device

Is it possible to run the benchmarks in that environment? Again, it'd just be nice to verify that this is going to fix the issue before it gets accepted.

Is it possible to run the benchmarks in that environment?

I did (isolated Guid benchmark only). The results on Azure are the same as on bare Linux.
FastGuid ~10x times faster than Guid.NewGuid()

A agree, it is not enough to be merge into Autofac

Sounds good. We'll close this one out for now, but if you can show that it will be a marked improvement in some environments maybe we can revive it and target those environments.