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

[Perf -6%] Regression in System.Text.Encodings.Web.Tests.Perf_Encoders #48519

Closed
DrewScoggins opened this issue Feb 19, 2021 · 7 comments · Fixed by #49373
Closed

[Perf -6%] Regression in System.Text.Encodings.Web.Tests.Perf_Encoders #48519

DrewScoggins opened this issue Feb 19, 2021 · 7 comments · Fixed by #49373
Assignees
@GrabYourPitchforks
Labels
arch-x64 arch-x86 area-System.Text.Encodings.Web os-windows tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark untriaged New issue has not been triaged by the area owner

Comments

@DrewScoggins
Copy link
Member

Run Information

Architecture x86
OS Windows 10.0.18362
Baseline 86835b839ca255c6c157beebda665f562e4def26
Compare 80b3a3f03f2a4f022b7d3a9ca35bfbc60a3bf965

Regressions in System.Text.Encodings.Web.Tests.Perf_Encoders

Benchmark Baseline Test Test/Base Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
EncodeUtf16 82.27 ns 86.92 ns 1.06 879.8472625460963 917.9081902745216 1.043258562421715 Trace Trace

graph
Historical Data in Reporting System

Repro

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f netcoreapp5.0 --filter 'System.Text.Encodings.Web.Tests.Perf_Encoders*'

.

Payloads

Baseline
Compare

Histogram

System.Text.Encodings.Web.Tests.Perf_Encoders.EncodeUtf16(arguments: UnsafeRelaxed,hello "there",16)

[79.590 ; 81.230) | @@@@@@@@@@@@@@@@@@
[81.230 ; 84.049) | @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
[84.049 ; 86.751) | @@@@@@@@@@@@@@@@@@@@@@@@@@
[86.751 ; 89.664) | @@@@@@@@@@@@@@@@@@@@@
[89.664 ; 93.595) | @@@@@@@

Baseline Jit Disasm

; System.Text.Encodings.Web.Tests.Perf_Encoders.EncodeUtf16(EncoderArguments)
       mov       ecx,edx
       cmp       [ecx],ecx
       call      System.Text.Encodings.Web.Tests.Perf_Encoders+EncoderArguments.EncodeUtf16()
       ret
; Total bytes of code 10
; System.Text.Encodings.Web.Tests.Perf_Encoders+EncoderArguments.EncodeUtf16()
       push      ebp
       mov       ebp,esp
       push      edi
       push      esi
       push      ebx
       sub       esp,8
       xor       eax,eax
       mov       [ebp+0FFF0],eax
       mov       [ebp+0FFEC],eax
       mov       eax,[ecx+8]
       mov       edx,[ecx+0C]
       test      edx,edx
       jne       short M01_L00
       xor       edx,edx
       xor       esi,esi
       jmp       short M01_L01
M01_L00:
       lea       esi,[edx+8]
       mov       edx,[edx+4]
       xchg      edx,esi
M01_L01:
       mov       ecx,[ecx+10]
       test      ecx,ecx
       jne       short M01_L02
       xor       edi,edi
       xor       ebx,ebx
       jmp       short M01_L03
M01_L02:
       lea       edi,[ecx+8]
       mov       ebx,[ecx+4]
M01_L03:
       push      esi
       push      edx
       push      ebx
       push      edi
       lea       edx,[ebp+0FFEC]
       push      edx
       push      1
       lea       edx,[ebp+0FFF0]
       mov       ecx,eax
       mov       eax,[eax]
       mov       eax,[eax+2C]
       call      dword ptr [eax+10]
       lea       esp,[ebp+0FFF4]
       pop       ebx
       pop       esi
       pop       edi
       pop       ebp
       ret
; Total bytes of code 91

Compare Jit Disasm

; System.Text.Encodings.Web.Tests.Perf_Encoders.EncodeUtf16(EncoderArguments)
       mov       ecx,edx
       cmp       [ecx],ecx
       call      System.Text.Encodings.Web.Tests.Perf_Encoders+EncoderArguments.EncodeUtf16()
       ret
; Total bytes of code 10
; System.Text.Encodings.Web.Tests.Perf_Encoders+EncoderArguments.EncodeUtf16()
       push      ebp
       mov       ebp,esp
       push      edi
       push      esi
       push      ebx
       sub       esp,8
       xor       eax,eax
       mov       [ebp+0FFF0],eax
       mov       [ebp+0FFEC],eax
       mov       eax,[ecx+8]
       mov       edx,[ecx+0C]
       test      edx,edx
       jne       short M01_L00
       xor       edx,edx
       xor       esi,esi
       jmp       short M01_L01
M01_L00:
       lea       esi,[edx+8]
       mov       edx,[edx+4]
       xchg      edx,esi
M01_L01:
       mov       ecx,[ecx+10]
       test      ecx,ecx
       jne       short M01_L02
       xor       edi,edi
       xor       ebx,ebx
       jmp       short M01_L03
M01_L02:
       lea       edi,[ecx+8]
       mov       ebx,[ecx+4]
M01_L03:
       push      esi
       push      edx
       push      ebx
       push      edi
       lea       edx,[ebp+0FFEC]
       push      edx
       push      1
       lea       edx,[ebp+0FFF0]
       mov       ecx,eax
       mov       eax,[eax]
       mov       eax,[eax+2C]
       call      dword ptr [eax+10]
       lea       esp,[ebp+0FFF4]
       pop       ebx
       pop       esi
       pop       edi
       pop       ebp
       ret
; Total bytes of code 91

Docs

Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository
@DrewScoggins DrewScoggins added arch-x86 os-windows tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark arch-x64 labels Feb 19, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner labels Feb 19, 2021
@DrewScoggins
Copy link
Member Author

This seems related to #48181

@danmoseley
Copy link
Member

@eerhardt

@danmoseley
Copy link
Member

If this is an inevitable consequence of removing the unsafe code, maybe it's acceptable., cc @GrabYourPitchforks since it's relating to unsafe code and encoding :)

@JulieLeeMSFT
Copy link
Member

As Dan said, it is text encoding related. I wonder if we need to change the codegen label to something else.

@GrabYourPitchforks
Copy link
Member

Been investigating potential perf optimizations recently for this code. Self-assigning since it falls under the same umbrella.

Regardless, I think the PR that introduced this was good for a whole slew of reasons, including memory and footprint savings.

@GrabYourPitchforks GrabYourPitchforks self-assigned this Feb 22, 2021
@danmoseley danmoseley added area-System.Text.Encodings.Web and removed area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Feb 22, 2021
@ghost
Copy link

ghost commented Feb 22, 2021

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

Issue Details

Run Information

Architecture x86
OS Windows 10.0.18362
Baseline 86835b839ca255c6c157beebda665f562e4def26
Compare 80b3a3f03f2a4f022b7d3a9ca35bfbc60a3bf965

Regressions in System.Text.Encodings.Web.Tests.Perf_Encoders

Benchmark Baseline Test Test/Base Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
EncodeUtf16 82.27 ns 86.92 ns 1.06 879.8472625460963 917.9081902745216 1.043258562421715 Trace Trace

graph
Historical Data in Reporting System

Repro

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f netcoreapp5.0 --filter 'System.Text.Encodings.Web.Tests.Perf_Encoders*'

.

Payloads

Baseline
Compare

Histogram

System.Text.Encodings.Web.Tests.Perf_Encoders.EncodeUtf16(arguments: UnsafeRelaxed,hello "there",16)

[79.590 ; 81.230) | @@@@@@@@@@@@@@@@@@
[81.230 ; 84.049) | @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
[84.049 ; 86.751) | @@@@@@@@@@@@@@@@@@@@@@@@@@
[86.751 ; 89.664) | @@@@@@@@@@@@@@@@@@@@@
[89.664 ; 93.595) | @@@@@@@

Baseline Jit Disasm

; System.Text.Encodings.Web.Tests.Perf_Encoders.EncodeUtf16(EncoderArguments)
       mov       ecx,edx
       cmp       [ecx],ecx
       call      System.Text.Encodings.Web.Tests.Perf_Encoders+EncoderArguments.EncodeUtf16()
       ret
; Total bytes of code 10
; System.Text.Encodings.Web.Tests.Perf_Encoders+EncoderArguments.EncodeUtf16()
       push      ebp
       mov       ebp,esp
       push      edi
       push      esi
       push      ebx
       sub       esp,8
       xor       eax,eax
       mov       [ebp+0FFF0],eax
       mov       [ebp+0FFEC],eax
       mov       eax,[ecx+8]
       mov       edx,[ecx+0C]
       test      edx,edx
       jne       short M01_L00
       xor       edx,edx
       xor       esi,esi
       jmp       short M01_L01
M01_L00:
       lea       esi,[edx+8]
       mov       edx,[edx+4]
       xchg      edx,esi
M01_L01:
       mov       ecx,[ecx+10]
       test      ecx,ecx
       jne       short M01_L02
       xor       edi,edi
       xor       ebx,ebx
       jmp       short M01_L03
M01_L02:
       lea       edi,[ecx+8]
       mov       ebx,[ecx+4]
M01_L03:
       push      esi
       push      edx
       push      ebx
       push      edi
       lea       edx,[ebp+0FFEC]
       push      edx
       push      1
       lea       edx,[ebp+0FFF0]
       mov       ecx,eax
       mov       eax,[eax]
       mov       eax,[eax+2C]
       call      dword ptr [eax+10]
       lea       esp,[ebp+0FFF4]
       pop       ebx
       pop       esi
       pop       edi
       pop       ebp
       ret
; Total bytes of code 91

Compare Jit Disasm

; System.Text.Encodings.Web.Tests.Perf_Encoders.EncodeUtf16(EncoderArguments)
       mov       ecx,edx
       cmp       [ecx],ecx
       call      System.Text.Encodings.Web.Tests.Perf_Encoders+EncoderArguments.EncodeUtf16()
       ret
; Total bytes of code 10
; System.Text.Encodings.Web.Tests.Perf_Encoders+EncoderArguments.EncodeUtf16()
       push      ebp
       mov       ebp,esp
       push      edi
       push      esi
       push      ebx
       sub       esp,8
       xor       eax,eax
       mov       [ebp+0FFF0],eax
       mov       [ebp+0FFEC],eax
       mov       eax,[ecx+8]
       mov       edx,[ecx+0C]
       test      edx,edx
       jne       short M01_L00
       xor       edx,edx
       xor       esi,esi
       jmp       short M01_L01
M01_L00:
       lea       esi,[edx+8]
       mov       edx,[edx+4]
       xchg      edx,esi
M01_L01:
       mov       ecx,[ecx+10]
       test      ecx,ecx
       jne       short M01_L02
       xor       edi,edi
       xor       ebx,ebx
       jmp       short M01_L03
M01_L02:
       lea       edi,[ecx+8]
       mov       ebx,[ecx+4]
M01_L03:
       push      esi
       push      edx
       push      ebx
       push      edi
       lea       edx,[ebp+0FFEC]
       push      edx
       push      1
       lea       edx,[ebp+0FFF0]
       mov       ecx,eax
       mov       eax,[eax]
       mov       eax,[eax+2C]
       call      dword ptr [eax+10]
       lea       esp,[ebp+0FFF4]
       pop       ebx
       pop       esi
       pop       edi
       pop       ebp
       ret
; Total bytes of code 91

Docs

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

Author: DrewScoggins
Assignees: GrabYourPitchforks
Labels:

arch-x64, arch-x86, area-System.Text.Encodings.Web, os-windows, tenet-performance, tenet-performance-benchmarks, untriaged
Milestone: -

@danmoseley
Copy link
Member

Fixed area label.

@GrabYourPitchforks GrabYourPitchforks mentioned this issue Mar 9, 2021
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Mar 9, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 19, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@GrabYourPitchforks GrabYourPitchforks

Labels
arch-x64 arch-x86 area-System.Text.Encodings.Web os-windows tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark untriaged New issue has not been triaged by the area owner
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

System.Text.Encodings.Web refactoring and code modernization GrabYourPitchforks/runtime
4 participants
@GrabYourPitchforks @danmoseley @DrewScoggins @JulieLeeMSFT