Micro optimize encoding checks #117
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eregon
reviewed
Nov 27, 2024
ext/strscan/strscan.c
Outdated
| StringValue(pattern); | ||
| rb_encoding *enc = rb_enc_check(p->str, pattern); | ||
| if (S_RESTLEN(p) < RSTRING_LEN(pattern)) { | ||
| if (!(strscan_ascii_compat_fastpath(p->str) && strscan_ascii_compat_fastpath(pattern))) { |
There was a problem hiding this comment.
Is this equivalent though?
> Encoding.compatible? "é".b, "é"
=> nil
So these are not compatible but they have both one of the 3 simple encodings
There was a problem hiding this comment.
Yeah, you're right. rb_enc_check end up in enc_compatible_latter, and if it returns 0 it raises: https://github.com/ruby/ruby/blob/43b059b6a3b5c49b7d883c49dd1200580c1f92be/encoding.c#L1068-L1128
So I'm missing a check for both encindex to be equal.
There was a problem hiding this comment.
I replaced it to just a fastpath if encindex are the same, which I think would most often be the case.
8eb2198 to
e92e6d1
Compare
Profiling shows a lot of time spent in various encoding check functions.
I'm working on optimizing them on the Ruby side, but if we assume most
strings are one of the simple 3 encodings, we can skip a lot of overhead.
```ruby
require 'strscan'
require 'benchmark/ips'
source = 10_000.times.map { rand(9999999).to_s }.join(",").force_encoding(Encoding::UTF_8).freeze
def scan_to_i(source)
scanner = StringScanner.new(source)
while number = scanner.scan(/\d+/)
number.to_i
scanner.skip(",")
end
end
def scan_integer(source)
scanner = StringScanner.new(source)
while scanner.scan_integer
scanner.skip(",")
end
end
Benchmark.ips do |x|
x.report("scan.to_i") { scan_to_i(source) }
x.report("scan_integer") { scan_integer(source) }
x.compare!
end
```
Before:
```
ruby 3.3.4 (2024-07-09 revision be1089c8ec) +YJIT [arm64-darwin23]
Warming up --------------------------------------
scan.to_i 93.000 i/100ms
scan_integer 232.000 i/100ms
Calculating -------------------------------------
scan.to_i 933.191 (± 0.2%) i/s (1.07 ms/i) - 4.743k in 5.082597s
scan_integer 2.326k (± 0.8%) i/s (429.99 μs/i) - 11.832k in 5.087974s
Comparison:
scan_integer: 2325.6 i/s
scan.to_i: 933.2 i/s - 2.49x slower
```
After:
```
ruby 3.3.4 (2024-07-09 revision be1089c8ec) +YJIT [arm64-darwin23]
Warming up --------------------------------------
scan.to_i 96.000 i/100ms
scan_integer 274.000 i/100ms
Calculating -------------------------------------
scan.to_i 969.489 (± 0.2%) i/s (1.03 ms/i) - 4.896k in 5.050114s
scan_integer 2.756k (± 0.1%) i/s (362.88 μs/i) - 13.974k in 5.070837s
Comparison:
scan_integer: 2755.8 i/s
scan.to_i: 969.5 i/s - 2.84x slower
```
e92e6d1 to
f3115ce
Compare
|
Thanks. |
hsbt
pushed a commit
to hsbt/ruby
that referenced
this pull request
Dec 2, 2024
(ruby/strscan#117) Profiling shows a lot of time spent in various encoding check functions. I'm working on optimizing them on the Ruby side, but if we assume most strings are one of the simple 3 encodings, we can skip a lot of overhead. ```ruby require 'strscan' require 'benchmark/ips' source = 10_000.times.map { rand(9999999).to_s }.join(",").force_encoding(Encoding::UTF_8).freeze def scan_to_i(source) scanner = StringScanner.new(source) while number = scanner.scan(/\d+/) number.to_i scanner.skip(",") end end def scan_integer(source) scanner = StringScanner.new(source) while scanner.scan_integer scanner.skip(",") end end Benchmark.ips do |x| x.report("scan.to_i") { scan_to_i(source) } x.report("scan_integer") { scan_integer(source) } x.compare! end ``` Before: ``` ruby 3.3.4 (2024-07-09 revision ruby/strscan@be1089c8ec) +YJIT [arm64-darwin23] Warming up -------------------------------------- scan.to_i 93.000 i/100ms scan_integer 232.000 i/100ms Calculating ------------------------------------- scan.to_i 933.191 (± 0.2%) i/s (1.07 ms/i) - 4.743k in 5.082597s scan_integer 2.326k (± 0.8%) i/s (429.99 μs/i) - 11.832k in 5.087974s Comparison: scan_integer: 2325.6 i/s scan.to_i: 933.2 i/s - 2.49x slower ``` After: ``` ruby 3.3.4 (2024-07-09 revision ruby/strscan@be1089c8ec) +YJIT [arm64-darwin23] Warming up -------------------------------------- scan.to_i 96.000 i/100ms scan_integer 274.000 i/100ms Calculating ------------------------------------- scan.to_i 969.489 (± 0.2%) i/s (1.03 ms/i) - 4.896k in 5.050114s scan_integer 2.756k (± 0.1%) i/s (362.88 μs/i) - 13.974k in 5.070837s Comparison: scan_integer: 2755.8 i/s scan.to_i: 969.5 i/s - 2.84x slower ``` ruby/strscan@c02b1ce684
hsbt
pushed a commit
to ruby/ruby
that referenced
this pull request
Dec 2, 2024
(ruby/strscan#117) Profiling shows a lot of time spent in various encoding check functions. I'm working on optimizing them on the Ruby side, but if we assume most strings are one of the simple 3 encodings, we can skip a lot of overhead. ```ruby require 'strscan' require 'benchmark/ips' source = 10_000.times.map { rand(9999999).to_s }.join(",").force_encoding(Encoding::UTF_8).freeze def scan_to_i(source) scanner = StringScanner.new(source) while number = scanner.scan(/\d+/) number.to_i scanner.skip(",") end end def scan_integer(source) scanner = StringScanner.new(source) while scanner.scan_integer scanner.skip(",") end end Benchmark.ips do |x| x.report("scan.to_i") { scan_to_i(source) } x.report("scan_integer") { scan_integer(source) } x.compare! end ``` Before: ``` ruby 3.3.4 (2024-07-09 revision ruby/strscan@be1089c8ec) +YJIT [arm64-darwin23] Warming up -------------------------------------- scan.to_i 93.000 i/100ms scan_integer 232.000 i/100ms Calculating ------------------------------------- scan.to_i 933.191 (± 0.2%) i/s (1.07 ms/i) - 4.743k in 5.082597s scan_integer 2.326k (± 0.8%) i/s (429.99 μs/i) - 11.832k in 5.087974s Comparison: scan_integer: 2325.6 i/s scan.to_i: 933.2 i/s - 2.49x slower ``` After: ``` ruby 3.3.4 (2024-07-09 revision ruby/strscan@be1089c8ec) +YJIT [arm64-darwin23] Warming up -------------------------------------- scan.to_i 96.000 i/100ms scan_integer 274.000 i/100ms Calculating ------------------------------------- scan.to_i 969.489 (± 0.2%) i/s (1.03 ms/i) - 4.896k in 5.050114s scan_integer 2.756k (± 0.1%) i/s (362.88 μs/i) - 13.974k in 5.070837s Comparison: scan_integer: 2755.8 i/s scan.to_i: 969.5 i/s - 2.84x slower ``` ruby/strscan@c02b1ce684
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Profiling shows a lot of time spent in various encoding check functions. I'm working on optimizing them on the Ruby side, but if we assume most strings are one of the simple 3 encodings, we can skip a lot of overhead.
Before:
After: