Clean-up CacheEntry

[pentp]

I got inspired by #45962 and cleaned up CacheEntry internal structure (ICacheEntry contract remains the same).
This reduces the object size (incl. headers) from 152 to 104 on x64.

Also removed CacheEntryState as it was actually causing additional padding and there's still room for flags in the main class (1 byte of padding is unused currently, so we should start packing flags only if we were to add two more booleans).

Replaced all uses of DateTimeOffset with UTC DateTime because it has significantly lower overhead.
Added a guard against ICacheEntry.Size changes after it has been committed to cache.

[ghost]

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

Issue Details

---

I got inspired by #45962 and cleaned up CacheEntry internal structure (ICacheEntry contract remains the same).
This reduces the object size (incl. headers) from 152 to 104 on x64.

Also removed CacheEntryState as it was actually causing additional padding and there's still room for flags in the main class (1 byte of padding is unused currently, so we should start packing flags only if we were to add two more booleans).

Replaced all uses of DateTimeOffset with UTC DateTime because it has significantly lower overhead.
Added a guard against ICacheEntry.Size changes after it has been committed to cache.

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

[pentp]

cc @adamsitnik

[maryamariyan]

Thanks @pentp for the changes. Waiting for @adamsitnik to add their review to this PR as well.
Seems like there is a conflict on the PR right now.

[pentp]

Rebased and solved conflicts from #59310 and #59607

[adamsitnik]

Hi @pentp

Could you please run our microbenchmarks and share the results? They are available here. Please use two coreruns as described here.

Once we prove that there is an improvement using the microbenchmarks I am going to build your fork and validate it using our TechEmpower perf infra

[pentp]

I ran the benchmarks, but the 5 large allocation methods are extremely variable in their results (allocations varying over 2x between runs for the same test).

BenchmarkDotNet=v0.13.1.1611-nightly, OS=Windows 10.0.19043.1288 (21H1/May2021Update)
Intel Core i7-7700K CPU 4.20GHz (Kaby Lake), 1 CPU, 8 logical and 4 physical cores
.NET SDK=6.0.100-rc.2.21505.57
  [Host]     : .NET 6.0.0 (6.0.21.48005), X64 RyuJIT
  Job-OPCZQZ : .NET 7.0.0 (42.42.42.42424), X64 RyuJIT
  Job-TUMWWU : .NET 7.0.0 (42.42.42.42424), X64 RyuJIT

PowerPlanMode=00000000-0000-0000-0000-000000000000  Arguments=/p:DebugType=portable,-bl:benchmarkdotnet.binlog  IterationTime=250.0000 ms
MaxIterationCount=20  MinIterationCount=15  WarmupCount=1

Method	Toolchain	Mean	Error	StdDev	Median	Min	Max	Ratio	Gen 0	Gen 1	Allocated
GetHit	\ref-runtime	44.15 ns	0.297 ns	0.263 ns	44.09 ns	43.78 ns	44.70 ns	1.00	-	-	-
GetHit	\runtime	38.50 ns	0.142 ns	0.126 ns	38.49 ns	38.28 ns	38.74 ns	0.87	-	-	-
TryGetValueHit	\ref-runtime	44.51 ns	0.164 ns	0.128 ns	44.52 ns	44.30 ns	44.72 ns	1.00	-	-	-
TryGetValueHit	\runtime	38.98 ns	0.208 ns	0.184 ns	38.98 ns	38.63 ns	39.32 ns	0.88	-	-	-
GetMiss	\ref-runtime	36.89 ns	0.111 ns	0.099 ns	36.88 ns	36.71 ns	37.08 ns	1.00	-	-	-
GetMiss	\runtime	31.73 ns	0.176 ns	0.156 ns	31.66 ns	31.57 ns	32.05 ns	0.86	-	-	-
TryGetValueMiss	\ref-runtime	37.77 ns	0.086 ns	0.071 ns	37.76 ns	37.61 ns	37.88 ns	1.00	-	-	-
TryGetValueMiss	\runtime	31.99 ns	0.079 ns	0.066 ns	31.99 ns	31.91 ns	32.12 ns	0.85	-	-	-
SetOverride	\ref-runtime	115.07 ns	0.305 ns	0.255 ns	115.12 ns	114.69 ns	115.62 ns	1.00	0.0359	-	152 B
SetOverride	\runtime	103.42 ns	0.585 ns	0.547 ns	103.31 ns	102.59 ns	104.70 ns	0.90	0.0247	-	104 B
CreateEntry	\ref-runtime	14.87 ns	0.108 ns	0.096 ns	14.88 ns	14.71 ns	15.04 ns	1.00	0.0363	-	152 B
CreateEntry	\runtime	11.53 ns	0.173 ns	0.162 ns	11.44 ns	11.38 ns	11.85 ns	0.78	0.0248	-	104 B
AddThenRemove_NoExpiration	\ref-runtime	25,928.86 ns	207.246 ns	183.718 ns	25,870.69 ns	25,758.58 ns	26,346.94 ns	1.00	10.5560	0.1045	44,507 B
AddThenRemove_NoExpiration	\runtime	24,149.41 ns	126.858 ns	105.932 ns	24,161.94 ns	24,025.26 ns	24,410.91 ns	0.93	9.3126	-	39,291 B
AddThenRemove_AbsoluteExpiration	\ref-runtime	26,445.74 ns	206.733 ns	183.264 ns	26,426.71 ns	26,187.72 ns	26,871.61 ns	1.00	10.5219	-	44,171 B
AddThenRemove_AbsoluteExpiration	\runtime	20,668.31 ns	150.816 ns	133.695 ns	20,617.81 ns	20,519.02 ns	20,909.96 ns	0.78	5.0231	-	21,289 B
AddThenRemove_RelativeExpiration	\ref-runtime	29,428.73 ns	96.505 ns	75.345 ns	29,416.25 ns	29,290.77 ns	29,561.29 ns	1.00	11.0640	0.1177	46,571 B
AddThenRemove_RelativeExpiration	\runtime	19,813.25 ns	118.948 ns	105.444 ns	19,799.40 ns	19,660.89 ns	20,031.38 ns	0.67	4.9716	-	21,097 B
AddThenRemove_SlidingExpiration	\ref-runtime	23,219.00 ns	255.456 ns	238.954 ns	23,108.57 ns	22,934.67 ns	23,702.81 ns	1.00	7.9394	-	33,402 B
AddThenRemove_SlidingExpiration	\runtime	21,286.75 ns	96.493 ns	85.539 ns	21,270.38 ns	21,145.66 ns	21,405.96 ns	0.92	6.6147	-	27,802 B
AddThenRemove_ExpirationTokens	\ref-runtime	32,030.91 ns	129.835 ns	115.096 ns	32,003.25 ns	31,850.22 ns	32,248.18 ns	1.00	11.2245	0.1276	47,139 B
AddThenRemove_ExpirationTokens	\runtime	32,465.74 ns	148.718 ns	139.111 ns	32,471.83 ns	32,172.27 ns	32,711.12 ns	1.01	12.3956	0.1305	52,211 B

[pentp]

Test errors are "No space left on device"...

[danmoseley]

@dotnet/dnceng

    DllImportGenerator.UnitTests.Diagnostics.TargetFrameworkNotSupported_NoGeneratedDllImport_NoDiagnostic(targetFramework: Framework) [FAIL]
      NuGet.Protocol.Core.Types.FatalProtocolException : Error downloading 'Microsoft.NETFramework.ReferenceAssemblies.net48.1.0.0' from 'https://api.nuget.org/v3-flatcontainer/microsoft.netframework.referenceassemblies.net48/1.0.0/microsoft.netframework.referenceassemblies.net48.1.0.0.nupkg'.
      ---- System.UnauthorizedAccessException : Access to the path '/Users/helix-runner/.nuget/packages/microsoft.netframework.referenceassemblies.net48/1.0.0' is denied.
      -------- System.IO.IOException : Permission denied
      Stack Trace:
           at NuGet.Protocol.GetDownloadResultUtility.GetDownloadResultAsync(HttpSource client, PackageIdentity identity, Uri uri, PackageDownloadContext downloadContext, String globalPackagesFolder, ILogger logger, CancellationToken token)
           at NuGet.Protocol.DownloadResourceV3.GetDownloadResourceResultAsync(PackageIdentity identity, PackageDownloadContext downloadContext, String globalPackagesFolder, ILogger logger, CancellationToken token)
        /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/ReferenceAssemblies.cs(323,0): at Microsoft.CodeAnalysis.Testing.ReferenceAssemblies.ResolveCoreAsync(String language, CancellationToken cancellationToken)
        /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/ReferenceAssemblies.cs(197,0): at Microsoft.CodeAnalysis.Testing.ReferenceAssemblies.ResolveAsync(String language, CancellationToken cancellationToken)
        /_/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/TestUtils.cs(170,0): at DllImportGenerator.UnitTests.TestUtils.ResolveReferenceAssemblies(ReferenceAssemblies referenceAssemblies)
        /_/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/TestUtils.cs(113,0): at DllImportGenerator.UnitTests.TestUtils.CreateCompilationWithReferenceAssemblies(SyntaxTree[] sources, ReferenceAssemblies referenceAssemblies, OutputKind outputKind, Boolean allowUnsafe)
        /_/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/Diagnostics.cs(78,0): at DllImportGenerator.UnitTests.Diagnostics.TargetFrameworkNotSupported_NoGeneratedDllImport_NoDiagnostic(TargetFramework targetFramework)
        --- End of stack trace from previous location ---
        ----- Inner Stack Trace -----
        /_/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs(369,0): at System.IO.FileSystem.CreateDirectory(String fullPath)
        /_/src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs(39,0): at System.IO.Directory.CreateDirectory(String path)
           at NuGet.Packaging.PackageExtractor.<>c__DisplayClass3_0.<<InstallFromSourceAsync>b__0>d.MoveNext()
        --- End of stack trace from previous location ---
           at NuGet.Common.ConcurrencyUtilities.ExecuteWithFileLockedAsync[T](String filePath, Func`2 action, CancellationToken token)
           at NuGet.Common.ConcurrencyUtilities.ExecuteWithFileLockedAsync[T](String filePath, Func`2 action, CancellationToken token)
           at NuGet.Packaging.PackageExtractor.InstallFromSourceAsync(String source, PackageIdentity packageIdentity, Func`2 copyToAsync, VersionFolderPathResolver versionFolderPathResolver, PackageExtractionContext packageExtractionContext, CancellationToken token, Guid parentId)
           at NuGet.Protocol.GlobalPackagesFolderUtility.AddPackageAsync(String source, PackageIdentity packageIdentity, Stream packageStream, String globalPackagesFolder, Guid parentId, ClientPolicyContext clientPolicyContext, ILogger logger, CancellationToken token)
           at NuGet.Protocol.GetDownloadResultUtility.<>c__DisplayClass3_0.<<GetDownloadResultAsync>b__0>d.MoveNext()
        --- End of stack trace from previous location ---
           at NuGet.Protocol.HttpSource.<>c__DisplayClass16_0`1.<<ProcessStreamAsync>b__0>d.MoveNext()
        --- End of stack trace from previous location ---
           at NuGet.Protocol.HttpSource.ProcessResponseAsync[T](HttpSourceRequest request, Func`2 processAsync, SourceCacheContext cacheContext, ILogger log, CancellationToken token)
           at NuGet.Protocol.HttpSource.ProcessStreamAsync[T](HttpSourceRequest request, Func`2 processAsync, SourceCacheContext cacheContext, ILogger log, CancellationToken token)
           at NuGet.Protocol.GetDownloadResultUtility.GetDownloadResultAsync(HttpSource client, PackageIdentity identity, Uri uri, PackageDownloadContext downloadContext, String globalPackagesFolder, ILogger logger, CancellationToken token)
        ----- Inner Stack Trace -----

also,

Agent name: 'NetCore1ESPool-Public 185'
Agent machine name: 'f8ea81eec00000A'

seems to need its disk cleaning up. could you please take a look?

[riarenas]

@danmoseley this has been a common issue since we moved to the 1ES hosted pools, as we lost the mechanisms that cleaned the workspaces. It's being tracked by https://github.com/dotnet/core-eng/issues/14682 and we are experimenting with attempting some of the suggestions from the 1ES team: https://github.com/dotnet/core-eng/issues/14683

There's also a tracking issue in runtime for this: #60610

[stephentoub]

@pentp, I think I saw in a separate thread you said you were going to have some more time soon for side efforts like this. Should we re-open this one?

[pentp]

Yes, I will address feedback for this PR also.

[maryamariyan]

@pentp let me know if I can help with this.

[maryamariyan]

Overall changes LGTM. Are the benchmarks up to date with the latest changes?

Is there a test for this behavioral change: #59110 (comment)

[pentp]

I addressed all feedback, replaced unsafe DateTime uses with just ticks.
Also removed the only functional change (size modification check after entry has been committed) as there's no test coverage for that and don't want to delay this PR any further.

[maryamariyan]

Thanks for the contribution @pentp. Merging.

[DrewScoggins]

Improvements: dotnet/perf-autofiling-issues#5073, dotnet/perf-autofiling-issues#5116, dotnet/perf-autofiling-issues#5146, dotnet/perf-autofiling-issues#5155