.NET 6 vs .NET 7 reflection performance regression

[ekalchev]

Description

All the details are here
https://davecallan.com/net-6-v-net-7-reflection-performance-benchmarks/

Reproduction Steps

None

Expected behavior

None

Actual behavior

None

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[En3Tho]

A simple repro worth more than a thousand blog posts. (c) Sun Cputzu

Is problem in FieldInfo.GetValue?

Can you share benchmarks code?

[ekalchev]

I am just bringing this to your attention. This is not my blog and I don't know more than you know.

[En3Tho]

using System.Reflection;
using BenchmarkDotNet.Attributes;

namespace Benchmarks.CSharp.Lib
{
    public class Target
    {
        private string secret = "42";
        public string Secret => secret;
    }
    
    [MemoryDiagnoser, DisassemblyDiagnoser]
    public class FieldInfoReflection
    {
        private readonly Target instance = new Target();
        private readonly FieldInfo field =
            typeof(Target)
               .GetField("secret",
                    BindingFlags.Instance |
                    BindingFlags.NonPublic
                )!;
        
        [Benchmark]
        public string GetValue() => (string)field.GetValue(instance);
    }
}

I guess it's something like this. I'm on laptop now, will try to confirm regression later

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

All the details are here
https://davecallan.com/net-6-v-net-7-reflection-performance-benchmarks/

Reproduction Steps

None

Expected behavior

None

Actual behavior

None

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

Author:	ekalchev
Assignees:	-
Labels:	area-System.Reflection, untriaged
Milestone:	-

[En3Tho]

I'm able to confirm regression on Net7 preview 7 on Ryzen 5950x using benchmark above.

BenchmarkDotNet=v0.13.1, OS=Windows 10.0.22000
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK=7.0.100-preview.7.22377.5
  [Host] : .NET 7.0.0 (7.0.22.37506), X64 RyuJIT DEBUG
  Net6   : .NET 6.0.8 (6.0.822.36306), X64 RyuJIT
  Net7   : .NET 7.0.0 (7.0.22.37506), X64 RyuJIT


|   Method |  Job |  Runtime |     Mean |    Error |   StdDev | Code Size | Allocated |
|--------- |----- |--------- |---------:|---------:|---------:|----------:|----------:|
| GetValue | Net6 | .NET 6.0 | 17.47 ns | 0.017 ns | 0.013 ns |      71 B |         - |
| GetValue | Net7 | .NET 7.0 | 22.95 ns | 0.066 ns | 0.062 ns |      72 B |         - |

[En3Tho]

I believe this is because of Reflection changes that introduced FieldAccessor to RTFieldInfo. GetValue call now uses Invoker (FieldAccessor) property which always null checks and lazily creates FieldAccessor and then uses it to call back to itself's GetValueNonEmit method. Also, before logic of non-emit was inlined into GetValue, now it's in another method. So I guess those 5ns are this additional null check and 2 calls? Not sure JIT can totally optimize this.

Old code
Just RTFieldInfo.GetValue

New code
RTFieldInfo.GetValue -> RTFIeldInfo.Accessor -> FieldAccessor.GetValue -> back to RTFieldInfo.GetValueNonEmit

There is a // Todo: add strategy for calling IL Emit-based version in FieldAccessor. But I'm not sure it will be implemented in Net7? So maybe just revert those changes partially to fix the regression?

P.S. I hope I was looking into right class/code -_-

[steveharter]

I was actually looking at this due to recent bechmarks in dotnet/performance#2561 which shows the regression, primarily in GetValue:

|                                               Method |        Job |                                                                                                   Toolchain |       Mean |     Error |     StdDev |     Median |        Min |        Max | Ratio | RatioSD |  Gen 0 | Allocated | Alloc Ratio |
|----------------------------------------------------- |----------- |------------------------------------------------------------------------------------------------------------ |-----------:|----------:|-----------:|-----------:|-----------:|-----------:|------:|--------:|-------:|----------:|------------:|
|                                        Field_Get_int | Job-BQDNCA | \runtime60\artifacts\bin\testhost\net6.0-windows-Release-x64\shared\Microsoft.NETCore.App\6.0.9\corerun.exe |  30.993 ns | 0.1622 ns |  0.1438 ns |  30.972 ns |  30.768 ns |  31.259 ns |  0.87 |    0.01 | 0.0022 |      24 B |        1.00 |
|                                        Field_Get_int | Job-RQCXYM |   \runtime\artifacts\bin\testhost\net7.0-windows-Release-x64\shared\Microsoft.NETCore.App\7.0.0\corerun.exe |  35.749 ns | 0.2806 ns |  0.2487 ns |  35.752 ns |  35.461 ns |  36.344 ns |  1.00 |    0.00 | 0.0021 |      24 B |        1.00 |
|                                                      |            |                                                                                                             |            |           |            |            |            |            |       |         |        |           |             |
|                                        Field_Set_int | Job-BQDNCA | \runtime60\artifacts\bin\testhost\net6.0-windows-Release-x64\shared\Microsoft.NETCore.App\6.0.9\corerun.exe |  36.037 ns | 0.2146 ns |  0.1903 ns |  36.018 ns |  35.744 ns |  36.418 ns |  0.98 |    0.01 | 0.0023 |      24 B |        1.00 |
|                                        Field_Set_int | Job-RQCXYM |   \runtime\artifacts\bin\testhost\net7.0-windows-Release-x64\shared\Microsoft.NETCore.App\7.0.0\corerun.exe |  36.766 ns | 0.2736 ns |  0.2425 ns |  36.666 ns |  36.378 ns |  37.268 ns |  1.00 |    0.00 | 0.0023 |      24 B |        1.00 |