Skip to content

Avoid closures in CachedTypeInspector#954

Merged
EdwardCooke merged 2 commits into
aaubry:masterfrom
MattKotsenas:refactor/cachedtypeinspector-allocs
Aug 31, 2024
Merged

Avoid closures in CachedTypeInspector#954
EdwardCooke merged 2 commits into
aaubry:masterfrom
MattKotsenas:refactor/cachedtypeinspector-allocs

Conversation

@MattKotsenas

Copy link
Copy Markdown
Contributor

The CachedTypeInspector creates a closure in order to capture the method parameters. This results in 2 allocations per object serialized. I switched to using the ConcurrentDictionary.GetOrAdd() method that takes a third context parameter in order to avoid allocations.

I also added an extension method as described in dotnet/runtime#13978 for runtimes where the overload doesn't exist. I also updated the language version from 8 to 9 in order to get access to static lambdas in order to ensure that the lambdas don't capture.

This reduces allocations by about ~4% in the benchmark:

Before

// * Summary *

BenchmarkDotNet v0.14.0, Windows 11 (10.0.22635.4010)
Intel Core i9-10940X CPU 3.30GHz, 1 CPU, 28 logical and 14 physical cores
.NET SDK 9.0.100-preview.7.24407.12
  [Host]                       : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX-512F+CD+BW+DQ+VL
  MediumRun-.NET 8.0           : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX-512F+CD+BW+DQ+VL
  MediumRun-.NET Framework 4.7 : .NET Framework 4.8.1 (4.8.9261.0), X64 RyuJIT VectorSize=256

IterationCount=15  LaunchCount=2  WarmupCount=10

| Method     | Job                          | Runtime            | Mean      | Error    | StdDev   | Gen0      | Gen1     | Gen2     | Allocated |
|----------- |----------------------------- |------------------- |----------:|---------:|---------:|----------:|---------:|---------:|----------:|
| Serializer | MediumRun-.NET 8.0           | .NET 8.0           |  50.25 ms | 1.055 ms | 1.578 ms | 2000.0000 | 500.0000 |        - |  24.44 MB |
| Serializer | MediumRun-.NET Framework 4.7 | .NET Framework 4.7 | 118.87 ms | 3.662 ms | 5.367 ms | 7800.0000 | 600.0000 | 200.0000 |     48 MB |

// * Warnings *
MinIterationTime
  SerializationBenchmarks.Serializer: MediumRun-.NET 8.0 -> The minimum observed iteration time is 93.442ms which is very small. It's recommended to increase it to at least 100ms using more operations.

After

// * Summary *

BenchmarkDotNet v0.14.0, Windows 11 (10.0.22635.4010)
Intel Core i9-10940X CPU 3.30GHz, 1 CPU, 28 logical and 14 physical cores
.NET SDK 9.0.100-preview.7.24407.12
  [Host]                       : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX-512F+CD+BW+DQ+VL
  MediumRun-.NET 8.0           : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX-512F+CD+BW+DQ+VL
  MediumRun-.NET Framework 4.7 : .NET Framework 4.8.1 (4.8.9261.0), X64 RyuJIT VectorSize=256

IterationCount=15  LaunchCount=2  WarmupCount=10

| Method     | Job                          | Runtime            | Mean      | Error    | StdDev   | Gen0      | Gen1     | Allocated |
|----------- |----------------------------- |------------------- |----------:|---------:|---------:|----------:|---------:|----------:|
| Serializer | MediumRun-.NET 8.0           | .NET 8.0           |  49.40 ms | 1.186 ms | 1.701 ms | 2000.0000 | 500.0000 |  23.52 MB |
| Serializer | MediumRun-.NET Framework 4.7 | .NET Framework 4.7 | 115.33 ms | 5.518 ms | 8.259 ms | 7500.0000 | 500.0000 |  47.08 MB |

// * Warnings *
MinIterationTime
  SerializationBenchmarks.Serializer: MediumRun-.NET 8.0 -> The minimum observed iteration time is 89.875ms which is very small. It's recommended to increase it to at least 100ms using more operations.

@MattKotsenas

Copy link
Copy Markdown
Contributor Author

Note that this PR uses the same benchmark introduced in #953. So if / when one of these PRs merge, the other should be rebased on top of latest main to remove the duplicate commit.

I'm happy to do this housekeeping. I just mention it for clarity, and so that each PR can be reviewed independently, rather than forcing them to all be chained together.

@MattKotsenas MattKotsenas force-pushed the refactor/cachedtypeinspector-allocs branch from 3732b8d to 9b03412 Compare August 31, 2024 23:49
@EdwardCooke EdwardCooke merged commit 462c5a0 into aaubry:master Aug 31, 2024
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