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

Optimize SocketMixin#write to avoid string copying #964

Merged
merged 1 commit into from Nov 20, 2020

Conversation

casperisfine
Copy link

Ref: #962 (comment)

The key thing here is that byteslice is smart enough to create shared strings:

>> puts ObjectSpace.dump(PAYLOAD.byteslice(4096..-1).byteslice(4096..-1).byteslice(4096..-1))
{"address":"0x7fd1c93e0178", "type":"STRING", "class":"0x7fd1c90b7578", "shared":true, "encoding":"UTF-8", "references":["0x7fd1c93a3778"], "memsize":40, "flags":{"wb_protected":true}}

In such case only a RString is instantiated, the content isn't copied, so it's way faster. This was benched as 75x faster, however it's a synthetic benchmark without IOs.

cc @SamSaffron maybe you'd like to take a look?

@SamSaffron
Copy link
Contributor

This is a fantastic change, I very much support it!

It reduces a bunch of code duplication and makes stuff much faster for large payloads which is awesome.

When I looked at this yesterday it "seemed" like stuff was structured in a way that we allowed people to override the _write_to_socket method, I think this is probably a bit too much flexibility especially it kind of follows from the naming that this is a private thing, but it may break some people so it is something to watch for.

That said, I 100% support this change and think we should go ahead with it!!!

@byroot byroot merged commit f7d354f into redis:master Nov 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants