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] Windows/x64: 4 Regressions on 4/11/2024 11:29:08 PM #101134

Closed
performanceautofiler bot opened this issue Apr 16, 2024 · 4 comments
Closed

[Perf] Windows/x64: 4 Regressions on 4/11/2024 11:29:08 PM #101134

performanceautofiler bot opened this issue Apr 16, 2024 · 4 comments
Assignees
Labels
arch-x64 area-System.Runtime needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration os-windows runtime-coreclr specific to the CoreCLR runtime tenet-performance Performance related issue

Comments

@performanceautofiler
Copy link

performanceautofiler bot commented Apr 16, 2024

Run Information

Name Value
Architecture x64
OS Windows 10.0.22621
Queue TigerWindows
Baseline a8158c170b694f8c1dbae114c63c346b38244901
Compare 324bd561b9e9d01ded8d2cb086cf55d23fc37eab
Diff Diff
Configs CompilationMode:tiered, RunKind:micro

Regressions in System.Collections.AddGivenSize<String>

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio
2.85 μs 3.16 μs 1.11 0.11 False
2.03 μs 2.18 μs 1.08 0.12 False

graph
graph
Test Report

Repro

General Docs link: https://github.com/dotnet/performance/blob/main/docs/benchmarking-workflow-dotnet-runtime.md

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

System.Collections.AddGivenSize<String>.ICollection(Size: 512)

ETL Files

Histogram

JIT Disasms

System.Collections.AddGivenSize<String>.List(Size: 512)

ETL Files

Histogram

JIT Disasms

Docs

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


Run Information

Name Value
Architecture x64
OS Windows 10.0.22621
Queue TigerWindows
Baseline a8158c170b694f8c1dbae114c63c346b38244901
Compare 324bd561b9e9d01ded8d2cb086cf55d23fc37eab
Diff Diff
Configs CompilationMode:tiered, RunKind:micro

Regressions in System.Reflection.Invoke

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio
217.96 ns 240.77 ns 1.10 0.06 False
303.04 ns 350.44 ns 1.16 0.05 True

graph
graph
Test Report

Repro

General Docs link: https://github.com/dotnet/performance/blob/main/docs/benchmarking-workflow-dotnet-runtime.md

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net8.0 --filter 'System.Reflection.Invoke*'

System.Reflection.Invoke.StaticMethod4_ByRefParams_int_string_struct_class

ETL Files

Histogram

JIT Disasms

System.Reflection.Invoke.StaticMethod5_ByRefParams_int_string_struct_class_bool

ETL Files

Histogram

JIT Disasms

Docs

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

@performanceautofiler performanceautofiler bot added arch-x64 os-windows runtime-coreclr specific to the CoreCLR runtime untriaged New issue has not been triaged by the area owner labels Apr 16, 2024
@LoopedBard3 LoopedBard3 removed the untriaged New issue has not been triaged by the area owner label Apr 16, 2024
@jkotas jkotas transferred this issue from dotnet/perf-autofiling-issues Apr 16, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Apr 16, 2024
@jkotas jkotas added area-System.Runtime tenet-performance Performance related issue and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Apr 16, 2024
Copy link
Contributor

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

@jkoritzinsky jkoritzinsky reopened this Apr 16, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Apr 16, 2024
@jkotas
Copy link
Member

jkotas commented Apr 16, 2024

We are leaving the performance on the table in JIT_Box helper. It should be possible to structure this helper to have fast and slow paths, similar to how other allocation helpers are structured.

JIT_Box is inlined by the JIT for situations that matter, so the perf did not matter much. We are calling it directly now that is changing the equation.

@jkoritzinsky
Copy link
Member

I'm still investigating, but I think one of the culprits (based on the related worse Mono regression in the exact same two benchmarks) is the introduction of the type.IsNullableOfT check in AllocateValueType from the MethodBaseInvoker.TryByRefFastPath path (for which that will never be true).

I'm looking now to see if inlining that branch into the callers where it fixes the regression.

@tannergooding tannergooding added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed untriaged New issue has not been triaged by the area owner labels Jun 24, 2024
@jkoritzinsky
Copy link
Member

Fixed by #101137

image

@github-actions github-actions bot locked and limited conversation to collaborators Aug 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-x64 area-System.Runtime needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration os-windows runtime-coreclr specific to the CoreCLR runtime tenet-performance Performance related issue
Projects
None yet
Development

No branches or pull requests

4 participants