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

Regression in System.Drawing.Tests.Perf_Color #58443

Closed
performanceautofiler bot opened this issue Aug 30, 2021 · 12 comments
Closed

Regression in System.Drawing.Tests.Perf_Color #58443

performanceautofiler bot opened this issue Aug 30, 2021 · 12 comments
Assignees
Labels
arch-x86 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI os-windows
Milestone

Comments

@performanceautofiler
Copy link

Run Information

Architecture x86
OS Windows 10.0.18362
Baseline 5a5b1045ab210852c19849f1dbaffa3a3a668166
Compare a67259dfe4c7a0b292c8533af5a36e27b7e2e080
Diff Diff

Regressions in System.Drawing.Tests.Perf_Color

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
GetHue - Duration of single invocation 1.15 μs 1.27 μs 1.11 0.05 False

graph
Historical Data in Reporting System

Repro

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

Payloads

Baseline
Compare

Histogram

System.Drawing.Tests.Perf_Color.GetHue


Docs

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

@kunalspathak kunalspathak changed the title [Perf] Changes at 8/25/2021 3:32:50 AM Regression in System.Drawing.Tests.Perf_Color Aug 31, 2021
@kunalspathak kunalspathak transferred this issue from dotnet/perf-autofiling-issues Aug 31, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Drawing untriaged New issue has not been triaged by the area owner labels Aug 31, 2021
@ghost
Copy link

ghost commented Aug 31, 2021

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

Issue Details

Run Information

Architecture x86
OS Windows 10.0.18362
Baseline 5a5b1045ab210852c19849f1dbaffa3a3a668166
Compare a67259dfe4c7a0b292c8533af5a36e27b7e2e080
Diff Diff

Regressions in System.Drawing.Tests.Perf_Color

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
GetHue - Duration of single invocation 1.15 μs 1.27 μs 1.11 0.05 False

graph
Historical Data in Reporting System

Repro

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

Payloads

Baseline
Compare

Histogram

System.Drawing.Tests.Perf_Color.GetHue


Docs

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

Author: performanceautofiler[bot]
Assignees: -
Labels:

area-System.Drawing, untriaged

Milestone: -

@kunalspathak
Copy link
Member

Possibly from #57094. @SingleAccretion

@SingleAccretion
Copy link
Contributor

Skimming the code, hard to see how where it comes from, nevertheless, I will take a look. Note that I may not have as much free time going forward, so it'll take a bit. I presume that's ok since this is a 7.0-specific regression.

@kunalspathak
Copy link
Member

This is windows/x86 specific...can you double check using SPMI if there are asmdiffs from your change on windows/x86?

@tarekgh tarekgh added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed area-System.Drawing labels Aug 31, 2021
@ghost
Copy link

ghost commented Aug 31, 2021

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

Issue Details

Run Information

Architecture x86
OS Windows 10.0.18362
Baseline 5a5b1045ab210852c19849f1dbaffa3a3a668166
Compare a67259dfe4c7a0b292c8533af5a36e27b7e2e080
Diff Diff

Regressions in System.Drawing.Tests.Perf_Color

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
GetHue - Duration of single invocation 1.15 μs 1.27 μs 1.11 0.05 False

graph
Historical Data in Reporting System

Repro

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

Payloads

Baseline
Compare

Histogram

System.Drawing.Tests.Perf_Color.GetHue


Docs

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

Author: performanceautofiler[bot]
Assignees: -
Labels:

arch-x86, os-windows, area-CodeGen-coreclr, untriaged

Milestone: -

@tarekgh
Copy link
Member

tarekgh commented Aug 31, 2021

Looking at all commits, and nothing related to System.Drawing. Changed the label to codegen as the commits there mainly related to codegen.

@SingleAccretion
Copy link
Contributor

SingleAccretion commented Aug 31, 2021

So, as expected, no SPMI diffs for my change.

I can reproduce the regression locally, yet the codegen (for now obtained via BDN) is identical.
Indeed, the dumps for the methods in question are identical as well. Puzzling.
And if I try with --iterationTime 1000 (or 500 for that matter), the regression disappears... I will stop my investigation at this point.

@JulieLeeMSFT JulieLeeMSFT removed the untriaged New issue has not been triaged by the area owner label Aug 31, 2021
@JulieLeeMSFT JulieLeeMSFT added this to the 6.0.0 milestone Aug 31, 2021
@kunalspathak
Copy link
Member

Looks to be noisy, most likely will close it.

image

@kunalspathak
Copy link
Member

I can reproduce the regression locally, yet the codegen (for now obtained via BDN) is identical.

Are you able to reproduce regression with vs. without your change?

@SingleAccretion
Copy link
Contributor

SingleAccretion commented Sep 2, 2021

Are you able to reproduce regression with vs. without your change?

Yes, that was how it was originally, but higher --iterationTime washes away the difference, so I suspect some systematic error got in the way. I also note I was not able to find any codegen differences in the calling stack of the benchmark (for my-change-vs-without-my-change or for the first commit in the cited diff vs the last).

@kunalspathak
Copy link
Member

I double checked and I don't see any asm difference in benchmark. Also, it seems historically there are spikes in the test.

image

From code alignment perspective the loop inside benchmark method GetHue() itself doesn't get aligned because there is a call in it, but looking at it, it looks like there is lot of fetching / decoding going on because the loop body is spread.

Unrelated to this regression, but we should rearrange the code such that the entire loop code is present contiguous. It looks to me that the new blocks are added by LSRA resolution phase.

                                                ;; bbWeight=1    PerfScore 5.50
G_M63925_IG02:              ;; offset=000CH
 0ac704cc        C5F857C0     vxorps   xmm0, xmm0
 0ac704d0        C5FA1145F0   vmovss   dword ptr [ebp-10H], xmm0
 0ac704d5        8B35F4329407 mov      esi, gword ptr [classVar[0xac355b4]]
 0ac704db        33FF         xor      edi, edi
 0ac704dd        8B5E04       mov      ebx, dword ptr [esi+4]
; ............................... 32B boundary ...............................
 0ac704e0        85DB         test     ebx, ebx
 0ac704e2        7E3B         jle      SHORT G_M63925_IG07
                                                ;; bbWeight=1    PerfScore 5.33
G_M63925_IG03:              ;; offset=0024H
 0ac704e4        8BCF         mov      ecx, edi
 0ac704e6        C1E104       shl      ecx, 4
 0ac704e9        8D4C0E08     lea      ecx, bword ptr [esi+ecx+8]
 0ac704ed        E876B44FFE   call     System.Drawing.Color:GetHue():float:this
 0ac704f2        D95DEC       fstp     dword ptr [ebp-14H]
 0ac704f5        C5FA1045EC   vmovss   xmm0, dword ptr [ebp-14H]
 0ac704fa        C5FA5845F0   vaddss   xmm0, xmm0, dword ptr [ebp-10H]
 0ac704ff        47           inc      edi
; ............................... 32B boundary ...............................
 0ac70500        3BDF         cmp      ebx, edi
 0ac70502        7F11         jg       SHORT G_M63925_IG06
                                                ;; bbWeight=4    PerfScore 47.00
G_M63925_IG04:              ;; offset=0044H
 0ac70504        C5FA1145F0   vmovss   dword ptr [ebp-10H], xmm0
 0ac70509        D945F0       fld      dword ptr [ebp-10H]
                                                ;; bbWeight=1    PerfScore 1.00
G_M63925_IG05:              ;; offset=004CH
 0ac7050c        8D65F4       lea      esp, [ebp-0CH]
 0ac7050f        5B           pop      ebx
 0ac70510        5E           pop      esi
 0ac70511        5F           pop      edi
 0ac70512        5D           pop      ebp
 0ac70513        C3           ret
                                                ;; bbWeight=1    PerfScore 3.50
G_M63925_IG06:              ;; offset=0054H
 0ac70514        C5FA1145F0   vmovss   dword ptr [ebp-10H], xmm0
 0ac70519        EBC9         jmp      SHORT G_M63925_IG03
                                                ;; bbWeight=2    PerfScore 5.00
G_M63925_IG07:              ;; offset=005BH
 0ac7051b        C5FA1045F0   vmovss   xmm0, dword ptr [ebp-10H]
; ............................... 32B boundary ...............................
 0ac70520        EBE2         jmp      SHORT G_M63925_IG04
                                                ;; bbWeight=0.50 PerfScore 2.00

@kunalspathak
Copy link
Member

As such, I will go ahead and close this issue as there is nothing actionable.

@ghost ghost locked as resolved and limited conversation to collaborators Oct 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-x86 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI os-windows
Projects
None yet
Development

No branches or pull requests

4 participants