paulk-asert commented on code in PR #2895:
URL: https://github.com/apache/groovy/pull/2895#discussion_r3943889193
##########
subprojects/groovy-sql/src/main/java/groovy/sql/Sql.java:
##########
@@ -5057,8 +5111,18 @@ private Statement
getAbstractStatement(AbstractStatementCommand cmd, Connection
if (cacheStatements) {
stmt = statementCache.get(sql);
if (stmt == null) {
- stmt = cmd.execute(connection, sql);
- statementCache.put(sql, stmt);
+ Statement created = cmd.execute(connection, sql);
+ synchronized (statementCache) {
+ Statement raced = statementCache.get(sql);
Review Comment:
I think either approach is fine in terms of pros/cons.
> DCL moves cmd.execute inside the lock. That JDBC prepare is a DB
round-trip, and Collections.synchronizedMap uses one mutex for the whole map —
so while one thread prepares a statement under the lock, every other thread's
cache lookup and every other prepare — even of completely different SQL —
blocks on that same mutex. DCL serializes all statement preparation across a
shared Sql.
>
> My version deliberately never holds the mutex across the DB call, so
unrelated prepares proceed in parallel. The cost is exactly what Daniel
spotted: a rare double-create.
>
> So it's a genuine trade:
> - Mine: parallel prepares; a rare wasted prepare when two threads hit the
same brand-new SQL simultaneously.
> - His DCL: never double-creates; but serializes all preparation behind one
lock.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
