Am 01.12.14 14:49 schrieb "Willem Jiang" unter <willem.ji...@gmail.com>:

>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