Regressions in System.Tests.Perf_Type

[performanceautofiler[bot]]

Run Information

Architecture	x64
OS	Windows 10.0.19042
Baseline	5181e1cd5b3fde9da15fe3f8d3eceaa9d3f64e01
Compare	bfaa457c804efb9247f495b7e04444b2bf216308
Diff	Diff

Regressions in System.Tests.Perf_Type

Benchmark	Baseline	Test	Test/Base	Test Quality	Edge Detector	Baseline IR	Compare IR	IR Ratio	Baseline ETL	Compare ETL
[op_Equality - Duration of single invocation](<https://pvscmdupload.blob.core.windows.net/reports/allTestHistory/refs/heads/main_x64_Windows 10.0.19042/amd/System.Tests.Perf_Type.op_Equality.html>)	1.38 ns	3.08 ns	2.24	0.13	True

Test Report

Repro

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Tests.Perf_Type*'

### Payloads [Baseline]() [Compare]() ### Histogram #### System.Tests.Perf_Type.op_Equality ```log ``` ### Docs [Profiling workflow for dotnet/runtime repository](https://github.com/dotnet/performance/blob/master/docs/profiling-workflow-dotnet-runtime.md) [Benchmarking workflow for dotnet/runtime repository](https://github.com/dotnet/performance/blob/master/docs/benchmarking-workflow-dotnet-runtime.md)

Run Information

Architecture	x64
OS	Windows 10.0.19042
Baseline	5181e1cd5b3fde9da15fe3f8d3eceaa9d3f64e01
Compare	bfaa457c804efb9247f495b7e04444b2bf216308
Diff	Diff

[kunalspathak]

Type equality is related to https://github.com/dotnet/runtime/pull/59499

[kunalspathak]

area of improvement: Align methods that are recursion - Ackermann benchmark? Double check if we set fgHasLoops flag when we convert a recursive method to loops

[dotnet-issue-labeler[bot]]

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.

[kunalspathak]

Caused by https://github.com/dotnet/runtime/pull/59499

[kunalspathak]

Also - https://github.com/dotnet/perf-autofiling-issues/issues/1577

[EgorBo]

I guess the native implementation could potentially benefit from Native PGO? since it's a question of nanoseconds (or maybe it's Managed PGO that made C# impl worse). As a quick fix we can mark operator== as AggressiveInlining if it worth it

cc @jkotas

Benchmark: https://github.com/dotnet/performance/blob/d7dac8a7ca12a28d099192f8a901cf8e30361384/src/benchmarks/micro/libraries/System.Runtime/Perf.Type.cs

operator == should exit at if (left is RuntimeType || right is RuntimeType) check

[jkotas]

The regression is caused by several factors:

• Tiered JIT overhead. Managed->managed calls go through an extra indirection. This extra indirection was not there for the FCall implementation. Not sure what we can do about it.
• Unnecessary register shuffling. Current code:

  mov     rax,rcx
  mov     r8,qword ptr [rax]
  mov rax,7FF9CBD49280h
  cmp     r8,rax

  This can be just:

  mov     r8,7FF9CBD49280h
  cmp     [rcx],r8

  @EgorBo How hard would be to fix this in the JIT?

• Extra jump due to duplicated epilogs. I will submit PR to fix that.

As a quick fix we can mark operator== as AggressiveInlining

I think it would break late recognition of the operator== intrinsic. (Also, the AggressiveInlining does not feel like the right tradeoff here.)

[ghost]

Tagging subscribers to this area: @JulieLeeMSFT See info in area-owners.md if you want to be subscribed.

Issue Details

---

### Run Information Architecture | x64 -- | -- OS | Windows 10.0.19042 Baseline | [5181e1cd5b3fde9da15fe3f8d3eceaa9d3f64e01](https://github.com/dotnet/runtime/commit/5181e1cd5b3fde9da15fe3f8d3eceaa9d3f64e01) Compare | [bfaa457c804efb9247f495b7e04444b2bf216308](https://github.com/dotnet/runtime/commit/bfaa457c804efb9247f495b7e04444b2bf216308) Diff | [Diff](https://github.com/dotnet/runtime/compare/5181e1cd5b3fde9da15fe3f8d3eceaa9d3f64e01...bfaa457c804efb9247f495b7e04444b2bf216308) ### Regressions in System.Tests.Perf_Type Benchmark | Baseline | Test | Test/Base | Test Quality | Edge Detector | Baseline IR | Compare IR | IR Ratio | Baseline ETL | Compare ETL -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- [op_Equality - Duration of single invocation]() | 1.38 ns | 3.08 ns | 2.24 | 0.13 | True | | | ![graph]() [Test Report]() ### Repro ```cmd git clone https://github.com/dotnet/performance.git py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Tests.Perf_Type*' ```

### Payloads [Baseline]() [Compare]() ### Histogram #### System.Tests.Perf_Type.op_Equality ```log ``` ### Docs [Profiling workflow for dotnet/runtime repository](https://github.com/dotnet/performance/blob/master/docs/profiling-workflow-dotnet-runtime.md) [Benchmarking workflow for dotnet/runtime repository](https://github.com/dotnet/performance/blob/master/docs/benchmarking-workflow-dotnet-runtime.md)

### Run Information Architecture | x64 -- | -- OS | Windows 10.0.19042 Baseline | [5181e1cd5b3fde9da15fe3f8d3eceaa9d3f64e01](https://github.com/dotnet/runtime/commit/5181e1cd5b3fde9da15fe3f8d3eceaa9d3f64e01) Compare | [bfaa457c804efb9247f495b7e04444b2bf216308](https://github.com/dotnet/runtime/commit/bfaa457c804efb9247f495b7e04444b2bf216308) Diff | [Diff](https://github.com/dotnet/runtime/compare/5181e1cd5b3fde9da15fe3f8d3eceaa9d3f64e01...bfaa457c804efb9247f495b7e04444b2bf216308)

Author:	performanceautofiler[bot]
Assignees:	-
Labels:	`os-windows`, `tenet-performance`, `tenet-performance-benchmarks`, `arch-x64`, `area-CodeGen-coreclr`, `untriaged`
Milestone:	-

[jkotas]

The remaining actionable problem here is the unnecessary register shuffling from is RuntimeType check (see above).

[EgorBo]

@EgorBo How hard would be to fix this in the JIT?

@jkotas from a quick look I guess it's because of the way we inline cast/isinst - isinst in this case is only used for a bool expression but we save its result into a temp always.

STMT00006 (IL 0x003...  ???)
N009 ( 18, 22) [000040] -A---O------              *  JTRUE     void  
N008 ( 16, 20) [000010] JA---O?N----              \--*  EQ        int    $c2
N006 ( 13,  9) [000052] -A---O------                 +--*  COMMA     long   $102
N004 ( 10,  7) [000050] -A---O--R---                 |  +--*  ASG       long   $VN.Void
N003 (  3,  2) [000049] D------N----                 |  |  +--*  LCL_VAR   long   V03 cse0         d:1 $102
N002 (  6,  4) [000009] #----O?-----                 |  |  \--*  IND       long   $102
N001 (  3,  2) [000008] ------?-----                 |  |     \--*  LCL_VAR   ref    V02 tmp1         u:1 $80
N005 (  3,  2) [000051] ------------                 |  \--*  LCL_VAR   long   V03 cse0         u:1 $102
N007 (  2, 10) [000007] H-----?-----                 \--*  CNS_INT(h) long   0x7ffdc8eaf480 class $140

so in theory in this specific case there should not be any COMMA/ASG, just JTRUE(EQ(IND...

there was an issue for it already somewhere.

Simple repro:

using System;
using System.Runtime.CompilerServices;

class Program
{
    static void Main()
    {
        Test("hello");
    }

    [MethodImpl(MethodImplOptions.NoInlining | 
                MethodImplOptions.AggressiveOptimization)]
    public static int Test(object left)
    {
        if (left is null || left is string)
            return 0;
        return left.GetHashCode();
    }
}

[JulieLeeMSFT]

Setting this to .NET 7. Please update it if you think it needs to be addressed in .NET 6. Assigning to @EgorBo since inlining related work remains.

[EgorBo]

Closing as the regression is no more: