Hi Babak,

Thanks for the book reference, it made a good point, I will update the code 
with your suggestion shortly.

--  
Willem Jiang

Red Hat, Inc.
Web: http://www.redhat.com
Blog: http://willemjiang.blogspot.com (English)
http://jnn.iteye.com (Chinese)
Twitter: willemjiang  
Weibo: 姜宁willem



On December 1, 2014 at 10:23:17 PM, Babak Vahdat (babak.vah...@swissonline.ch) 
wrote:
>  
>  
> Am 01.12.14 14:49 schrieb "Willem Jiang" unter :
>  
> >Hi Babak,
> >
> >As we just create the UnstoppableTime instance inside of the
> >NettyClientBossPoolBuilder and don’t want to other people to create the
> >instance out side of NettyClientBossPoolBuilder, I don’t think we need to
> >define the UnstoppableTime as the static member class.
>  
> Maybe I was not clear enough about what I mean:
>  
> Instead of the current class declaration which is:
> class UnstoppableTimer implements Timer {
>  
> We should better do:
> private static class UnstoppableTimer implements Timer {
>  
> That is:
>  
> add 'private' => Instead of the current 'package private' access we get a
> more restricted visibility which would be *preciser*.
> add 'static' => avoid the extraneous reference to its *every* enclosing
> instance
>  
> See also Effective Java, Item 22, page 106: Favor static member classes
> over nonstatic
>  
> Babak
>  
>  
> >
> >--
> >Willem Jiang
> >
> >Red Hat, Inc.
> >Web: http://www.redhat.com
> >Blog: http://willemjiang.blogspot.com (English)
> >http://jnn.iteye.com (Chinese)
> >Twitter: willemjiang
> >Weibo: 姜宁willem
> >
> >
> >
> >On December 1, 2014 at 1:57:31 PM, Babak Vahdat
> >(babak.vah...@swissonline.ch) wrote:
> >> Hi Willem,
> >>
> >> Yeah that makes sense.
> >>
> >> I guess the newly introduced UnstoppableTimer should be better declared
> >>as
> >> (private) *static* member class instead of nonstatic. This would avoid
> >>the
> >> extraneous reference to its *every* enclosing instance (which is
> >> NettyClientBossPoolBuilder).
> >>
> >> Babak
> >>
> >>
> >> Am 01.12.14 05:57 schrieb "Willem Jiang" unter :
> >>
> >> >Hi Babak,
> >> >
> >> >As the we share the timer across the camel producers, we cannot just
> >> >simply call the releaseExternalResources method which stops the shared
> >> >timer here.
> >> >
> >> >So I did change on the NettyClientBossPoolBuilder to avoid stopping the
> >> >timer by wrapping it
> >> >
> >> >--
> >> >Willem Jiang
> >> >
> >> >Red Hat, Inc.
> >> >Web: http://www.redhat.com
> >> >Blog: http://willemjiang.blogspot.com (English)
> >> >http://jnn.iteye.com (Chinese)
> >> >Twitter: willemjiang
> >> >Weibo: 姜宁willem
> >> >
> >> >
> >> >
> >> >On November 30, 2014 at 7:06:57 PM, bvah...@apache.org
> >> >(bvah...@apache.org) wrote:
> >> >> Repository: camel
> >> >> Updated Branches:
> >> >> refs/heads/master 2f4010e03 -> 829458c70
> >> >>
> >> >>
> >> >> CAMEL-8094: Do not use org.jboss.netty.util.internal.ExecutorUtil as
> >>it
> >> >>breaks
> >> >> the camel-netty Karaf feature
> >> >>
> >> >> Project: http://git-wip-us.apache.org/repos/asf/camel/repo
> >> >> Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/829458c7  
> >> >> Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/829458c7
> >> >> Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/829458c7
> >> >>
> >> >> Branch: refs/heads/master
> >> >> Commit: 829458c700a8dcbaa0d710a7d77ff12636afa0a7
> >> >> Parents: 2f4010e
> >> >> Author: Babak Vahdat
> >> >> Authored: Sun Nov 30 12:06:09 2014 +0100
> >> >> Committer: Babak Vahdat
> >> >> Committed: Sun Nov 30 12:06:09 2014 +0100
> >> >>
> >> >>
> >>----------------------------------------------------------------------  
> >> >> .../netty/NettyClientBossPoolBuilder.java | 16 -----------
> >> >> .../camel/component/netty/NettyProducer.java | 14 +++++++---
> >> >> .../netty/NettyServerBossPoolBuilder.java | 18 +------------
> >> >> .../component/netty/NettyWorkerPoolBuilder.java | 28
> >> >>+-------------------
> >> >> 4 files changed, 13 insertions(+), 63 deletions(-)
> >> >>
> >>----------------------------------------------------------------------  
> >> >>
> >> >>
> >> >>
> >>
> >>>>http://git-wip-us.apache.org/repos/asf/camel/blob/829458c7/components/c  
> >>>>am
> >>
> >>>>el-netty/src/main/java/org/apache/camel/component/netty/NettyClientBoss  
> >>>>Po
> >> >>olBuilder.java
> >> >>
> >>----------------------------------------------------------------------  
> >> >> diff --git
> >>
> >>>>a/components/camel-netty/src/main/java/org/apache/camel/component/netty  
> >>>>/N
> >> >>ettyClientBossPoolBuilder.java
> >> >>
> >>
> >>>>b/components/camel-netty/src/main/java/org/apache/camel/component/netty  
> >>>>/N
> >> >>ettyClientBossPoolBuilder.java
> >> >> index cc9bfb0..62caf1e 100644
> >> >> ---
> >>
> >>>>a/components/camel-netty/src/main/java/org/apache/camel/component/netty  
> >>>>/N
> >> >>ettyClientBossPoolBuilder.java
> >> >> +++
> >>
> >>>>b/components/camel-netty/src/main/java/org/apache/camel/component/netty  
> >>>>/N
> >> >>ettyClientBossPoolBuilder.java
> >> >> @@ -23,7 +23,6 @@ import org.jboss.netty.channel.socket.nio.BossPool;
> >> >> import org.jboss.netty.channel.socket.nio.NioClientBossPool;
> >> >> import org.jboss.netty.util.ThreadNameDeterminer;
> >> >> import org.jboss.netty.util.Timer;
> >> >> -import org.jboss.netty.util.internal.ExecutorUtil;
> >> >>
> >> >> /**
> >> >> * A builder to create Netty {@link
> >> >>org.jboss.netty.channel.socket.nio.BossPool}
> >> >> which can be used for sharing boss pools
> >> >> @@ -78,19 +77,4 @@ public final class NettyClientBossPoolBuilder {
> >> >> BossPool build() {
> >> >> return new NioClientBossPool(Executors.newCachedThreadPool(),
> >> >>bossCount, timer,
> >> >> new CamelNettyThreadNameDeterminer(pattern, name));
> >> >> }
> >> >> -
> >> >> - class CamelNioClientBossPool extends NioClientBossPool {
> >> >> - private Executor executor;
> >> >> - CamelNioClientBossPool(Executor bossExecutor, int bossCount, Timer
> >> >>timer, ThreadNameDeterminer
> >> >> determiner) {
> >> >> - super(bossExecutor, bossCount, timer, determiner);
> >> >> - executor = bossExecutor;
> >> >> - }
> >> >> -
> >> >> - // Just make sure we shutdown the executor;
> >> >> - public void shutdown() {
> >> >> - super.shutdown();
> >> >> - ExecutorUtil.shutdownNow(executor);
> >> >> - }
> >> >> -
> >> >> - }
> >> >> }
> >> >>
> >> >>
> >>
> >>>>http://git-wip-us.apache.org/repos/asf/camel/blob/829458c7/components/c  
> >>>>am
> >>
> >>>>el-netty/src/main/java/org/apache/camel/component/netty/NettyProducer.j  
> >>>>av
> >> >>a
> >> >>
> >>----------------------------------------------------------------------  
> >> >> diff --git
> >>
> >>>>a/components/camel-netty/src/main/java/org/apache/camel/component/netty  
> >>>>/N
> >> >>ettyProducer.java
> >> >>
> >>
> >>>>b/components/camel-netty/src/main/java/org/apache/camel/component/netty  
> >>>>/N
> >> >>ettyProducer.java
> >> >> index 491ee38..ac1ecef 100644
> >> >> ---
> >>
> >>>>a/components/camel-netty/src/main/java/org/apache/camel/component/netty  
> >>>>/N
> >> >>ettyProducer.java
> >> >> +++
> >>
> >>>>b/components/camel-netty/src/main/java/org/apache/camel/component/netty  
> >>>>/N
> >> >>ettyProducer.java
> >> >> @@ -52,6 +52,7 @@ import
> >> >>org.jboss.netty.channel.socket.nio.NioClientSocketChannelFactory;
> >> >> import org.jboss.netty.channel.socket.nio.NioDatagramChannelFactory;  
> >> >> import org.jboss.netty.channel.socket.nio.NioDatagramWorkerPool;
> >> >> import org.jboss.netty.channel.socket.nio.WorkerPool;
> >> >> +import org.jboss.netty.util.ExternalResourceReleasable;
> >> >> import org.slf4j.Logger;
> >> >> import org.slf4j.LoggerFactory;
> >> >>
> >> >> @@ -161,7 +162,12 @@ public class NettyProducer extends
> >> >>DefaultAsyncProducer {
> >> >> bossPool = null;
> >> >> }
> >> >> if (workerPool != null) {
> >> >> - workerPool.shutdown();
> >> >> + if (workerPool instanceof ExternalResourceReleasable) {
> >> >> + // this will first invoke workerPool#shutdown() internally (e.g.
> >> >>org.jboss.netty.channel.socket.nio.AbstractNioWorkerPool)
> >> >> + ((ExternalResourceReleasable)
> >>workerPool).releaseExternalResources();
> >> >> + } else {
> >> >> + workerPool.shutdown();
> >> >> + }
> >> >> workerPool = null;
> >> >> }
> >> >>
> >> >> @@ -174,12 +180,14 @@ public class NettyProducer extends
> >> >>DefaultAsyncProducer {
> >> >> }
> >> >>
> >> >> if (channelFactory != null) {
> >> >> - channelFactory.shutdown();
> >> >> + // this will first invoke channelFactory#shutdown() internally (see
> >> >>it's javadoc)
> >> >> + channelFactory.releaseExternalResources();
> >> >> channelFactory = null;
> >> >> }
> >> >>
> >> >> if (datagramChannelFactory != null) {
> >> >> - datagramChannelFactory.shutdown();
> >> >> + // this will first invoke datagramChannelFactory#shutdown()
> >> >>internally (see it's
> >> >> javadoc)
> >> >> + datagramChannelFactory.releaseExternalResources();
> >> >> datagramChannelFactory = null;
> >> >> }
> >> >>
> >> >>
> >> >>
> >>
> >>>>http://git-wip-us.apache.org/repos/asf/camel/blob/829458c7/components/c  
> >>>>am
> >>
> >>>>el-netty/src/main/java/org/apache/camel/component/netty/NettyServerBoss  
> >>>>Po
> >> >>olBuilder.java
> >> >>
> >>----------------------------------------------------------------------  
> >> >> diff --git
> >>
> >>>>a/components/camel-netty/src/main/java/org/apache/camel/component/netty  
> >>>>/N
> >> >>ettyServerBossPoolBuilder.java
> >> >>
> >>
> >>>>b/components/camel-netty/src/main/java/org/apache/camel/component/netty  
> >>>>/N
> >> >>ettyServerBossPoolBuilder.java
> >> >> index 3be5a64..6404ae1 100644
> >> >> ---
> >>
> >>>>a/components/camel-netty/src/main/java/org/apache/camel/component/netty  
> >>>>/N
> >> >>ettyServerBossPoolBuilder.java
> >> >> +++
> >>
> >>>>b/components/camel-netty/src/main/java/org/apache/camel/component/netty  
> >>>>/N
> >> >>ettyServerBossPoolBuilder.java
> >> >> @@ -22,7 +22,6 @@ import java.util.concurrent.Executors;
> >> >> import org.jboss.netty.channel.socket.nio.BossPool;
> >> >> import org.jboss.netty.channel.socket.nio.NioServerBossPool;
> >> >> import org.jboss.netty.util.ThreadNameDeterminer;
> >> >> -import org.jboss.netty.util.internal.ExecutorUtil;
> >> >>
> >> >> /**
> >> >> * A builder to create Netty {@link
> >> >>org.jboss.netty.channel.socket.nio.BossPool}
> >> >> which can be used for sharing boss pools
> >> >> @@ -65,21 +64,6 @@ public final class NettyServerBossPoolBuilder {
> >> >> * Creates a new boss pool.
> >> >> */
> >> >> BossPool build() {
> >> >> - return new CamelNioServerBossPool(Executors.newCachedThreadPool(),
> >> >>bossCount,
> >> >> new CamelNettyThreadNameDeterminer(pattern, name));
> >> >> - }
> >> >> -
> >> >> - class CamelNioServerBossPool extends NioServerBossPool {
> >> >> - private Executor executor;
> >> >> - CamelNioServerBossPool(Executor bossExecutor, int bossCount,
> >> >>ThreadNameDeterminer
> >> >> determiner) {
> >> >> - super(bossExecutor, bossCount, determiner);
> >> >> - executor = bossExecutor;
> >> >> - }
> >> >> -
> >> >> - // Just make sure we shutdown the executor;
> >> >> - public void shutdown() {
> >> >> - super.shutdown();
> >> >> - ExecutorUtil.shutdownNow(executor);
> >> >> - }
> >> >> -
> >> >> + return new NioServerBossPool(Executors.newCachedThreadPool(),
> >> >>bossCount, new
> >> >> CamelNettyThreadNameDeterminer(pattern, name));
> >> >> }
> >> >> }
> >> >>
> >> >>
> >>
> >>>>http://git-wip-us.apache.org/repos/asf/camel/blob/829458c7/components/c  
> >>>>am
> >>
> >>>>el-netty/src/main/java/org/apache/camel/component/netty/NettyWorkerPool  
> >>>>Bu
> >> >>ilder.java
> >> >>
> >>----------------------------------------------------------------------  
> >> >> diff --git
> >>
> >>>>a/components/camel-netty/src/main/java/org/apache/camel/component/netty  
> >>>>/N
> >> >>ettyWorkerPoolBuilder.java
> >> >>
> >>
> >>>>b/components/camel-netty/src/main/java/org/apache/camel/component/netty  
> >>>>/N
> >> >>ettyWorkerPoolBuilder.java
> >> >> index 5b6253a..c60cc25 100644
> >> >> ---
> >>
> >>>>a/components/camel-netty/src/main/java/org/apache/camel/component/netty  
> >>>>/N
> >> >>ettyWorkerPoolBuilder.java
> >> >> +++
> >>
> >>>>b/components/camel-netty/src/main/java/org/apache/camel/component/netty  
> >>>>/N
> >> >>ettyWorkerPoolBuilder.java
> >> >> @@ -22,7 +22,6 @@ import java.util.concurrent.Executors;
> >> >> import org.jboss.netty.channel.socket.nio.NioWorkerPool;
> >> >> import org.jboss.netty.channel.socket.nio.WorkerPool;
> >> >> import org.jboss.netty.util.ThreadNameDeterminer;
> >> >> -import org.jboss.netty.util.internal.ExecutorUtil;
> >> >>
> >> >> /**
> >> >> * A builder to create Netty {@link WorkerPool} which can be used for
> >> >>sharing worker pools
> >> >> @@ -67,32 +66,7 @@ public final class NettyWorkerPoolBuilder {
> >> >> */
> >> >> public WorkerPool build() {
> >> >> int count = workerCount > 0 ? workerCount :
> >> >>NettyHelper.DEFAULT_IO_THREADS;
> >> >> - workerPool = new
> >>CamelNioWorkerPool(Executors.newCachedThreadPool(),
> >> >>count,
> >> >> new CamelNettyThreadNameDeterminer(pattern, name));
> >> >> + workerPool = new NioWorkerPool(Executors.newCachedThreadPool(),
> >> >>count, new CamelNettyThreadNameDeterminer(pattern,
> >> >> name));
> >> >> return workerPool;
> >> >> }
> >> >> -
> >> >> - class CamelNioWorkerPool extends NioWorkerPool {
> >> >> - private Executor executor;
> >> >> - CamelNioWorkerPool(Executor workerExecutor, int count,
> >> >>ThreadNameDeterminer
> >> >> determiner) {
> >> >> - super(workerExecutor, count, determiner);
> >> >> - executor = workerExecutor;
> >> >> - }
> >> >> -
> >> >> - // Just make sure we shutdown the executor;
> >> >> - public void shutdown() {
> >> >> - super.shutdown();
> >> >> - ExecutorUtil.shutdownNow(executor);
> >> >> - }
> >> >> -
> >> >> - }
> >> >> -
> >> >> - /**
> >> >> - * Shutdown the created worker pool
> >> >> - */
> >> >> - public void destroy() {
> >> >> - if (workerPool != null) {
> >> >> - workerPool.shutdown();
> >> >> - workerPool = null;
> >> >> - }
> >> >> - }
> >> >> }
> >> >>
> >> >>
> >> >
> >>
> >>
> >>
> >
>  
>  
>  

Reply via email to