MemoryCache: optimized clock access, optimized expiration checks

[edevoogd]

This PR is intended as an alternative for (or precursor to) #47239: Developers observe more efficient expiration checks in Microsoft.Extensions.Caching.MemoryCache.

There is quite a bit to unpack here.

This PR does not employ a timer for the background expiration scan, yet. Still relying on the background scan trigger checks in a bunch of public methods, the commits in this PR already eke out quite a bit more performance from the cache.

Benchmarks are included below. Additional performance improvements are still expected, if the background scan trigger checks were to be dropped in favor of timer-based background scans.

Benchmark comparison

No Slower results for the provided threshold = 1% and noise filter = 0.3ns.

Faster	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.TryGetValueMiss	2.70	42.08	15.58
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.GetMiss	2.49	41.75	16.79
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.TryGetValueHit	1.39	49.94	35.86
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.GetHit	1.33	49.11	36.99
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.AddThenRemove_NoExpir	1.29	36223.19	28155.27
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.AddThenRemove_Relativ	1.17	36824.81	31378.14
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.SetOverride	1.17	197.99	169.48
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.AddThenRemove_Absolut	1.15	35007.65	30454.63
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.AddThenRemove_Sliding	1.15	34268.17	29817.72
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.AddThenRemove_Expirat	1.12	42634.63	38148.16
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.CreateEntry	1.02	66.28	65.03

An extensive description of groups of individual commits explains the steps taken to introduce the alternative approach, which is based on 3 pillars:

1. Try to make most expiration decisions without needing a clock reading at all (clock quantization)
2. Access the clock as little and lazily as possible (only when absolutely required - e.g., skip for expired entries)
3. Access the clock as cheaply as possible
   • Introduce time-based properties based on a clock-specific unit of time, possibly independent of DateTimeOffset representation
   • For .NET 5.0+, leverage Environment.TickCount64

All existing unit tests are passing locally, but I would really like to have more eyes on this. Specifically, I would like to gage:

• Does the approach look sound?
• Do the benchmarked performance improvements outweigh added complexity and increased code/assembly size?
• Do the benchmarked performance improvements translate into tangible and desirable TE throughput improvements?
• Are there any objections to expanding the NuGet package with a .NET 5.0+ assembly?

[ghost]

Tagging subscribers to this area: @eerhardt, @maryamariyan, @michaelgsharp
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This PR is intended as an alternative for (or precursor to) #47239: Developers observe more efficient expiration checks in Microsoft.Extensions.Caching.MemoryCache.

There is quite a bit to unpack here.

This PR does not employ a timer for the background expiration scan, yet. Still relying on the background scan trigger checks in a bunch of public methods, the commits in this PR already eke out quite a bit more performance from the cache.

Benchmarks are included below. Additional performance improvements are still expected, if the background scan trigger checks were to be dropped in favor of timer-based background scans.

Benchmark comparison

No Slower results for the provided threshold = 1% and noise filter = 0.3ns.

Faster	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.TryGetValueMiss	2.70	42.08	15.58
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.GetMiss	2.49	41.75	16.79
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.TryGetValueHit	1.39	49.94	35.86
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.GetHit	1.33	49.11	36.99
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.AddThenRemove_NoExpir	1.29	36223.19	28155.27
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.AddThenRemove_Relativ	1.17	36824.81	31378.14
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.SetOverride	1.17	197.99	169.48
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.AddThenRemove_Absolut	1.15	35007.65	30454.63
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.AddThenRemove_Sliding	1.15	34268.17	29817.72
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.AddThenRemove_Expirat	1.12	42634.63	38148.16
Microsoft.Extensions.Caching.Memory.Tests.MemoryCacheTests.CreateEntry	1.02	66.28	65.03

An extensive description of groups of individual commits explains the steps taken to introduce the alternative approach, which is based on 3 pillars:

1. Try to make most expiration decisions without needing a clock reading at all (clock quantization)
2. Access the clock as little and lazily as possible (only when absolutely required - e.g., skip for expired entries)
3. Access the clock as cheaply as possible
   • Introduce time-based properties based on a clock-specific unit of time, possibly independent of DateTimeOffset representation
   • For .NET 5.0+, leverage Environment.TickCount64

All existing unit tests are passing locally, but I would really like to have more eyes on this. Specifically, I would like to gage:

• Does the approach look sound?
• Do the benchmarked performance improvements outweigh added complexity and increased code/assembly size?
• Do the benchmarked performance improvements translate into tangible and desirable TE throughput improvements?
• Are there any objections to expanding the NuGet package with a .NET 5.0+ assembly?

Author:	edevoogd
Assignees:	-
Labels:	area-Extensions-Caching
Milestone:	-

[dnfadmin]

All CLA requirements met.

[Tratcher]

Is the cache already benefiting from the runtime clock improvements?
#50263

[edevoogd]

Is the cache already benefiting from the runtime clock improvements?

Yes, "Base" as measured benefits from #50263.

[adamsitnik]

Hi @edevoogd

Big thanks for you PR. Could you please solve the merge conflict? As soon as you do that I am going to test your changes using our TechEmpower infrastructure.

[edevoogd]

Hi @adamsitnik, merge conflicts were resolved.

The failing build checks appear to be due to WinHttpHandler functional tests and hence unrelated to this PR:

.packages\microsoft.dotnet.helix.sdk\6.0.0-beta.21311.3\tools\Microsoft.DotNet.Helix.Sdk.MultiQueue.targets(78,5): error : (NETCORE_ENGINEERING_TELEMETRY=Test) Work item System.Net.Http.WinHttpHandler.Functional.Tests in job 4bbfaacb-d1b2-4f95-bd14-e1a68a22b02c has failed.
Failure log: https://helix.dot.net/api/2019-06-17/jobs/4bbfaacb-d1b2-4f95-bd14-e1a68a22b02c/workitems/System.Net.Http.WinHttpHandler.Functional.Tests/console

[edevoogd]

Hi @ViktorHofer, I hope you can educate me on this topic. When I opened this PR, a couple of weeks ago, all build checks were passing. When @adamsitnik recently asked me to resolve merge conflicts that had occurred since, it turned out that the build was now failing with more than half of the checks not passing. Apparently, some things had changed in the build infrastructure, such as as the adoption of a new SDK version. But there was more than that and it appears that recent failures were triggered by the net5.0 target that I had originally added.

As I was looking for clues, I bumped into #54012 and I took inspiration from some of the changes that you did. That said, I'm quite confident that I don't fully grasp the underlying build infrastructure changes that required me to change the project file; it turned out to be quite a trial-and-error experience for me. The build is passing now, but I'm wondering if my approach is correct. As you can see from commit 6636433, I set out to fix the net5.0 build, as well as follow your suggestion that a $(NetCoreAppCurrent) be added for libraries that have a net5.0 target. Unfortunately, I ran into build issues with $(NetCoreAppCurrent) and decided to do without for now (commit d0aac6b).

At the moment, as I don't fundamentally understand what it is exactly that I'm "fixing", I'm not very confident with my changes in the latest 2 commits. Could you please shed some light on recent build infrastructure changes that I should accommodate and if these commits are in line with expectations? Do we need the $(NetCoreAppCurrent) target as well? If so, I'm very much open to suggestions on what that would look like. A final question: should the targets in the "ref" project file mirror those in the "src" project file?

[maryamariyan]

Thank you for your contribution and approach proposal.
We triaged issue #47239 out of 6.0 release.
We are in later iterations of 6.0 and right now we don't want to take the risk of accepting this change.

Later, it would be good to first discuss the approach over the issue and expected behavior, before taking this refactored change.