search

taoensso / carmine

Redis client + message queue for Clojure
https://www.taoensso.com/carmine
Eclipse Public License 1.0
1.16k stars 132 forks source link

Reflection in pooled-conn function is very expensive #56

Closed harob closed 11 years ago

harob commented 11 years ago

While profiling a performance-critical Clojure app using Carmine (using the VisualVM sampler), it looks like the call to (satisfies? IConnectionPool opts) in connections/pooled-coon is extremely expensive (taking as much CPU time as all the rest of the work done in Carmine+redis). This is because the call to satisfies? ultimately results in a Class.getInterfaces reflection call.

Furthermore, from looking at the code I can't see why this check is even necessary; a comment mentions "; Pass through pre-made pools", but I can't see any case or the code or documentation where a caller would be passing an IConnectionPool object rather than a simple map of options.

If the IConnectionPool case is no longer necessary then presumably we can just remove the satisfies? check all together; if it still is then maybe we can avoid reflection by passing in the information about its type in an argument rather than using reflection?

Thanks!

ptaoussanis commented 11 years ago

Hi Harry, thanks a lot for bringing this to my attention. All my own use cases (incl. the benchmarks unfortunately) all used default (nil) opts which short circuit the reflection.

The satisfies? call was for API compatibility with Carmine v1 and for cases where folks want to provide their own pool implementation - but we can achieve the same result w/o reflection.

Just pushed v2.2.2 with the fix. Please let me know if you run into any further issues - cheers! :-)

harob commented 11 years ago

Thanks Peter, that was amazing turn-around on this!