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.

Sign up for GitHub

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

Jump to bottom

Regressions in System.Collections.Sort<Int32> #69732

Closed
performanceautofiler bot opened this issue May 24, 2022 · 8 comments
Closed

Regressions in System.Collections.Sort<Int32> #69732

performanceautofiler bot opened this issue May 24, 2022 · 8 comments
Assignees
@kunalspathak
Labels
arch-x64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI PGO runtime-coreclr specific to the CoreCLR runtime tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark
Milestone
7.0.0

Comments

@performanceautofiler
Copy link

Run Information

Architecture x64
OS Windows 10.0.18362
Baseline a0ff92c8d3cb3cb3b35357117e629cdd19f7080a
Compare cd6512d071d0a32c90fa81a64abd258d707cfa5f
Diff Diff

Regressions in System.Collections.Sort<Int32>

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
Array_ComparerClass - Duration of single invocation 27.47 μs 29.30 μs 1.07 0.01 False

graph
Test Report

Repro

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Collections.Sort&lt;Int32&gt;*'

Payloads

Baseline
Compare

Histogram

System.Collections.Sort<Int32>.Array_ComparerClass(Size: 512)

Description of detection logic

IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsRegressionBase: Marked as regression because the compare was 5% greater than the baseline, and the value was not too small.
IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsRegressionWindowed: Marked as regression because 29.298916923076927 > 28.86969814615385.
IsChangePoint: Marked as a change because one of 4/20/2022 8:23:28 AM, 5/23/2022 3:43:39 PM, 5/24/2022 7:58:36 AM falls between 5/15/2022 6:57:22 PM and 5/24/2022 7:58:36 AM.
IsRegressionStdDev: Marked as regression because -50.203827738710984 (T) = (0 -29295.707692307697) / Math.Sqrt((91015.60543795506 / (55)) + (20.598324260357977 / (2))) is less than -2.0040447832881556 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (55) + (2) - 2, .025) and -0.07518661710402194 = (27247.091087512486 - 29295.707692307697) / 27247.091087512486 is less than -0.05.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsChangeEdgeDetector: Marked not as a regression because Edge Detector said so.

Docs

Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository
@performanceautofiler performanceautofiler bot added CoreClr PGO untriaged New issue has not been triaged by the area owner labels May 24, 2022
@kunalspathak kunalspathak changed the title [Perf] Changes at 5/23/2022 7:50:08 PM Regressions in System.Collections.Sort<Int32> May 24, 2022
@kunalspathak kunalspathak transferred this issue from dotnet/perf-autofiling-issues May 24, 2022
@dotnet-issue-labeler
Copy link

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
Copy link
Member

Possibly from #69247 @jakobbotsch

@kunalspathak kunalspathak added tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark labels May 24, 2022
@jakobbotsch
Copy link
Member

jakobbotsch commented May 24, 2022 •
edited
Loading

This diff in System.Collections.Generic.ArraySortHelper`1[Int32][System.Int32]:IntroSort(System.Span`1[Int32],int,System.Comparison`1[Int32]) is the only relevant one I can find:

@@ -201,9 +201,7 @@ G_M3347_IG10:        ; gcrefRegs=00000040 {rsi}, byrefRegs=00000008 {rbx}, byref
 G_M3347_IG11:        ; gcrefRegs=00000040 {rsi}, byrefRegs=00000008 {rbx}, byref
        ; gcrRegs +[rsi]
        ; byrRegs +[rbx]
-       mov      ecx, r14d
-       mov      eax, ebp
-       cmp      rcx, rax
+       cmp      r14d, ebp
        ja       G_M3347_IG14
        mov      ecx, r14d
        not      ecx
@@ -221,16 +219,14 @@ G_M3347_IG11:        ; gcrefRegs=00000040 {rsi}, byrefRegs=00000008 {rbx}, byref
        ; byrRegs -[rcx rbx]
        ; gcr arg pop 0
        jmp      G_M3347_IG16
-						;; size=57 bbWeight=0    PerfScore 0.00
+						;; size=52 bbWeight=0    PerfScore 0.00
 G_M3347_IG12:        ; gcrefRegs=00000040 {rsi}, byrefRegs=00000008 {rbx}, byref, isz
        ; gcrRegs +[rsi]
        ; byrRegs +[rbx]
        test     edi, edi
        jne      SHORT G_M3347_IG13
-       mov      ecx, r14d
-       mov      eax, ebp
-       cmp      rcx, rax
-       ja       G_M3347_IG14
+       cmp      r14d, ebp
+       ja       SHORT G_M3347_IG14
        mov      ecx, r14d
        not      ecx
        shr      ecx, 31
@@ -249,14 +245,12 @@ G_M3347_IG12:        ; gcrefRegs=00000040 {rsi}, byrefRegs=00000008 {rbx}, byref
        ; byrRegs -[rcx rbx]
        ; gcr arg pop 0
        jmp      G_M3347_IG16
-						;; size=66 bbWeight=0    PerfScore 0.00
+						;; size=57 bbWeight=0    PerfScore 0.00
 G_M3347_IG13:        ; gcrefRegs=00000040 {rsi}, byrefRegs=00000008 {rbx}, byref, isz
        ; gcrRegs +[rsi]
        ; byrRegs +[rbx]
        dec      edi
-       mov      ecx, r14d
-       mov      eax, ebp
-       cmp      rcx, rax
+       cmp      r14d, ebp
        ja       SHORT G_M3347_IG14
        mov      ecx, r14d
        not      ecx
@@ -283,7 +277,7 @@ G_M3347_IG13:        ; gcrefRegs=00000040 {rsi}, byrefRegs=00000008 {rbx}, byref
        mov      edx, ebp
        cmp      rax, rdx
        jbe      SHORT G_M3347_IG15
-						;; size=78 bbWeight=0    PerfScore 0.00
+						;; size=73 bbWeight=0    PerfScore 0.00
 G_M3347_IG14:        ; gcrefRegs=00000000 {}, byrefRegs=00000000 {}, byref
        ; gcrRegs -[rsi]
        ; byrRegs -[rbx]
@@ -331,7 +325,7 @@ G_M3347_IG16:        ; gcrefRegs=00000000 {}, byrefRegs=00000000 {}, byref, epil
        ret      
 						;; size=15 bbWeight=0    PerfScore 0.00
 
-; Total bytes of code 569, prolog size 21, PerfScore 91.65, instruction count 153, allocated bytes for code 569 (MethodHash=3d07f2ec) for method System.Collections.Generic.ArraySortHelper`1[Int32][System.Int32]:IntroSort(System.Span`1[Int32],int,System.Comparison`1[Int32])
+; Total bytes of code 550, prolog size 21, PerfScore 89.75, instruction count 147, allocated bytes for code 550 (MethodHash=3d07f2ec) for method System.Collections.Generic.ArraySortHelper`1[Int32][System.Int32]:IntroSort(System.Span`1[Int32],int,System.Comparison`1[Int32])
 ; ============================================================
 
 Unwind Info:

But better perf score and lower code size, so not sure if this is the cause.

@jeffschwMSFT jeffschwMSFT added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 25, 2022
@ghost
Copy link

ghost commented May 25, 2022

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.18362
Baseline a0ff92c8d3cb3cb3b35357117e629cdd19f7080a
Compare cd6512d071d0a32c90fa81a64abd258d707cfa5f
Diff Diff

Regressions in System.Collections.Sort<Int32>

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
Array_ComparerClass - Duration of single invocation 27.47 μs 29.30 μs 1.07 0.01 False

graph
Test Report

Repro

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Collections.Sort&lt;Int32&gt;*'

Payloads

Baseline
Compare

Histogram

System.Collections.Sort<Int32>.Array_ComparerClass(Size: 512)

Description of detection logic

IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsRegressionBase: Marked as regression because the compare was 5% greater than the baseline, and the value was not too small.
IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsRegressionWindowed: Marked as regression because 29.298916923076927 > 28.86969814615385.
IsChangePoint: Marked as a change because one of 4/20/2022 8:23:28 AM, 5/23/2022 3:43:39 PM, 5/24/2022 7:58:36 AM falls between 5/15/2022 6:57:22 PM and 5/24/2022 7:58:36 AM.
IsRegressionStdDev: Marked as regression because -50.203827738710984 (T) = (0 -29295.707692307697) / Math.Sqrt((91015.60543795506 / (55)) + (20.598324260357977 / (2))) is less than -2.0040447832881556 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (55) + (2) - 2, .025) and -0.07518661710402194 = (27247.091087512486 - 29295.707692307697) / 27247.091087512486 is less than -0.05.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsChangeEdgeDetector: Marked not as a regression because Edge Detector said so.

Docs

Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository

Author: performanceautofiler[bot]
Assignees: kunalspathak
Labels:

tenet-performance, tenet-performance-benchmarks, area-CodeGen-coreclr, untriaged, refs/heads/main, RunKind=micro, Windows 10.0.18362, PGO, Regression, CoreClr, x64
Milestone: -

@JulieLeeMSFT JulieLeeMSFT removed the untriaged New issue has not been triaged by the area owner label May 25, 2022
@JulieLeeMSFT JulieLeeMSFT added this to the 7.0.0 milestone May 25, 2022
@kunalspathak
Copy link
Member

It could be noisy. I will wait for few weeks to see the trend before closing this.

@dakersnar
Copy link
Contributor

Arm64 regressions possibly caused by this update: dotnet/perf-autofiling-issues#5542

@kunalspathak
Copy link
Member

The only culprit I can think of is #69127 but I don't have a reason to believe why they would introduce regressions.

@kunalspathak
Copy link
Member

I don't think there is anything actionable to do here.

@ghost ghost locked as resolved and limited conversation to collaborators Aug 18, 2022
@jeffhandley jeffhandley added runtime-coreclr specific to the CoreCLR runtime and removed CoreClr labels Dec 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@kunalspathak kunalspathak

Labels
arch-x64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI PGO runtime-coreclr specific to the CoreCLR runtime tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark
Projects
None yet
Milestone
7.0.0
Development

No branches or pull requests
6 participants
@jeffhandley @jakobbotsch @jeffschwMSFT @kunalspathak @dakersnar @JulieLeeMSFT