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

Performance regression on Fortunes Windows #85930

Closed
sebastienros opened this issue May 8, 2023 · 4 comments
Closed

Performance regression on Fortunes Windows #85930

sebastienros opened this issue May 8, 2023 · 4 comments
Labels
area-VM-coreclr tenet-performance Performance related issue

Comments

@sebastienros
Copy link
Member

sebastienros commented May 8, 2023

There is a throughput regression for the Fortunes benchmarks on Windows (only).

image

The minimum set of commits is b4e14b4...3232ad3

NB: there is a perf improvement right before that I haven't ruled out as unrelated. This regression could be a fix to a previous issue.

Before

crank --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/scenarios/database.benchmarks.yml --scenario fortunes --profile aspnet-citrine-win --application.framework net8.0 --application.aspNetCoreVersion 8.0.0-preview.5.23253.15 --application.runtimeVersion 8.0.0-preview.5.23253.15 --application.sdkVersion 8.0.100-preview.5.23254.3

After

crank --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/scenarios/database.benchmarks.yml --scenario fortunes --profile aspnet-citrine-win --application.framework net8.0 --application.aspNetCoreVersion 8.0.0-preview.5.23255.6 --application.runtimeVersion 8.0.0-preview.5.23254.1 --application.sdkVersion 8.0.100-preview.5.23254.20
@sebastienros sebastienros added the tenet-performance Performance related issue label May 8, 2023
@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 May 8, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label May 8, 2023
@sebastienros
Copy link
Member Author

The improvement that we can see a few runs before is due to one of these changes:

b02d7a1...08ba40d

Before:

crank --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/scenarios/database.benchmarks.yml --scenario fortunes --profile aspnet-citrine-win --application.framework net8.0 --application.aspNetCoreVersion 8.0.0-preview.5.23251.10 --application.runtimeVersion 8.0.0-preview.5.23251.7 --application.sdkVersion 8.0.100-preview.5.23228.7

After:

crank --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/scenarios/database.benchmarks.yml --scenario fortunes --profile aspnet-citrine-win --application.framework net8.0 --application.aspNetCoreVersion 8.0.0-preview.5.23251.10 --application.runtimeVersion 8.0.0-preview.5.23252.1 --application.sdkVersion 8.0.100-preview.5.23228.7

@jeffschwMSFT
Copy link
Member

Looking at the ranges, it is not obvious. Adding @tannergooding as those commits look the most interesting.

@tannergooding
Copy link
Member

There was a bug introduced in #85536 (the improvement range) that was fixed in #85714 (the regression range). It's likely this pair of changes

@sebastienros
Copy link
Member Author

Thanks for confirming.

@ghost ghost removed the untriaged New issue has not been triaged by the area owner label May 9, 2023
@am11 am11 removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 16, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-VM-coreclr tenet-performance Performance related issue
Projects
None yet
Development

No branches or pull requests

4 participants