You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Maarten Bosteels <mb...@gmail.com> on 2007/11/22 22:50:19 UTC

Re: Another question on transition

Hello Frederic,

try connector.dispose();

http://mina.apache.org/report/trunk/apidocs/org/apache/mina/common/IoService.html#dispose()

Maarten

On Nov 22, 2007 6:53 PM, Frédéric Brégier <fr...@free.fr> wrote:

> Hi Trustin and all !
> In the past in trunk (6 month ago) we had :
> connector.setWorkerTimeout(1);
> But there is no more such a function now in trunk.
> Does it have a reason ?
>
> So far, it seems I almost finished (still without testing)
> rewriting my code, so at most 3 days.
>
> Thanks to all
>
> Frederic

Re: Another question on transition

Posted by Trustin Lee <tr...@gmail.com>.
On Nov 23, 2007 3:06 PM, Trustin Lee <tr...@gmail.com> wrote:
> Hi Frédéric,
>
> On Nov 23, 2007 7:53 AM, Frédéric Brégier <fr...@free.fr> wrote:
> > Hi Maarten,
> >
> > Correct me if I am missing something.
> > dispose() seems to release all ressources from the IoService (or connector), so when you don't need anymore the
> > connector.
> > setWorkerTimeout() was intend to release one thread only inside the pool of threads after the specified timeout
> > after no use.
> >
> > I read the code, specially from the 6 months ago trunk version and the current version.
> > It seems that in previous version Mina tries to re-use thread during some time before returning
> > them definitively to the pool (or to nothing since the default pool was NewThreadExecutor).
> > In case of NewThreadExecutor, trying to reuse thread instead of created a new one everyone,
> > I suppose it was the point.
> > Am I correct if I say that, since the underlying pool of thread in the new trunk
> > is a CachedPool, you don't need anymore this function as you rely on the
> > pool management from CachedPool itself, such that Mina don't have to take care of this ?
>
> 2.0 still uses NewThreadExecutor for acceptor & connector threads, so
> there's some overhead of spawning a new thread.  This becomes a
> problem when a user tries to request a bunch of connection with bad
> timing because threads will be spawned for each request.  To avoid
> that situation, I programmed AbstractPollingIoConnector's Worker not
> to exit until dispose() is called.  However, this is waste of thread
> if you explicitly specified a thread pool, as you pointed out.
>
> The solution could be not using NewThreadExecutor but using something
> different such as a cached thread pool executor.  However, we are not
> able to share the cached thread pool executor among all acceptors and
> connectors because it means user have to call some global shutdown
> method to shut down the shared thread pool.  Anyways, let me fix the
> problem by replacing NewThreadExecutor with
> Executors.newCachedThreadPool().  :)

Done.  It seems to work fine now.

Thanks,
Trustin
-- 
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6

Re: Another question on transition

Posted by Trustin Lee <tr...@gmail.com>.
Hi Frédéric,

Sorry for getting back to you late.  I was busy implementing JMX
integration.  I bet you will like it.  I will also look forward to
your feed back ;)

On Nov 23, 2007 5:10 PM, Frédéric Brégier <fr...@free.fr> wrote:
> Now in trunk, NioSocketConnector and NioSocketAcceptor
> use SimpleIoProcessorPool when you specify no Executor or no Processor.
> SimpleIoProcessorPool implies a newCachedThreadPool, in the past
> it was a NewThreadExecutor.
> That is what I point out in order to try to understand
> (and explain to myself and perhaps to others)
> why the setWorkerTimeout disapears from the trunk.
> In the past, the NewThreadExecutor implies to take care of the threads
> creation and deletion. Now with CachedPool, we don't have to.
> Correct me of course if I am wrong...

workerTimeout is not for a I/O processor thread but for a connector
thread.  Peter (Royal) had performance issue with fast connection
attempt, and that was why he added workerTimeout property to prevent
the connector thread from exiting and closing its selector, which slow
down the number of connections made per second seriously.  Now we have
explicit dispose() method in IoConnector, so there's no overhead for
closing and opening selector.  And we are not using NewThreadExecutor
but using a thread pool executor with 1 max pool size, so there's no
need for workerTimeout anymore, either.

> One remark on the same subject, on the first constructor of both
> NioSocketConnector and NioSocketAcceptor. I take the Acceptor
> as example since Connector is the same :
> - public NioSocketAcceptor()
>     OK, except the comment says NewThreadExecutor but it is in fact a
>     SimpleIoProcessorPool so a CachedPool and 1 single thread.
>     Looking at the code : it uses Runtime.getRuntime().availableProcessors() + 1 by default
>     thread in the CachedThreadPool and the same number of  IoProcessor.
> - public NioSocketAcceptor(int processorCount)
>     This one is clear at its does what it means (SimpleIoProcessorPool and processorCount of IoProcessor).
> - public NioSocketAcceptor(IoProcessor<NioSession> processor)
>     Same as previous except the processorCount is extract from processor and
>     creates an implicit SimpleIoProcessorPool.
> - public NioSocketAcceptor(Executor executor, IoProcessor<NioSession> processor)
>     Same as previous except the processorCount is extract from processor and
>     the executor is the one passed as argument.

We need a lot of documentation work here.  It is outdated right now
but we will make it ready to go before we release RC.

> Ok, sorry for so long mails and I fell like I can be a bit ennoying,
> but as it seems something to me that can affect performance a bit...

It was never annoying.  Thank you again for your continuous feed back,
and let me look forward to your another question. :)

Cheers,
Trustin
-- 
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6

Re: Another question on transition

Posted by Frédéric Brégier <fr...@free.fr>.
Hi Trustin,
Well you go so fast !! I didn't download again trunk right now (but will !).
In fact, I was probably not enough precise.
Let me say it again, because I feel like I mixed two things.
I agree NewThreadExecutor was a "simple" pool of thread since it was not
(until few hours) a real pool of thread but just a new thread created each time.

Now in trunk, NioSocketConnector and NioSocketAcceptor
use SimpleIoProcessorPool when you specify no Executor or no Processor.
SimpleIoProcessorPool implies a newCachedThreadPool, in the past
it was a NewThreadExecutor. 
That is what I point out in order to try to understand 
(and explain to myself and perhaps to others)
why the setWorkerTimeout disapears from the trunk.
In the past, the NewThreadExecutor implies to take care of the threads 
creation and deletion. Now with CachedPool, we don't have to.
Correct me of course if I am wrong...
If I am correct, probably on tip to add in the "1.x to 2.x" page...

Now on your modification, I see by looking at the snv through http
that you remove NewThreadExecutor. I hope this will be OK with
all part. I don't have any idea where this could be usefull somewhere,
so let see if the community is ok with that (I think they will).

One remark on the same subject, on the first constructor of both
NioSocketConnector and NioSocketAcceptor. I take the Acceptor
as example since Connector is the same :
- public NioSocketAcceptor() 
    OK, except the comment says NewThreadExecutor but it is in fact a 
    SimpleIoProcessorPool so a CachedPool and 1 single thread.
    Looking at the code : it uses Runtime.getRuntime().availableProcessors() + 1 by default
    thread in the CachedThreadPool and the same number of  IoProcessor.
- public NioSocketAcceptor(int processorCount) 
    This one is clear at its does what it means (SimpleIoProcessorPool and processorCount of IoProcessor).
- public NioSocketAcceptor(IoProcessor<NioSession> processor)
    Same as previous except the processorCount is extract from processor and 
    creates an implicit SimpleIoProcessorPool.
- public NioSocketAcceptor(Executor executor, IoProcessor<NioSession> processor)
    Same as previous except the processorCount is extract from processor and 
    the executor is the one passed as argument.

Ok, sorry for so long mails and I fell like I can be a bit ennoying,
but as it seems something to me that can affect performance a bit...

Thanks again Trustin !
Frederic

----- Original Message ----- 

2.0 still uses NewThreadExecutor for acceptor & connector threads, so
there's some overhead of spawning a new thread.  This becomes a
problem when a user tries to request a bunch of connection with bad
timing because threads will be spawned for each request.  To avoid
that situation, I programmed AbstractPollingIoConnector's Worker not
to exit until dispose() is called.  However, this is waste of thread
if you explicitly specified a thread pool, as you pointed out.

The solution could be not using NewThreadExecutor but using something
different such as a cached thread pool executor.  However, we are not
able to share the cached thread pool executor among all acceptors and
connectors because it means user have to call some global shutdown
method to shut down the shared thread pool.  Anyways, let me fix the
problem by replacing NewThreadExecutor with
Executors.newCachedThreadPool().  :)

Thank you so much for your feed back!

Cheers,
Trustin
-- 
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6


Re: Another question on transition

Posted by Trustin Lee <tr...@gmail.com>.
Hi Frédéric,

On Nov 23, 2007 7:53 AM, Frédéric Brégier <fr...@free.fr> wrote:
> Hi Maarten,
>
> Correct me if I am missing something.
> dispose() seems to release all ressources from the IoService (or connector), so when you don't need anymore the
> connector.
> setWorkerTimeout() was intend to release one thread only inside the pool of threads after the specified timeout
> after no use.
>
> I read the code, specially from the 6 months ago trunk version and the current version.
> It seems that in previous version Mina tries to re-use thread during some time before returning
> them definitively to the pool (or to nothing since the default pool was NewThreadExecutor).
> In case of NewThreadExecutor, trying to reuse thread instead of created a new one everyone,
> I suppose it was the point.
> Am I correct if I say that, since the underlying pool of thread in the new trunk
> is a CachedPool, you don't need anymore this function as you rely on the
> pool management from CachedPool itself, such that Mina don't have to take care of this ?

2.0 still uses NewThreadExecutor for acceptor & connector threads, so
there's some overhead of spawning a new thread.  This becomes a
problem when a user tries to request a bunch of connection with bad
timing because threads will be spawned for each request.  To avoid
that situation, I programmed AbstractPollingIoConnector's Worker not
to exit until dispose() is called.  However, this is waste of thread
if you explicitly specified a thread pool, as you pointed out.

The solution could be not using NewThreadExecutor but using something
different such as a cached thread pool executor.  However, we are not
able to share the cached thread pool executor among all acceptors and
connectors because it means user have to call some global shutdown
method to shut down the shared thread pool.  Anyways, let me fix the
problem by replacing NewThreadExecutor with
Executors.newCachedThreadPool().  :)

Thank you so much for your feed back!

Cheers,
Trustin
-- 
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6

Re: Another question on transition

Posted by Frédéric Brégier <fr...@free.fr>.
Hi Maarten,

Correct me if I am missing something.
dispose() seems to release all ressources from the IoService (or connector), so when you don't need anymore the 
connector.
setWorkerTimeout() was intend to release one thread only inside the pool of threads after the specified timeout 
after no use.

I read the code, specially from the 6 months ago trunk version and the current version.
It seems that in previous version Mina tries to re-use thread during some time before returning
them definitively to the pool (or to nothing since the default pool was NewThreadExecutor).
In case of NewThreadExecutor, trying to reuse thread instead of created a new one everyone,
I suppose it was the point.
Am I correct if I say that, since the underlying pool of thread in the new trunk
is a CachedPool, you don't need anymore this function as you rely on the
pool management from CachedPool itself, such that Mina don't have to take care of this ?

The more I read, the more I found myself (I hope not in wrong direction).
It was just a quick question as some users from 1.x will probably ask the question too
in a few weeks or so... So probably a new point in the "Change from 1.x to 2" web page.

Frederic

----- Original Message ----- 
From: "Maarten Bosteels"


Hello Frederic,

try connector.dispose();

http://mina.apache.org/report/trunk/apidocs/org/apache/mina/common/IoService.html#dispose()

Maarten

On Nov 22, 2007 6:53 PM, Frédéric Brégier <fr...@free.fr> wrote:

> Hi Trustin and all !
> In the past in trunk (6 month ago) we had :
> connector.setWorkerTimeout(1);
> But there is no more such a function now in trunk.
> Does it have a reason ?
>
> So far, it seems I almost finished (still without testing)
> rewriting my code, so at most 3 days.
>
> Thanks to all
>
> Frederic