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 <[email protected]> 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: [email protected] > 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
