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

Reduce work in ConversionsBase.AddUserDefinedConversionsToExplicitCandidateSet #76835

Conversation

ToddGrun
Copy link
Contributor

The ArrayBuilder usage in this method is showing up slightly in speedometer CPU profiles (0.1%) but more heavily in local profiles I've taken using a much larger version of the file that Kayle provided than what the speedometer tests use (2.6%)

This method is called quite often and the (isExplicit && isChecked) check seems to not hit very often (specifically, never in my testing of opening/simple editing of the file). The usage of the pool and populating the array builder aren't necessary unless that case is hit, so I've reshuffled the code around a bit to avoid that.

…didateSet

The ArrayBuilder usage in this method is showing up slightly in speedometer CPU profiles (0.1%) but more heavily in local profiles I've taken using a much larger version of the file than what the speedometer tests use (2.6%)

This method is called quite often and the (isExplicit && isChecked) check seems to not hit very often (specifically, never in my testing of opening/simple editing of the file). The usage of the pool and populating the array builder aren't necessary unless that case is hit, so I've reshuffled the code around a bit to avoid that.
@ToddGrun ToddGrun requested a review from a team as a code owner January 21, 2025 18:39
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Jan 21, 2025
@ToddGrun
Copy link
Contributor Author

@dotnet/roslyn-compiler for 2nd review

Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (commit 2)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants