Prepared statement caching does not work

[Joni Freeman]

It looks like 0.8 version never returns cached PreparedStatements. I suspect that this check always resets the cached statement to null:

https://github.com/wwadge/bonecp/blob/master/bonecp/src/main/java/com/jolbox/bonecp/StatementCache.java#L162

That line was changed in this commit:

https://github.com/wwadge/bonecp/commit/9ccd3c4a7994f8e515195cb35ed57c1201632b1a

I noticed this bug while working on Databench benchmark application (http://databen.ch). Downgrading to BoneCP 0.7.1 fixed the performance problem with statement caching:

https://github.com/databench/databench/pull/4

[Just Asking For A CD (java-artisan)]

Yup. Same here. I've got a huge batch of millions of the very same insert statement. They're not cached at all.

With 0.7.1 the jmx statistics indicate caching, but still indicates the creation of prepared statements too.

[Wallace Wadge (wwadge)]

I disagree. That line says: if you're trying to use the same statement but it's already marked as being in use, don't retreive from the cache but return null so a new statement get's created.

Remember that for statement caching to work, you must be recycling the same connection because statements are bound to a connection so this would be cached:

Connection c = getConnection()
Statement s = c.prepareStatement("foo")
s.close();
s2 = c.prepareStatement("foo")

while this won't:
Connection c = getConnection()
Statement s = c.prepareStatement("foo")
s.close();
Connection c2 = getConnection()
s2 = c2.prepareStatement("foo")

is that what you're seeing?

[Joni Freeman (joni-freeman)]

I'm using pooled connections so eventually getConnection() will return a connection that already saw a particular prepared statement. 0.7.1 seems to work this way at least. Has the semantics of statement caching been changed between 0.7.1 and 0.8?

[Wallace Wadge (wwadge)]

What changed is the removal of the release statement helper threads + connection handles are no longer recycled but recreated so unless there are bugs, nothing much should have changed.

I have a unit test to check for caching that seems to work fine - are you able to provide me with a unit test?

[Joni Freeman (joni-freeman)]

I have a performance test which shows a dramatic difference between 0.7.1 and 0.8. I can try to simplify that test to a bare minimum if that's ok?

[Wallace Wadge (wwadge)]

Yes please :-)

I'm kinda surprised you see a performance difference, JDBC drivers can do their own caching too (which is why I default to off).

[Joni Freeman (joni-freeman)]

Benchmark which shows the performance degradation:

https://github.com/jonifreeman/bonecp_bug_1168616

[Wallace Wadge (wwadge)]

Found it - it was a one-liner fix. Thanks Joni!

Deployed fix in 0.8.0-rc4-SNAPSHOT

[Joni Freeman (joni-freeman)]

Cool, thanks a lot!