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

Consider removing IsFastSort optimizations from String #13434

Closed
GrabYourPitchforks opened this issue Sep 18, 2019 · 2 comments · Fixed by dotnet/coreclr#26759
Closed

Consider removing IsFastSort optimizations from String #13434

GrabYourPitchforks opened this issue Sep 18, 2019 · 2 comments · Fixed by dotnet/coreclr#26759
Assignees
@jkotas
Labels
area-System.Runtime

Comments

@GrabYourPitchforks
Copy link
Member

We might not actually need these optimizations in practice, and removing them would allow to simplify the code. See also dotnet/coreclr#24400 and https://github.com/dotnet/coreclr/issues/19672#issuecomment-415974153.
@jkotas jkotas self-assigned this Sep 18, 2019
@GrabYourPitchforks
Copy link
Member Author

Related: dotnet/coreclr#10632

/cc @tarekgh

@adamsitnik
Copy link
Member

adamsitnik commented Sep 18, 2019 •
edited
Loading

We might not actually need these optimizations in practice, and removing them would allow to simplify the code

This particular optimization can be also an anti-pattern. An example:

new string('a', 100_000).StartsWith("i")

the first call to StartsWith iterates through both entire strings, which is expensive (and not needed in this case)

I really like the idea of removing it 👍

jkotas referenced this issue in jkotas/coreclr Sep 20, 2019
@EgorBo @jkotas
Remove IsFastSort optimizations from String
f49f90c 
- Fix bugs in the Span-based globalization fast paths. Note that some of these bug fixes are going to affect performance of the globalization fast paths.
- Use Span-based globalization fast paths for strings
- Avoid static array allocation for HighCharTable

Fixes #26758
jkotas referenced this issue in dotnet/coreclr Sep 23, 2019
@jkotas
Remove IsFastSort optimizations from String (#26759)
5d1f15f 
* Remove IsFastSort optimizations from String
- Fix bugs in the Span-based globalization fast paths. Note that some of these bug fixes are going to affect performance of the globalization fast paths.
- Use Span-based globalization fast paths for strings
- Avoid static array allocation for HighCharTable

Fixes #26758
@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@jkotas jkotas

Labels
area-System.Runtime
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove IsFastSort optimizations from String jkotas/coreclr
3 participants
@GrabYourPitchforks @adamsitnik @jkotas