Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Bad performance for "hgetall" #112

Closed
mkotsbak opened this issue Dec 21, 2015 · 14 comments
Closed

Bad performance for "hgetall" #112

mkotsbak opened this issue Dec 21, 2015 · 14 comments
Milestone

Comments

@mkotsbak
Copy link

Trying to get a hashmap with 10 000 simple elements like this:

(tagkey8193,tagvalue8193)
(tagkey8748,tagvalue8748)
(tagkey6678,tagvalue6678)
(tagkey9674,tagvalue9674)

is extremely slow. It takes around 1 seconds to get all those values out, with a simple mapping. I suspect the algorithm to parse the results is too slow, and maybe is performing exponentially slow with increasing values.

@etaty
Copy link
Owner

etaty commented Dec 21, 2015

This is were thing are happening:

case class Hgetall[K, R](key: K)(implicit redisKey: ByteStringSerializer[K], deserializerR: ByteStringDeserializer[R]) extends RedisCommandMultiBulk[Map[String, R]] {

@etaty
Copy link
Owner

etaty commented Dec 21, 2015

Oh I see, it is iterating twice on the reply

val seq = r.map(_.toByteString)

Also I could give a sizeHint to the map builder

@etaty
Copy link
Owner

etaty commented Dec 21, 2015

def bulks(bs: ByteString, i: Int, acc: mutable.Buffer[RedisReply]): Option[(MultiBulk, ByteString)] = {

Here we could also use a Seq builder and give a sizeHint

@mkotsbak
Copy link
Author

Japp, just move the map inside the builder?

@mkotsbak
Copy link
Author

Moved back to https://github.com/debasishg/scala-redis with connection pooling. The same test (with 20k values) performs around 15 req/s.

etaty added a commit that referenced this issue Dec 27, 2015
etaty added a commit that referenced this issue Dec 27, 2015
etaty added a commit that referenced this issue Dec 27, 2015
@etaty
Copy link
Owner

etaty commented Dec 27, 2015

It should be resolved now

resolvers += "Sonatype OSS Snapshots" at "https://oss.sonatype.org/content/repositories/snapshots"
libraryDependencies += "com.github.etaty" %% "rediscala" % "1.6.0-SNAPSHOT"

Tell me if you don't see similar perf ( I can do 15 hgetall/s with 10k elements with one single connection)

@etaty etaty added this to the 1.6.0 milestone Dec 27, 2015
@etaty etaty closed this as completed Dec 27, 2015
@mkotsbak
Copy link
Author

Hmm, much better after the modifications:
Requests per second: 3.72 #/sec

but it still seems far behind scala-redis with the same data in the DB:

Requests per second: 14.45 #/sec

@etaty
Copy link
Owner

etaty commented Dec 28, 2015

Thanks,
I will try to do a benchmark diff with scala-redis
Can you post the code you are running in a gist?

@mkotsbak
Copy link
Author

I'm sorry, the code cannot be disclosed, but it should be easy to reproduce using scala-redis future with connection pool example. Is this project using multiple connections, or just one?

@etaty
Copy link
Owner

etaty commented Dec 28, 2015

Hum I see, the main difference is the number of connection to the redis server.
A RedisClient is just one connection while scala-redis use a pool in your bench.
But yeah I will publish some benchmarks.

@etaty
Copy link
Owner

etaty commented Dec 30, 2015

Okay,
I did rewrite the redis reply decoder, so it can keep a much closer state of what has been decoded. Something like progressive decoding (or async decoding).

Before I was decoding a full redis response, and if I did not have the full response, I was starting again when receiving the next packet. It works with small response. But the response for HGETALL with 10000 hashes is around 1MB! So the decoding is deadly slow.

Conclusion is that now I am even faster than scala-redis. I will clean and post the result before the end of the year.

@mkotsbak
Copy link
Author

Great! I can't see any code here yet?

@etaty
Copy link
Owner

etaty commented Dec 31, 2015

I create a PR #119

@etaty
Copy link
Owner

etaty commented Dec 31, 2015

snapshot available

resolvers += "Sonatype OSS Snapshots" at "https://oss.sonatype.org/content/repositories/snapshots"
libraryDependencies += "com.github.etaty" %% "rediscala" % "1.6.0-SNAPSHOT"

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants