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

Use Attribute.GetCustomAttributes to reduce allocations / improve performance #953

Merged
merged 2 commits into from
Aug 31, 2024

Conversation

MattKotsenas
Copy link
Contributor

@MattKotsenas MattKotsenas commented Aug 15, 2024

Because the project no longer targets netstandard1.3, we can replace the custom implementation inside ReflectionExtensions.GetAllCustomAttributes() with Attribute.GetCustomAttributes().

In addition to simpler code, in the provided benchmark this eliminates 4 List<Attribute> allocations per serialized element. In the benchmark that results in ~10% less CPU time and ~5% fewer allocations.

Here's the added BenchmarkDotNet results on my machine:

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           |  48.12 ms | 1.434 ms | 2.057 ms | 2000.0000 | 500.0000 |        - |  24.44 MB |
| Serializer | MediumRun-.NET Framework 4.7 | .NET Framework 4.7 | 108.69 ms | 2.218 ms | 3.181 ms | 7800.0000 | 600.0000 | 200.0000 |     48 MB |

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

// * Hints *
Outliers
  SerializationBenchmarks.Serializer: MediumRun-.NET 8.0 -> 2 outliers were removed (55.60 ms, 58.61 ms)

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     | Gen2     | Allocated |
|----------- |----------------------------- |------------------- |---------:|---------:|---------:|----------:|---------:|---------:|----------:|
| Serializer | MediumRun-.NET 8.0           | .NET 8.0           | 44.42 ms | 1.825 ms | 2.731 ms | 2000.0000 | 500.0000 |        - |  23.21 MB |
| Serializer | MediumRun-.NET Framework 4.7 | .NET Framework 4.7 | 94.75 ms | 1.211 ms | 1.774 ms | 7333.3333 | 666.6667 | 166.6667 |  45.55 MB |

// * Warnings *
MultimodalDistribution
  SerializationBenchmarks.Serializer: MediumRun-.NET 8.0 -> It seems that the distribution can have several modes (mValue = 3.17)
MinIterationTime
  SerializationBenchmarks.Serializer: MediumRun-.NET 8.0 -> The minimum observed iteration time is 78.527ms which is very small. It's recommended to increase it to at least 100ms using more operations.

and here's the object allocation graph for a single iteration of the benchmark:

Before

Type Allocations
| + System.Collections.Generic.List<> 40,103
|| - System.Collections.Generic.List<System.Attribute> 40,004
Function Name Allocations Bytes Module Name
 + YamlDotNet.ReflectionExtensions.GetAllCustomAttributes<>(System.Reflection.PropertyInfo) 40,004 1,280,128 yamldotnet
| + YamlDotNet.Serialization.TypeInspectors.ReadablePropertiesTypeInspector+ReflectionPropertyDescriptor.GetCustomAttribute<T>() 40,004 1,280,128 yamldotnet

After

Type Allocations
| + System.Collections.Generic.List<> 99
|| - System.Collections.Generic.List<System.Text.RegularExpressions.RegexNode> 33

@MattKotsenas
Copy link
Contributor Author

MattKotsenas commented Aug 15, 2024

If you're new to interpreting the Visual Studio allocation results, there's a good demo here walking through the process: https://youtu.be/TRFfTdzpk-M?si=gO4VrPjTi2EJTWLp

If you have any questions / concerns, please let me know!

@MattKotsenas MattKotsenas force-pushed the refactor/attribute-allocs branch from 3be85fd to a561c02 Compare August 31, 2024 23:44
@EdwardCooke EdwardCooke merged commit 6d89d8d into aaubry:master Aug 31, 2024
1 check passed
@MattKotsenas MattKotsenas deleted the refactor/attribute-allocs branch August 31, 2024 23:52
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