3 Replies Latest reply on Dec 19, 2005 1:27 PM by adrian.brock

    JBossManagedConnectionPool.getPool

      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).

        • 1. Re: JBossManagedConnectionPool.getPool

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

          • 2. Re: JBossManagedConnectionPool.getPool

             

            "adrian@jboss.org" wrote:

            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;
            }
            


            • 3. Re: JBossManagedConnectionPool.getPool