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.

Sign up for GitHub

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

Jump to bottom

Only use length-based dispatch optimization under /optimize+ #70680

Merged
merged 1 commit into from
Nov 3, 2023
Merged

Only use length-based dispatch optimization under /optimize+ #70680

merged 1 commit into from
Nov 3, 2023

Conversation

jcouv
Copy link
Member

@jcouv jcouv commented Nov 2, 2023

Resolves #69674

@jcouv jcouv self-assigned this Nov 2, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Nov 2, 2023
@jcouv jcouv marked this pull request as ready for review November 3, 2023 06:55
@jcouv jcouv requested a review from a team as a code owner November 3, 2023 06:55
@AlekseyTs
Copy link
Contributor

Is there a way to improve performance of the compiler instead? It might be interesting to understand why performance is bad for the scenario.

AlekseyTs
AlekseyTs approved these changes Nov 3, 2023
View reviewed changes
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 1)

@jcouv
Copy link
Member Author

jcouv commented Nov 3, 2023

Is there a way to improve performance of the compiler instead? It might be interesting to understand why performance is bad for the scenario.

The scenario that was reported involved a fairly large switch (8000 cases).
I did another scan of the algorithm involved (in LengthBasedStringSwitchData) and I didn't see a way to reduce the complexity.

@jcouv jcouv merged commit c446628 into dotnet:main Nov 3, 2023
25 checks passed
@jcouv jcouv deleted the length-optimize branch November 3, 2023 22:15
@ghost ghost added this to the Next milestone Nov 3, 2023
@allisonchou allisonchou mentioned this pull request Nov 9, 2023
Closed
@RikkiGibson RikkiGibson modified the milestones: Next, 17.9 P2 Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jjonescz jjonescz jjonescz approved these changes

@AlekseyTs AlekseyTs AlekseyTs approved these changes

Assignees

@jcouv jcouv

Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Milestone
17.9 P2
Development

Successfully merging this pull request may close these issues.

Consider move length-based string dispatch behind /optimize
4 participants
@jcouv @AlekseyTs @jjonescz @RikkiGibson