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

[Perf] Regressions in System.Numerics.Tests.Perf_BigInteger #57293

Closed
performanceautofiler bot opened this issue Aug 10, 2021 · 8 comments · Fixed by #57297
Closed

[Perf] Regressions in System.Numerics.Tests.Perf_BigInteger #57293

performanceautofiler bot opened this issue Aug 10, 2021 · 8 comments · Fixed by #57297
Labels
arch-arm64 area-System.Numerics os-linux Linux OS (any supported distro) tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark

Comments

@performanceautofiler
Copy link

Run Information

Architecture arm64
OS ubuntu 18.04
Baseline 749fa1381f122f7a0bb23374a9e220f834ccbbb3
Compare f83a9d9689433ce522b91e74a9631c83847e3b64
Diff Diff

Regressions in System.Numerics.Tests.Perf_BigInteger

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
Ctor_ByteArray - Duration of single invocation 24.62 ns 32.53 ns 1.32 0.43 False
Ctor_ByteArray - Duration of single invocation 12.83 ns 26.17 ns 2.04 0.41 False

graph
graph
Historical Data in Reporting System

Repro

git clone https://github.com/dotnet/performance.git
python3 .\performance\scripts\benchmarks_ci.py -f netcoreapp5.0 --filter 'System.Numerics.Tests.Perf_BigInteger*'

Payloads

Baseline
Compare

Histogram

System.Numerics.Tests.Perf_BigInteger.Ctor_ByteArray(numberString: -2147483648)


System.Numerics.Tests.Perf_BigInteger.Ctor_ByteArray(numberString: 123)


Docs

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

@DrewScoggins DrewScoggins transferred this issue from dotnet/perf-autofiling-issues Aug 12, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Numerics untriaged New issue has not been triaged by the area owner labels Aug 12, 2021
@ghost
Copy link

ghost commented Aug 12, 2021

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

Run Information

Architecture arm64
OS ubuntu 18.04
Baseline 749fa1381f122f7a0bb23374a9e220f834ccbbb3
Compare f83a9d9689433ce522b91e74a9631c83847e3b64
Diff Diff

Regressions in System.Numerics.Tests.Perf_BigInteger

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
Ctor_ByteArray - Duration of single invocation 24.62 ns 32.53 ns 1.32 0.43 False
Ctor_ByteArray - Duration of single invocation 12.83 ns 26.17 ns 2.04 0.41 False

graph
graph
Historical Data in Reporting System

Repro

git clone https://github.com/dotnet/performance.git
python3 .\performance\scripts\benchmarks_ci.py -f netcoreapp5.0 --filter 'System.Numerics.Tests.Perf_BigInteger*'

Payloads

Baseline
Compare

Histogram

System.Numerics.Tests.Perf_BigInteger.Ctor_ByteArray(numberString: -2147483648)


System.Numerics.Tests.Perf_BigInteger.Ctor_ByteArray(numberString: 123)


Docs

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

Author: performanceautofiler[bot]
Assignees: -
Labels:

area-System.Numerics, untriaged

Milestone: -

@DrewScoggins DrewScoggins changed the title [Perf] Changes at 8/2/2021 5:40:25 PM [Perf] Regressions in System.Numerics.Tests.Perf_BigInteger Aug 12, 2021
@DrewScoggins DrewScoggins added arch-arm64 os-linux Linux OS (any supported distro) tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark labels Aug 12, 2021
@DrewScoggins
Copy link
Member

@stephentoub
Copy link
Member

Assuming #53984, as something intended to be a minor cleanup but regressing some benchmarks by 30%-100%, at this point for .NET 6 it should likely be reverted and revisted for .NET 7.
cc: @tannergooding, @huoyaoyuan

@tannergooding
Copy link
Member

Agreed. Additionally, we should likely look into why the regression is so bad as a follow up.

I don't believe its expected that using indexers should be this impactful.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 12, 2021
@stephentoub
Copy link
Member

stephentoub commented Aug 12, 2021

https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AXEBDAzgWwB8ABAJgEYBYAKGIGYACMhgYQYG8aHuGBLAOwwBtALoNgvDLgDcXHvSbkkfQQwCy5ABRsAbtgA2AVxgBKOd07Ue1pgHYGeozAB0EqUIB65EbKs8AvuZMjMRKKhjqpNoOBsZmfhZB1sT2jsaukrhCaS5uuM4AMjD8AOYYABYMcAzevtaB1P5AA
JIT looks happy with index expression and generates slightly better code for it.

FYI, that's on .NET 5 and amd64, rather than .NET 6 and arm64.

@tannergooding
Copy link
Member

Notably, that's a relatively simple example. Looking through other scenarios and directly copying the full expression/context they are used in shows worse codegen in several cases.

I'm investigating more so I can log some issues on the runtime and/or compiler.

@AndyAyersMS
Copy link
Member

Possibly relevant: #11870.

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 12, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 11, 2021
@tannergooding tannergooding removed the untriaged New issue has not been triaged by the area owner label Jun 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-arm64 area-System.Numerics os-linux Linux OS (any supported distro) tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants