Impressive. +1  A 20% performance increase is amazing.  Way to go!

I'm curious.  What was causing the bottleneck with ExecutorFilter +
Executors.newFixedThreadPool() that UnorderedExecutorFilter +
OrderedThreadPoolExecutor overcomes?  Did you find this bottleneck using
a profiler?

After looking over OrderedThreadPoolExecutor, the only suggestion I have
is to modify setMaximumPoolSize(int maximumPoolSize).  Currently if the
new  maximumPoolSize is smaller then the actual maximumPoolSize, it
invokes removeWorker() multiple times based on the difference.
Shouldn't it really look something like:

    @Override
    public void setMaximumPoolSize(int maximumPoolSize) {
        synchronized (workers) {
            if (maximumPoolSize > corePoolSize) {
                setCorePoolSize(maximumPoolSize);
            }
            this.maximumPoolSize = maximumPoolSize;
        }
    }

This way the corePoolSize field will get updated if need be and
setCorePoolSize will update the the workers size if need be.  I don't
have much experience with the implementations of ThreadPoolExecutors so
let me know if I'm off base.

-Mike

Trustin Lee wrote:
> Hi community,
> 
> I've just implemented a subclass of ThreadPoolExecutor that accepts
> only IoEvents and IoFilterEvents and maintains the order of events by
> itself (i.e. without any help of ExecutorFilter).
> 
> The result is impressive; UnorderedExecutorFilter +
> OrderedThreadPoolFilter outperforms ExecutorFilter +
> Executors.newFixedThreadPool() by apx 20% in my machine.
> 
> Please review the code:
> 
> http://svn.apache.org/viewvc/mina/trunk/core/src/main/java/org/apache/mina/filter/executor/OrderedThreadPoolExecutor.java?view=markup
> 
> One limitation is that you cannot specify the Queue implementation.
> Except that, I think I implemented all features of the
> ThreadPoolExecutor.
> 
> So, what do you think about:
> 
> * Removing ExecutorFilter,
> * merging AbstractExecutorFilter and UnorderedExecutorFilter into the
> new ExecutorFilter
> * and letting it use OrderedThreadPoolExecutor by default?
> 
> I think most users won't need to specify other executor than
> OrderedThreadPoolExecutor and providing many convenience constructors
> in ExecutorFilter will compensate the possible breakage of backward
> compatibility.
> 
> If there's no objection, let me go ahead.
> 
> Trustin

Reply via email to