apa512 / clj-rethinkdb

Eclipse Public License 1.0
204 stars 42 forks source link

net/read-response* bottleneck (Ruby driver faster than Clojure) #107

Closed KeeganMyers closed 8 years ago

KeeganMyers commented 8 years ago

I have been running performance benchmarks between the old application I have been refactoring (rails with Mariadb back-end), and the current Clojure rethink stack. The results for basic crud have been disappointing. This lead me to initially assume that Rethinkdb its self may be slower than Mariadb for basic crud. So I tested this by connecting to rethink in ruby. But the result is substantially faster than in Clojure

Benchmark.ms {1000.times.each {r.db('arcDev').table('resource').get("a60401f0-7124-11e5-81af-b16d74de8809").run()}

559.1058265417814 ms

(time (with-open [conn (r/connect :host "172.17.0.10" :port 28015 :db "arcDev")](dotimes [n 1000] %28-> %28r/db) (r/table "resource") (r/get "c5144730-7124-11e5-81af-b16d74de8809") (r/run conn))))) "Elapsed time: 40018.837416 msecs"

After profiling the Clojure implementation I found that most time spent in this query occurs in read-response*. This seems to be directly related to two factors

1 casting bytes to int seems to be somewhat slower than DataInputStream.readInt or .readLong 2 reading from unbuffered inputStreams is known to be slower than making use of either bufferedInputStream or a buffered reader.

Making use of a buffered stream seems to give marginal improvements. But I think performance might be even better if we replace java.io with nio.2. The only limiting factor is that core.async + nio.2 may not account for back pressure well.

I have forked the repo and will create a pull request if I can find a way to significantly improve performance.

danielcompton commented 8 years ago

There's a few open issues about this. I suspect the long term solution is to use the official Java RethinkDB driver, but any short term improvements would be appreciated.

KeeganMyers commented 8 years ago

I wanted to provide an update on this issue. At the moment I have decreased the query time to a fairly meaningful degree

(time (dotimes [n 1000](-> %28query/db) (query/table "resource") (query/get "a60401f0-7124-11e5-81af-b16d74de8809") (query/run conn)))) "Elapsed time: 666.877891 msecs"

I removed most of the raw java.io logic and replaced it with a netty based abstraction clj-tcp. I was not able to devote as much time as I would have liked to this project over the weekend. But at this point I should just need to fix a race condition in read-response* and conduct far more scientific bench-marking.

I also added an optarg to changes so that we can make use of the squash parameter.

danielcompton commented 8 years ago

Awesome. I'm on paternity leave for the next two weeks so won't have a lot of time to look at it until afterwards, but open a PR when you're ready.

danielcompton commented 8 years ago

One difference between your two code samples (I think) is that the Ruby version uses an existing connection, while the Clojure one creates it. What are the timings when you use an existing connection?

danielcompton commented 8 years ago

Hey, I have a feeling that #114 was a big reason why there was such a massive performance difference between Ruby and Clojure. Would you be able to check again running clj-rethinkdb from master?

danielcompton commented 8 years ago

I'll close this for now, once you've tested master, can you let me know whether the speeds are comparable to the Ruby driver, and reopen if there is still a large gap?

KeeganMyers commented 8 years ago

I reviewed 114 and closed the related pull request specifically because of that change. I think that replacing input stream parsing will also yield performance improvements. However the changes required to do so are far too invasive.

danielcompton commented 8 years ago

Gotcha. I'd like to move across to the Java driver soon to make all of this a moot point :)