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

Potential performance regression in Array.Sort for arrays of reference types #59149

Closed
Tracked by #64603
adamsitnik opened this issue Sep 15, 2021 · 8 comments
Closed
Tracked by #64603
Assignees
Labels
Milestone

Comments

@adamsitnik
Copy link
Member

git clone https://github.com/dotnet/performance.git
py .\scripts\benchmarks_ci.py -f net5.0 net6.0 --filter "System.Collections.Sort<IntClass>.Array" --bdn-arguments "--miniterationcount 100 --maxiterationcount 101 --outliers dontremove --memoryrandomization true"

Both Windows and Unix are affected:

https://pvscmdupload.blob.core.windows.net/reports/allTestHistory%2frefs%2fheads%2fmain_x64_Windows%2010.0.18362%2fSystem.Collections.Sort(IntClass).Array(Size%3a%20512).html

image

https://pvscmdupload.blob.core.windows.net/reports/allTestHistory%2frefs%2fheads%2fmain_x64_ubuntu%2018.04%2fSystem.Collections.Sort(IntClass).Array(Size%3a%20512).html

image

@ghost
Copy link

ghost commented Sep 15, 2021

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

Issue Details
git clone https://github.com/dotnet/performance.git
py .\scripts\benchmarks_ci.py -f net5.0 net6.0 --filter "System.Collections.Sort<IntClass>.Array" --bdn-arguments "--miniterationcount 100 --maxiterationcount 101 --outliers dontremove --memoryrandomization true"

Both Windows and Unix are affected:

https://pvscmdupload.blob.core.windows.net/reports/allTestHistory%2frefs%2fheads%2fmain_x64_Windows%2010.0.18362%2fSystem.Collections.Sort(IntClass).Array(Size%3a%20512).html

image

https://pvscmdupload.blob.core.windows.net/reports/allTestHistory%2frefs%2fheads%2fmain_x64_ubuntu%2018.04%2fSystem.Collections.Sort(IntClass).Array(Size%3a%20512).html

image

Author: adamsitnik
Assignees: -
Labels:

area-System.Runtime, tenet-performance

Milestone: -

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Sep 15, 2021
@adamsitnik
Copy link
Member Author

There are more array sorting-related benchmarks that have regressed across multiple configs, so the better config would be:

--filter 'System.Collections.Sort*Array*'

@danmoseley
Copy link
Member

Is it possible to isolate a commit range? Seems like a sharp change. Might help see whether PGO is a suspect or not.

@jeffhandley jeffhandley 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 Sep 30, 2021
@jeffhandley jeffhandley added this to the 6.0.0 milestone Sep 30, 2021
@jeffhandley
Copy link
Member

@DrewScoggins Could you help narrow down the commit range for this regression?

@DrewScoggins
Copy link
Member

7b7954a...0df29f7

@DrewScoggins
Copy link
Member

There is a PGO update in there, as well as an improvement for division by unsigned constants. Those are the only two things that look like they could be making a difference.

@jeffhandley
Copy link
Member

@AndyAyersMS Do you think PGO could be the cause of this based on Drew's findings. (Thanks, @DrewScoggins!)

Since this was all the way back in May, I'm going to go ahead and move this to 7.0.0, as I can't imagine we'd change anything at this point in 6.0 for this regression.

@dakersnar
Copy link
Contributor

dakersnar commented Aug 8, 2022

@ghost ghost locked as resolved and limited conversation to collaborators Sep 8, 2022
@tannergooding tannergooding removed the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Jun 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants