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

Regressions in System.Tests.Perf_Uri #98668

Open
performanceautofiler bot opened this issue Feb 16, 2024 · 4 comments
Open

Regressions in System.Tests.Perf_Uri #98668

performanceautofiler bot opened this issue Feb 16, 2024 · 4 comments
Labels
area-System.Net runtime-coreclr specific to the CoreCLR runtime
Milestone

Comments

@performanceautofiler
Copy link

performanceautofiler bot commented Feb 16, 2024

Run Information

Name Value
Architecture arm64
OS ubuntu 22.04
Queue AmpereUbuntu
Baseline ec1a6b9e3b74bd352df5d3856c9088da8f902783
Compare 49fe3b06cdfce737ea1c8964e01f2dd2a4e77d44
Diff Diff
Configs CompilationMode:tiered, RunKind:micro

Regressions in System.Tests.Perf_Uri

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio
15.57 μs 17.81 μs 1.14 0.01 True

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
python3 .\performance\scripts\benchmarks_ci.py -f net8.0 --filter 'System.Tests.Perf_Uri*'

Payloads

Baseline
Compare

System.Tests.Perf_Uri.EscapeDataString(input: "üüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüü")

ETL Files

Histogram

JIT Disasms

Docs

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

@performanceautofiler performanceautofiler bot added arch-arm64 os-linux Linux OS (any supported distro) runtime-coreclr specific to the CoreCLR runtime untriaged New issue has not been triaged by the area owner labels Feb 16, 2024
@cincuranet cincuranet changed the title [Perf] Linux/arm64: 1 Regression on 2/13/2024 8:56:49 PM Regressions in System.Tests.Perf_Uri Feb 19, 2024
@cincuranet cincuranet removed the untriaged New issue has not been triaged by the area owner label Feb 19, 2024
@cincuranet cincuranet transferred this issue from dotnet/perf-autofiling-issues Feb 19, 2024
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 19, 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 Feb 19, 2024
@cincuranet
Copy link
Contributor

Diff. Likely suspect #98074. cc @MihaZupan

@MihaZupan MihaZupan added area-System.Net and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Feb 19, 2024
@ghost
Copy link

ghost commented Feb 19, 2024

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

Issue Details

Run Information

Name Value
Architecture arm64
OS ubuntu 22.04
Queue AmpereUbuntu
Baseline ec1a6b9e3b74bd352df5d3856c9088da8f902783
Compare 49fe3b06cdfce737ea1c8964e01f2dd2a4e77d44
Diff Diff
Configs CompilationMode:tiered, RunKind:micro

Regressions in System.Tests.Perf_Uri

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio
15.57 μs 17.81 μs 1.14 0.01 True

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
python3 .\performance\scripts\benchmarks_ci.py -f net8.0 --filter 'System.Tests.Perf_Uri*'

Payloads

Baseline
Compare

System.Tests.Perf_Uri.EscapeDataString(input: "üüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüü")

ETL Files

Histogram

JIT Disasms

Docs

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

Author: performanceautofiler[bot]
Assignees: -
Labels:

arch-arm64, area-System.Net, os-linux, untriaged, runtime-coreclr

Milestone: -

@MihaZupan MihaZupan removed arch-arm64 os-linux Linux OS (any supported distro) labels Feb 20, 2024
@MihaZupan MihaZupan added this to the 9.0.0 milestone Feb 20, 2024
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Feb 20, 2024
@MihaZupan MihaZupan removed the untriaged New issue has not been triaged by the area owner label Feb 20, 2024
@MihaZupan
Copy link
Member

MihaZupan commented Feb 20, 2024

I can repro this locally (Win x64). Triaging to 9.0.

Looks like the EscapeStringToBuilder helper is getting inlined into EscapeString, which now hurts some types of inputs more (e.g. long non-ASCII strings, like the benchmark in question here).

Slapping NoInlining on EscapeStringToBuilder does get us back to pre-#98074 numbers for the benchmark cited above.

The current logic tries to reuse more code, but we might want to consider tweaking it a bit more to reduce the per-call overhead for simple inputs too.
E.g. (where pr refers to #98074)

Method Toolchain input Mean Error Ratio Code Size
EscapeDataString before-pr aaaaaaaaaa 3.443 ns 0.0164 ns 1.00 905 B
EscapeDataString pr aaaaaaaaaa 4.702 ns 0.0193 ns 1.36 1,115 B
EscapeDataString noInline aaaaaaaaaa 4.777 ns 0.0363 ns 1.39 1,115 B

@DrewScoggins
Copy link
Member

DrewScoggins commented Feb 20, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Net runtime-coreclr specific to the CoreCLR runtime
Projects
None yet
Development

No branches or pull requests

3 participants