[
https://issues.apache.org/jira/browse/GROOVY-12371?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18112055#comment-18112055
]
ASF GitHub Bot commented on GROOVY-12371:
-----------------------------------------
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.
> SQL: bound the statement cache and make it thread-safe
> ------------------------------------------------------
>
> Key: GROOVY-12371
> URL: https://issues.apache.org/jira/browse/GROOVY-12371
> Project: Groovy
> Issue Type: Task
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
