JdbcXAConnection.PooledJdbcConnection.checkClosed() performance

[davide.cavestro]

Hi,
I noticed that / JdbcXAConnection.PooledJdbcConnection.checkClosed(boolean)
<http://h2database.googlecode.com/svn/tags/version-1.3.171/h2/src/main/org/h2/jdbcx/JdbcXAConnection.java>
/ is a /synchronized/ method.

Also taking some random thread dumps I noticed it seems called very
frequently (indirectly through /wasNull()/ at every field value extraction).

Even if I've not profiled it yet I suppose *there could be room enough for
some optimization*.
i.e. maybe you could use a synchronized block containing only the /isClosed/
check. I don't know if this is a situation where some double-checked
locking <http://en.wikipedia.org/wiki/Double-checked_locking> variant
could be applied, but it seems strange having a thread safe
/org.h2.jdbc.JdbcConnection/ with no synchronization on its /checkClosed/
method, overridden with a synchronized method.

Please note I'm not an expert, and at the moment I have no visibility enough
to propose a valid solution, but I only want to suggest you considering this
issue, cause on multi-core CPUs
<http://stackoverflow.com/questions/973518/are-synchronized-methods-slower-in-single-threaded-applications>
it could possibly lead to performance enhancements.



--
View this message in context: http://h2-database.66688.n3.nabble.com/JdbcXAConnection-PooledJdbcConnection-checkClosed-performance-tp4026417.html
Sent from the H2 Database mailing list archive at Nabble.com.

[Noel Grandin]

Show us the profile first, then we'll bother adding extra complexity.

"Premature optimisation is the root of all evil"

http://c2.com/cgi/wiki?PrematureOptimization

[Noel Grandin]

Specifically, the JVM uses a combination of techniques known as "thin
locking" and "biased locking" which means that synchronization is
relatively cheap as long as it's confined to the same method.
Synchronization only becomes expensive when you have multiple different
threads hitting the same synchronized block.

You are correct, the JdbcConnection#checkClosed() method should probably
be synchronized.
I didn't write the code so I can't say for sure, but I suspect it was
coded that way to catch fairly basic programming mistakes i.e. mistakes
in single-threaded code.

The JdbcXAConnection#checkClosed() method on the other hand, knows that
it is being used in a multi-thread context i.e. connection pooling, so
it plays it more carefully.
And given how often connection pooling libraries manage to get it wrong,
it makes a lot of sense.

On 2013-05-14 13:02, davide.cavestro wrote:

[davide.cavestro]

I agree with this *kind of analysis*, not the former ;-)

I'll manage to gather some samplings ASAP (as soon as I get an environment
with huge datasets an a jrockit jvm available). If it doesn't show
synchronization issues, then it is of course safer not to change the raw but
*tested solution*.

Cheers
Davide


Noel Grandin wrote

> Specifically, the JVM uses a combination of techniques known as "thin
> locking" and "biased locking" which means that synchronization is
> relatively cheap as long as it's confined to the same method.
> Synchronization only becomes expensive when you have multiple different
> threads hitting the same synchronized block.
>
> You are correct, the JdbcConnection#checkClosed() method should probably
> be synchronized.
> I didn't write the code so I can't say for sure, but I suspect it was
> coded that way to catch fairly basic programming mistakes i.e. mistakes
> in single-threaded code.
>
> The JdbcXAConnection#checkClosed() method on the other hand, knows that
> it is being used in a multi-thread context i.e. connection pooling, so
> it plays it more carefully.
> And given how often connection pooling libraries manage to get it wrong,
> it makes a lot of sense.
>
> On 2013-05-14 13:02, davide.cavestro wrote:
>> Hi,
>> I noticed that /
>> JdbcXAConnection.PooledJdbcConnection.checkClosed(boolean)

>> &lt;http://h2database.googlecode.com/svn/tags/version-1.3.171/h2/src/main/org/h2/jdbcx/JdbcXAConnection.java&gt;

>> / is a /synchronized/ method.
>>
>> Also taking some random thread dumps I noticed it seems called very
>> frequently (indirectly through /wasNull()/ at every field value
>> extraction).
>>
>> Even if I've not profiled it yet I suppose *there could be room enough
>> for
>> some optimization*.
>> i.e. maybe you could use a synchronized block containing only the
>> /isClosed/
>> check. I don't know if this is a situation where some double-checked

>> locking &lt;http://en.wikipedia.org/wiki/Double-checked_locking&gt;

>> variant
>> could be applied, but it seems strange having a thread safe
>> /org.h2.jdbc.JdbcConnection/ with no synchronization on its /checkClosed/
>> method, overridden with a synchronized method.
>>
>> Please note I'm not an expert, and at the moment I have no visibility
>> enough
>> to propose a valid solution, but I only want to suggest you considering
>> this
>> issue, cause on multi-core CPUs

>> &lt;http://stackoverflow.com/questions/973518/are-synchronized-methods-slower-in-single-threaded-applications&gt;

>> it could possibly lead to performance enhancements.
>>
>>
>>
>> --
>> View this message in context:
>> http://h2-database.66688.n3.nabble.com/JdbcXAConnection-PooledJdbcConnection-checkClosed-performance-tp4026417.html
>> Sent from the H2 Database mailing list archive at Nabble.com.
>>
>

> --
> You received this message because you are subscribed to the Google Groups
> "H2 Database" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to

> h2-database+unsubscribe@

> .
> To post to this group, send email to

> h2-database@

> .
> Visit this group at http://groups.google.com/group/h2-database?hl=en.
> For more options, visit https://groups.google.com/groups/opt_out.





--
View this message in context: http://h2-database.66688.n3.nabble.com/JdbcXAConnection-PooledJdbcConnection-checkClosed-performance-tp4026417p4026420.html