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

Replace boundary on StringByteIndexPrimitiveNode with faster specializations for boundary checks and single-byte-optimizable strings. #2380

Merged

Conversation

nirvdrum
Copy link
Collaborator

With this set of changes, I'm seeing double the performance for String.gsub(String, String) when the pattern fits inside the string (for single byte optimizable strings) and 4x - 5x when the pattern is too large. That last case might seem strange, but if gsub is being used to sanitize some text that may vary in size, it's quite possible for the sanitize pattern to be larger than the body it's applied against.

It's a bit hard to nail down an absolute speed improvement due to the gsub process varying both on the receiver and the pattern sizes. For the out-of-bounds cases, we can avoid fetching either rope's byte array. The rest of the improvements are due to removing the large boundary.

If you don't like the ToRopeNode, I can take another pass at that. I'm not entirely happy that it overlaps with RubyStringLibrary, but I did want to @CreateCast and just work with the ropes in the various specializations. Likewise, I can adjust the branch profiles as needed. They may not be needed at all.

Copy link
Member

@eregon eregon left a comment

Choose a reason for hiding this comment

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

Looks good but needs some tweaks.
ToRopeNode seems convenient.

src/main/java/org/truffleruby/core/string/StringNodes.java Outdated Show resolved Hide resolved
src/main/java/org/truffleruby/core/string/StringNodes.java Outdated Show resolved Hide resolved
src/main/java/org/truffleruby/core/cast/ToRopeNode.java Outdated Show resolved Hide resolved
@nirvdrum nirvdrum force-pushed the faster-string_byte_index-primitive branch from 56e55ba to b1520bc Compare June 18, 2021 14:11
@nirvdrum nirvdrum force-pushed the faster-string_byte_index-primitive branch from b1520bc to 83ca5e1 Compare June 18, 2021 14:44
Copy link
Member

@eregon eregon left a comment

Choose a reason for hiding this comment

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

Thanks for the PR and the quick turnaround!

@nirvdrum nirvdrum force-pushed the faster-string_byte_index-primitive branch from 83ca5e1 to fc34195 Compare June 18, 2021 15:02
…lizations for boundary checks and single-byte-optimizable strings.
@nirvdrum nirvdrum force-pushed the faster-string_byte_index-primitive branch from fc34195 to 1fcbd1d Compare June 18, 2021 15:04
@eregon eregon added the in-ci The PR is being tested in CI. Do not push new commits. label Jun 18, 2021
@eregon eregon added this to the 21.2.0 milestone Jun 18, 2021
@eregon eregon modified the milestones: 21.2.0, 21.3.0 Jun 20, 2021
graalvmbot pushed a commit that referenced this pull request Jun 21, 2021
…er specializations for boundary checks and single-byte-optimizable strings (#2380)

PullRequest: truffleruby/2742
@graalvmbot graalvmbot merged commit 1fcbd1d into oracle:master Jun 21, 2021
@tomstuart tomstuart deleted the faster-string_byte_index-primitive branch June 22, 2021 08:09
LillianZ pushed a commit to Shopify/truffleruby that referenced this pull request Jun 22, 2021
with faster specializations for boundary checks and single-byte-optimizable strings.
(Mirroring StringByteIndexPrimitiveNode after improvements in oracle#2380).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-ci The PR is being tested in CI. Do not push new commits. oca-signed shopify
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants