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

Pre-compute String size after #chomp() if possible #15153

Merged

Conversation

HertzDevil
Copy link
Contributor

If a String ends with one of the newlines and already has its number of characters known, the returned String must have 1 or 2 fewer characters than self. This PR avoids the need to re-compute @length in the new string again if possible.

This technique is also applicable to many more substring extraction methods in String.

spec/std/string_spec.cr Outdated Show resolved Hide resolved
spec/std/string_spec.cr Outdated Show resolved Hide resolved
@straight-shoota straight-shoota added this to the 1.15.0 milestone Nov 5, 2024
@straight-shoota straight-shoota merged commit 84a293d into crystal-lang:master Nov 6, 2024
69 checks passed
@HertzDevil HertzDevil deleted the perf/string-chomp-length branch November 7, 2024 08:33
CTC97 pushed a commit to CTC97/crystal that referenced this pull request Nov 9, 2024
…15153)

If a `String` ends with one of the newlines and already has its number of characters known, the returned `String` must have 1 or 2 fewer characters than `self`. This PR avoids the need to re-compute `@length` in the new string again if possible.

This technique is also applicable to many more substring extraction methods in `String`.
straight-shoota pushed a commit that referenced this pull request Nov 10, 2024
This is similar to #15153.

* Uses `Char::Reader` to read backwards from the end of the string. This does not require decoding the whole string, unlike calculating `size - 1`.
* If the current string does not end with an ASCII character and its size is unknown, `#single_byte_optimizable?` traverses the entire string. This is no longer unnecessary because the above handles everything already.
* If the current string's size is known, the new string's size can be derived from it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants