Mailing-List: contact dev-help@activemq.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@activemq.apache.org
Received-SPF: pass (athena.apache.org: domain of gary.tully@gmail.com
 designates 74.125.82.45 as permitted sender)
MIME-Version: 1.0
In-Reply-To: 
 <CAGB5yNm=wy=ztwyYHNZKX5Jb4-EDP1UbLeYvEEtLyiCAV2NKwQ@mail.gmail.com>
References: 
 <CAGB5yNm=wy=ztwyYHNZKX5Jb4-EDP1UbLeYvEEtLyiCAV2NKwQ@mail.gmail.com>
Date: Tue, 28 Aug 2012 13:45:24 +0100
Message-ID: 
 <CAH+vQmN-8_GJUcMZ2bezqgJKmm-hPVEQbA77WWUx8bw2C4TVSw@mail.gmail.com>
Subject: Re: activemq-pool - Understanding the maxConnections option and this
 code
From: Gary Tully <gary.tully@gmail.com>
To: dev@activemq.apache.org
Content-Type: text/plain; charset=ISO-8859-1

the removeFirst/add will ensure that we iterate over the pooled
connections once we reach the pool limit but in the case of
maxConnections == 1 it is unnecessary.
A pool of one is not really a pool though!

On 28 August 2012 12:31, Claus Ibsen <claus.ibsen@gmail.com> wrote:
> Hi
>
> I have been working on a patch for
> https://issues.apache.org/jira/browse/AMQ-3997
>
> And stumbled across some code in activemq-pool I cannot understand the
> reason. I guess my coffee isn't strong enough. So I am asking here on
> @dev first.
>
> Its the code in createConnection method in PooledConnectionFactory. I
> am pasting it below, and marking a **** where I am puzzled
>
>
>     public synchronized Connection createConnection(String userName,
> String password) throws JMSException {
>         if (stopped.get()) {
>             LOG.debug("PooledConnectionFactory is stopped, skip create
> new connection.");
>             return null;
>         }
>
>         ConnectionKey key = new ConnectionKey(userName, password);
>         LinkedList<ConnectionPool> pools = cache.get(key);
>
>         if (pools == null) {
>             pools = new LinkedList<ConnectionPool>();
>             cache.put(key, pools);
>         }
>
>         ConnectionPool connection = null;
>         if (pools.size() == maxConnections) {
>             *********
>             connection = pools.removeFirst();
>         }
>
>         // Now.. we might get a connection, but it might be that we need to
>         // dump it..
>         if (connection != null && connection.expiredCheck()) {
>             connection = null;
>         }
>
>         if (connection == null) {
>             ActiveMQConnection delegate = createConnection(key);
>             connection = createConnectionPool(delegate);
>         }
>         pools.add(connection);
>         return new PooledConnection(connection);
>     }
>
> 1)
> If you look at the ****** spot in the code above. Then the
> ConnectionPool connection instance only get set if the pools.size() ==
> maxConnections. So if this condition is false, then we never get a
> pooled connection, and would always have to create a new connection,
> in the code just below.
>
> Now a reason this may not have spotted before, is that the default
> value for maxConnections is 1. And therefore the condition is true in
> those. Then we remove the pooled connection, which mean the pools size
> is one less (= zero). But then just before existing the method, we add
> it back to the pools, so the size will become 1 again.
> (Now this code is a bit inefficient as we will keep removing and
> adding the same connection over and over again.)
>
> So my question is also, what should happen when the maxConnections is
> larger than 1, eg maxConnections=8. Then we would create 8 connections
> and put in the pools. But only the first in the pools will ever be
> used (eg removeFirst). Notice the method is synchronized.
>
>
> 2)
> Another issues is that we wrap the connection in a PooledConnection,
> eg in the last code line. And we use the new constructor. Which mean a
> new instance is always created. And creating a new instance of
> PooledConnection is not cheap, as it has 2 internal lists. And any
> list/map is a bit expensive to create in java, especially the
> concurrent ones.
>
> During my hunt for AMQ-3997 I noticed a lot of instances of concurrent
> maps/lists and their internal instances such as $Node etc being
> created. We may optimize this further to gain speed. It seems this
> code is 10% slower than the spring cached pool (a very rough estimate
> based on times outputted from that test code, with the link from
> AMQ-3997).
>
>
>
> Anyway I guess I am going for stronger coffee.
>
>
>
>
>
> --
> Claus Ibsen
> -----------------
> FuseSource
> Email: cibsen@fusesource.com
> Web: http://fusesource.com
> Twitter: davsclaus, fusenews
> Blog: http://davsclaus.com
> Author of Camel in Action: http://www.manning.com/ibsen


-- 
http://fusesource.com
http://blog.garytully.com