Repository navigation
[Perf -6%] Regression in System.Text.Encodings.Web.Tests.Perf_Encoders #48519
Description
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 |
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*'Details
.
Payloads
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 91Compare 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 91Docs
Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository
Activity
DrewScoggins commentedon Feb 19, 2021
This seems related to #48181
danmoseley commentedon Feb 19, 2021
@eerhardt
danmoseley commentedon Feb 19, 2021
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 commentedon Feb 19, 2021
As Dan said, it is text encoding related. I wonder if we need to change the codegen label to something else.
GrabYourPitchforks commentedon Feb 22, 2021
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.
danmoseley commentedon Feb 22, 2021
Fixed area label.