JBossManagedConnectionPool.getPool

[adrian.brock]

The synchronized block in this method has been identified as a potential
bottleneck.

 /** The subpools */
 private final Map pools = new HashMap();

 /** Track by connection by tx values */
 private final Map trackByTxPools = new ConcurrentReaderHashMap();


 /**
 * Determine the correct pool for this request,
 * creates a new one when necessary
 *
 * @param key the key to the pool
 * @param subject the subject of the pool
 * @param cri the connection request info
 * @return the pool
 * @throws ResourceException for any error
 */
 private InternalManagedConnectionPool getPool(Object key, Subject subject, ConnectionRequestInfo cri)
 throws ResourceException
 {
 InternalManagedConnectionPool mcp = null;
 synchronized (pools)
 {
 mcp = (InternalManagedConnectionPool) pools.get(key);
 if (mcp == null)
 {
 mcp = new InternalManagedConnectionPool(mcf, clf, subject, cri, poolParams, log);
 pools.put(key, mcp);
 TransactionManager tm = getTransactionManager();
 if (tm != null)
 trackByTxPools.put(key, new TransactionLocal(tm));
 }
 }
 return mcp;
 }

Again, this could do with "putIfNotPresent" construction semantics on a concurrent map.

However, there is a complication in that creating a new sub-pool currently has side-affects.
i.e. registering the subpool with the idle remover.

This will need to be separated into an initialization operation once the pool
has been established as the one to use (rather than just thrown away because
it is duplicate construction request).

[adrian.brock]

The other "side-affect" is the track-by-tx transaction local.

[adrian.brock]

Again, this could do with "putIfNotPresent" construction semantics on a concurrent map.

However, there is a complication in that creating a new sub-pool currently has side-affects.
i.e. registering the subpool with the idle remover.

This will need to be separated into an initialization operation once the pool
has been established as the one to use (rather than just thrown away because
it is duplicate construction request).

Let me reword this so it makes sense :-)

You can only use putIfNotPresent semantics if discarding the duplicate
instance does not cause problems.
In this case, the internal pool is registered with the idle remover in its constructor.
Which must either be undone or done after the "pool to use" is established.

The option of initalizing lazily can only work properly if it is "temporarily optional"
for the correct behaviour of the pool.
e.g. This race condition
Thread 1: Establish pool
Thread 2: Use created pool
Thread 1: Complete initialization of pool

In this case, the registration with the idle remover isn't important for Thread2
to use the pool.

The track-by-tx context is important, so it would be better if this moved
into a single context object that is added to one map (rather than two separate maps).

private class InternalPoolContext
{
 InternalManagedConnectionPool subPool;
 TransactionLocal trackByTx;
}

[adrian.brock]

JIRA issue: http://jira.jboss.com/jira/browse/JBAS-2578