-
Notifications
You must be signed in to change notification settings - Fork 55
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
Various Ruby driver optimizations #184
Merged
Merged
Conversation
This file contains 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
THe main gain is to have proper method calls rather than dynamic `send`. Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
All RESP3 types start with a single line containing a postive integer that announce the size of the data. e.g. `*42\r\n....`. Before this commit we would read one line, hence allocate a String, then parse it with `Kernel.Integer`. By instead reading bytes one by one and rebuilding the integer we save that string allocation, which has a significant impact. The `gets_integer` method also optimistically assumes the buffer already contains the line, which saves some methods calls in the happy path. Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
redis-client when parsing a string first attempt to encode it as `Encoding.default_external` and if invalid, fallbacks to encode it as binary. By having the buffer encoded with the default external encoding we save having to change the encoding once on every parsed string. Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
This method is a big hotspot for parsing, by not pessimistically checking the size of the buffer we save a few method calls on the green path. Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
casperisfine
force-pushed
the
bench-hiredis-vs-ruby
branch
from
March 22, 2024 11:58
8e606ca
to
134f295
Compare
The TruffleRuby failure seem legit and may be indicative of a TruffleRuby bug:
I'll dig this one down and try to come up with a minimal repro. FYI @eregon |
There was some recent YJIT optimization for getbyte and some other methods we use a lot in the Ruby driver.
casperisfine
force-pushed
the
bench-hiredis-vs-ruby
branch
from
March 22, 2024 13:05
134f295
to
8c1c68b
Compare
Ruby spec for the TruffleRuby discrepancy: ruby/spec#1145 |
21.1:
This branch:
A very slight improvement to Sidekiq's benchmark. |
The behavior of `IO#read_nonblock` differs sligthly. See: ruby/spec#1145
casperisfine
pushed a commit
that referenced
this pull request
Apr 12, 2024
As of #184, the buffer String is no longer BINARY but UTF-8. I missed that the code that search for newlines was using `.index` instead of `.byteindex`, causing the buffer offset to go out of sync.
casperisfine
pushed a commit
that referenced
this pull request
Apr 12, 2024
As of #184, the buffer String is no longer BINARY but UTF-8. I missed that the code that search for newlines was using `.index` instead of `.byteindex`, causing the buffer offset to go out of sync.
casperisfine
pushed a commit
that referenced
this pull request
Apr 12, 2024
As of #184, the buffer String is no longer BINARY but UTF-8. I missed that the code that search for newlines was using `.index` instead of `.byteindex`, causing the buffer offset to go out of sync.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I recently paired with @tenderlove on trying to see if the Ruby vs Hiredis performance difference could be closed.
We came up with a bunch of small changes that really close the gap quite significantly, especially when YJIT is enabled.
The Ruby driver is now either on par with hiredis, or just marginally slower on some cases.
There are a few more optimization we thought of, but they require some changes in Ruby, I'll try to work on these this year. Notably a way to create hashes with a given capacity would help a lot.
Before (YJIT)
ruby:
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [arm64-darwin23]
redis-server:
Redis server v=7.0.12 sha=00000000:0 malloc=libc bits=64 build=a11d0151eabf466c
small string x 100
large string x 100
small list x 100
large list
small hash x 100
large hash
After (YJIT)
ruby:
ruby 3.4.0dev (2024-03-19T14:18:56Z master 5c2937733c) [arm64-darwin23]
redis-server:
Redis server v=7.0.12 sha=00000000:0 malloc=libc bits=64 build=a11d0151eabf466c
small string x 100
large string x 100
small list x 100
large list
small hash x 100
large hash
NB: the later use
ruby-head
to benefit from some YJIT opts.