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

Improve efficiency of readBulk buffer reading #3

Merged
merged 1 commit into from
Jun 6, 2013

Conversation

jpfuentes2
Copy link
Contributor

The previous implementation was reading 1 byte at a time and then
copying it to a target buffer. Now it reads as many bytes as
possible into the target buffer up to the byte length.

Also changed num_bytes to numBytes as it was probably a Ruby typo

The previous implementation was reading 1 byte at a time and then
copying it to a target buffer. Now it reads as many bytes as
possible into the target buffer up to the byte length.

Also changed num_bytes to numBytes as it was probably a Ruby typo
@inkel
Copy link
Owner

inkel commented Jun 6, 2013

It's much faster indeed!

~/dev/go/src/github.com/inkel/gedis (master:v0.0.5) >> go test -bench=readBulk .
PASS
Benchmark_readBulk       1000000              1109 ns/op
ok      github.com/inkel/gedis  2.518s

~/dev/go/src/github.com/inkel/gedis (master) >> git co jpfuentes2/improve_readBulk_buffering

~/dev/go/src/github.com/inkel/gedis ((21d5f6f...)) >> go test -bench=readBulk .
PASS
Benchmark_readBulk       5000000               713 ns/op
ok      github.com/inkel/gedis  11.932s

That's a 26% improvement!

Now I recall the reason why I did it like this was because I wanted to check for EOF, but Read already does that, so awesome work!

Funny thing: I did implement it like this in the server/reader.go version of readBulk.

Thanks @jpfuentes2!

inkel added a commit that referenced this pull request Jun 6, 2013
Improve efficiency of readBulk buffer reading
@inkel inkel merged commit 9bdf6b7 into inkel:master Jun 6, 2013
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.

2 participants