Hi Rob,
Sorry for the late response.
On 5/17/07, Rob Butler <[EMAIL PROTECTED]> wrote:
Hi Trustin,
Thanks for your rapid response. I guess my confusion stems from passing a
processorCount and Executor to SocketAcceptor. If you want a fixed number of
IO processors shouldn't you either:
1) Pass only a processorCount parameter and let SocketAcceptor use the most
appropriate Executor internally. Even better, maybe not use an executor at all
and just create a thread dedicated to each processorCount. Why bother with the
overhead of a pool if the thread is basically dedicated to the IO processor?
I/O processor threads exit when there's no more channels to manage,
and start to run a new connection is added to it. Creating a new
thread is an expensive task, so providing an executor will improve the
performance. We started to utilize Executor interface because one of
the users wanted to control how the threads are created in a container
environment.
or
2) Pass only an Executor and if you want a fixed number of IO processors use
Executor.newFixedThreadPool(nThreads).
My thought is someone could do something like this:
new SocketAcceptor(10, Executor.newFixedThreadPool(3));
And that's probably really a bad idea.
True. it should be Executors.newFixedThreadPool(11). (10 I/O
processors and one acceptor thread).
If instead the constructors were changed to:
public SocketAcceptor(){
this(1);
}
public SocketAcceptor(int processorCount){
this(processorCount, Executor.newFixedThreadPool(processorCount));
}
Looks good so far.
private SocketAcceptor(int processorCount, Executor executor){
// existing code of constructor here.
// or refactor code into SocketAcceptor(int processorCount) constructor
}
Things would be more bullet proof and prevent someone from making a mistake
like I've shown above. Since the Executor (actually ExecutorService) is
internal to the SocketAcceptor it would be responsible for calling
ExecutorServiceImpl.shutdown(). This could be done in the doUnbind() method of
SocketAcceptor.
Failing such modifications, the default constructor for SocketAcceptor uses
NewThreadExecutor(). Wouldn't it be more efficient to use
Executor.newFixedThreadPool(1)?
It will be more efficient, but user will not have control over the
life cycle of the executor (thread pool). When should shutdown() of
the thread pool called? I think most users will need to control it.
Instead, what about providing a self diagnostic code that detects when
a thread pool runs out of thread? For example, we could log something
when I/O processor's run() method is not being executed for certain
period of time (e.g. 60 sec) since Executor.execute() is called.
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6
