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
FEATURE: support custom codecs for transport #250
Conversation
Adds support for MessageBus.transport_codec which can be used to optimise performance of transport - Oj can be used as a serializer vs JSON which is a bit slower - Users can implement custom codecs - OjFast can be used as a serializer for efficient user_id lookups Backwards compatible change, switching codecs to OjFast will invalidate all existing storage. Work in progress
Do not merge yet, work in progress |
Sadly spec suite is not quite passing cause encoded data explodes if \x00 is in the stream, need to figure out why Bench though is spectacularly good, so we are making progress
lib/message_bus/codec/oj_fast.rb
Outdated
found = (0...length).bsearch do |index| | ||
@packed.byteslice(index * 4, 4).unpack1("V") >= id | ||
end | ||
|
||
if found | ||
found && @packed.byteslice(found * 4, 4).unpack1("V") == id | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: you mentioned the problem with 4 bytes, but why is it not possible to use "Q"
to pack 8 bytes?
I made a test and it seems to work:
numbers = [1,999999999999999].pack("Q*")
(0...numbers.length).step(8).map { |index| numbers.byteslice(index, 8).unpack1("Q") }
=> [1, 999999999999999]
Sorry if that doesn't make sense :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure made Q vs V optional in the demo codec in the bench directory.
Sorry for not being able to review the previous issue in a timely manner. I'm fine with custom codecs, as long as the default is still JSON for backwards compatibility. |
- Allow redis backend to handle \u0000 in exceptional cases like the unsupported Marshal codec - Ship only Oj and JSON codecs out of the box - Add more benchmarks to show when packing user ids helps vs hinders - Update readme
See my latest changes, stuff should be reasonably tuned and well documented now. Let me know what you think. Absolutely, on top of that I am only shipping Codecs I consider 100% supported. (Oj and JSON) which are cross compatible. Very interestingly @ohler55 JSON seems to outperform OJ for our typical use case cause we need to use Oj in compat mode which can be slower than JSON |
Co-authored-by: Rafael Garcia <rafbgarcia@gmail.com>
@rafbgarcia do you feel comfortable with where this is at? Should I go ahead and merge. |
@SamSaffron yes I'm fine with this, thanks! |
Adds support for MessageBus.transport_codec which can be used to optimise
performance of transport
Backwards compatible change, switching codecs to OjFast will invalidate all
existing storage.
Work in progress