You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Julien Vermillard <jv...@apache.org> on 2009/06/29 11:13:24 UTC
Re: svn commit: r789164 - /mina/trunk/core/src/main/java/org/apache/mina/core/service/SimpleIoProcessorPool.java
Hi,
Interesting optimization. The idea is to make SimpleIoPPool
instantiation faster ?
Julien
On Mon, Jun 29, 2009 at 12:30 AM, <ed...@apache.org> wrote:
> Author: edeoliveira
> Date: Sun Jun 28 22:30:02 2009
> New Revision: 789164
>
> URL: http://svn.apache.org/viewvc?rev=789164&view=rev
> Log:
> Fix to do less reflection ops whilie initialising the pool
>
> Modified:
> mina/trunk/core/src/main/java/org/apache/mina/core/service/SimpleIoProcessorPool.java
>
> Modified: mina/trunk/core/src/main/java/org/apache/mina/core/service/SimpleIoProcessorPool.java
> URL: http://svn.apache.org/viewvc/mina/trunk/core/src/main/java/org/apache/mina/core/service/SimpleIoProcessorPool.java?rev=789164&r1=789163&r2=789164&view=diff
> ==============================================================================
> --- mina/trunk/core/src/main/java/org/apache/mina/core/service/SimpleIoProcessorPool.java (original)
> +++ mina/trunk/core/src/main/java/org/apache/mina/core/service/SimpleIoProcessorPool.java Sun Jun 28 22:30:02 2009
> @@ -19,6 +19,7 @@
> */
> package org.apache.mina.core.service;
>
> +import java.lang.reflect.Constructor;
> import java.util.concurrent.Executor;
> import java.util.concurrent.ExecutorService;
> import java.util.concurrent.Executors;
> @@ -73,44 +74,57 @@
> * @param <T> the type of the {@link IoSession} to be managed by the specified
> * {@link IoProcessor}.
> */
> -public class SimpleIoProcessorPool<T extends AbstractIoSession> implements IoProcessor<T> {
> -
> - private static final int DEFAULT_SIZE = Runtime.getRuntime().availableProcessors() + 1;
> - private static final AttributeKey PROCESSOR = new AttributeKey(SimpleIoProcessorPool.class, "processor");
> -
> - private final static Logger LOGGER = LoggerFactory.getLogger(SimpleIoProcessorPool.class);
> +public class SimpleIoProcessorPool<T extends AbstractIoSession> implements
> + IoProcessor<T> {
> +
> + private static final int DEFAULT_SIZE = Runtime.getRuntime()
> + .availableProcessors() + 1;
> +
> + private static final AttributeKey PROCESSOR = new AttributeKey(
> + SimpleIoProcessorPool.class, "processor");
> +
> + private final static Logger LOGGER = LoggerFactory
> + .getLogger(SimpleIoProcessorPool.class);
>
> private final IoProcessor<T>[] pool;
> +
> private final AtomicInteger processorDistributor = new AtomicInteger();
> +
> private final Executor executor;
> +
> private final boolean createdExecutor;
> -
> +
> private final Object disposalLock = new Object();
> +
> private volatile boolean disposing;
> +
> private volatile boolean disposed;
> -
> +
> public SimpleIoProcessorPool(Class<? extends IoProcessor<T>> processorType) {
> this(processorType, null, DEFAULT_SIZE);
> }
> -
> - public SimpleIoProcessorPool(Class<? extends IoProcessor<T>> processorType, int size) {
> +
> + public SimpleIoProcessorPool(Class<? extends IoProcessor<T>> processorType,
> + int size) {
> this(processorType, null, size);
> }
>
> - public SimpleIoProcessorPool(Class<? extends IoProcessor<T>> processorType, Executor executor) {
> + public SimpleIoProcessorPool(Class<? extends IoProcessor<T>> processorType,
> + Executor executor) {
> this(processorType, executor, DEFAULT_SIZE);
> }
> -
> +
> @SuppressWarnings("unchecked")
> - public SimpleIoProcessorPool(Class<? extends IoProcessor<T>> processorType, Executor executor, int size) {
> + public SimpleIoProcessorPool(Class<? extends IoProcessor<T>> processorType,
> + Executor executor, int size) {
> if (processorType == null) {
> throw new NullPointerException("processorType");
> }
> if (size <= 0) {
> - throw new IllegalArgumentException(
> - "size: " + size + " (expected: positive integer)");
> + throw new IllegalArgumentException("size: " + size
> + + " (expected: positive integer)");
> }
> -
> +
> if (executor == null) {
> this.executor = executor = Executors.newCachedThreadPool();
> this.createdExecutor = true;
> @@ -118,56 +132,72 @@
> this.executor = executor;
> this.createdExecutor = false;
> }
> -
> +
> pool = new IoProcessor[size];
> -
> +
> boolean success = false;
> + Constructor<? extends IoProcessor<T>> processorConstructor = null;
> + boolean usesExecutorArg = true;
> +
> try {
> - for (int i = 0; i < pool.length; i ++) {
> - IoProcessor<T> processor = null;
> -
> - // Try to create a new processor with a proper constructor.
> + // We create at least one processor
> + try {
> try {
> - try {
> - processor = processorType.getConstructor(ExecutorService.class).newInstance(executor);
> - } catch (NoSuchMethodException e) {
> - // To the next step...
> - }
> -
> - if (processor == null) {
> - try {
> - processor = processorType.getConstructor(Executor.class).newInstance(executor);
> - } catch (NoSuchMethodException e) {
> - // To the next step...
> - }
> - }
> -
> - if (processor == null) {
> - try {
> - processor = processorType.getConstructor().newInstance();
> - } catch (NoSuchMethodException e) {
> - // To the next step...
> - }
> - }
> - } catch (RuntimeException e) {
> - throw e;
> - } catch (Exception e) {
> - throw new RuntimeIoException(
> - "Failed to create a new instance of " + processorType.getName(), e);
> + processorConstructor = processorType
> + .getConstructor(ExecutorService.class);
> + pool[0] = processorConstructor.newInstance(executor);
> + } catch (NoSuchMethodException e) {
> + // To the next step...
> }
> -
> +
> + try {
> + processorConstructor = processorType
> + .getConstructor(Executor.class);
> + pool[0] = processorConstructor.newInstance(executor);
> + } catch (NoSuchMethodException e) {
> + // To the next step...
> + }
> +
> + try {
> + processorConstructor = processorType.getConstructor();
> + usesExecutorArg = false;
> + pool[0] = processorConstructor.newInstance();
> + } catch (NoSuchMethodException e) {
> + // To the next step...
> + }
> + } catch (RuntimeException e) {
> + throw e;
> + } catch (Exception e) {
> + throw new RuntimeIoException(
> + "Failed to create a new instance of "
> + + processorType.getName(), e);
> + }
> +
> + if (processorConstructor == null) {
> // Raise an exception if no proper constructor is found.
> - if (processor == null) {
> - throw new IllegalArgumentException(
> - String.valueOf(processorType) + " must have a public constructor " +
> - "with one " + ExecutorService.class.getSimpleName() + " parameter, " +
> - "a public constructor with one " + Executor.class.getSimpleName() +
> - " parameter or a public default constructor.");
> + throw new IllegalArgumentException(String
> + .valueOf(processorType)
> + + " must have a public constructor "
> + + "with one "
> + + ExecutorService.class.getSimpleName()
> + + " parameter, "
> + + "a public constructor with one "
> + + Executor.class.getSimpleName()
> + + " parameter or a public default constructor.");
> + }
> +
> + // Constructor found now use it for all subsequent instantiations
> + for (int i = 1; i < pool.length; i++) {
> + try {
> + if (usesExecutorArg) {
> + pool[i] = processorConstructor.newInstance(executor);
> + } else {
> + pool[i] = processorConstructor.newInstance();
> + }
> + } catch (Exception e) {
> + // Won't happen because it has been done previously
> }
> -
> - pool[i] = processor;
> }
> -
> success = true;
> } finally {
> if (!success) {
> @@ -175,7 +205,7 @@
> }
> }
> }
> -
> +
> public final void add(T session) {
> getProcessor(session).add(session);
> }
> @@ -191,7 +221,7 @@
> public final void updateTrafficControl(T session) {
> getProcessor(session).updateTrafficControl(session);
> }
> -
> +
> public boolean isDisposed() {
> return disposed;
> }
> @@ -208,7 +238,7 @@
> synchronized (disposalLock) {
> if (!disposing) {
> disposing = true;
> - for (int i = pool.length - 1; i >= 0; i --) {
> + for (int i = pool.length - 1; i >= 0; i--) {
> if (pool[i] == null || pool[i].isDisposing()) {
> continue;
> }
> @@ -216,15 +246,14 @@
> try {
> pool[i].dispose();
> } catch (Exception e) {
> - LOGGER.warn(
> - "Failed to dispose a " +
> - pool[i].getClass().getSimpleName() +
> - " at index " + i + ".", e);
> + LOGGER.warn("Failed to dispose a "
> + + pool[i].getClass().getSimpleName()
> + + " at index " + i + ".", e);
> } finally {
> pool[i] = null;
> }
> }
> -
> +
> if (createdExecutor) {
> ((ExecutorService) executor).shutdown();
> }
> @@ -233,30 +262,28 @@
>
> disposed = true;
> }
> -
> +
> @SuppressWarnings("unchecked")
> private IoProcessor<T> getProcessor(T session) {
> IoProcessor<T> p = (IoProcessor<T>) session.getAttribute(PROCESSOR);
> if (p == null) {
> p = nextProcessor();
> - IoProcessor<T> oldp =
> - (IoProcessor<T>) session.setAttributeIfAbsent(PROCESSOR, p);
> + IoProcessor<T> oldp = (IoProcessor<T>) session
> + .setAttributeIfAbsent(PROCESSOR, p);
> if (oldp != null) {
> p = oldp;
> }
> }
> -
> +
> return p;
> }
>
> private IoProcessor<T> nextProcessor() {
> - checkDisposal();
> - return pool[Math.abs(processorDistributor.getAndIncrement()) % pool.length];
> - }
> -
> - private void checkDisposal() {
> if (disposed) {
> - throw new IllegalStateException("A disposed processor cannot be accessed.");
> + throw new IllegalStateException(
> + "A disposed processor cannot be accessed.");
> }
> + return pool[Math.abs(processorDistributor.getAndIncrement())
> + % pool.length];
> }
> }
>
>
>
Re : svn commit: r789164 - /mina/trunk/core/src/main/java/org/apache/mina/core/service/SimpleIoProcessorPool.java
Posted by Edouard De Oliveira <do...@yahoo.fr>.
Yes
in previous version each constructor type was tested again and again in the loop
The new code is slightly more complex but it is based on the fact that we need at least one processor
so we discover which constructor to use and then we loop and use the previously determined constructor.
Cordialement, Regards,
-Edouard De Oliveira-
Blog: http://tedorgwp.free.fr
WebSite: http://tedorg.free.fr/en/main.php
----- Message d'origine ----
De : Julien Vermillard <jv...@apache.org>
À : dev@mina.apache.org
Envoyé le : Lundi, 29 Juin 2009, 11h13mn 24s
Objet : Re: svn commit: r789164 - /mina/trunk/core/src/main/java/org/apache/mina/core/service/SimpleIoProcessorPool.java
Hi,
Interesting optimization. The idea is to make SimpleIoPPool
instantiation faster ?
Julien
On Mon, Jun 29, 2009 at 12:30 AM, <ed...@apache.org> wrote:
> Author: edeoliveira
> Date: Sun Jun 28 22:30:02 2009
> New Revision: 789164
>
> URL: http://svn.apache.org/viewvc?rev=789164&view=rev
> Log:
> Fix to do less reflection ops whilie initialising the pool
>
> Modified:
> mina/trunk/core/src/main/java/org/apache/mina/core/service/SimpleIoProcessorPool.java
>
> Modified: mina/trunk/core/src/main/java/org/apache/mina/core/service/SimpleIoProcessorPool.java
> URL: http://svn.apache.org/viewvc/mina/trunk/core/src/main/java/org/apache/mina/core/service/SimpleIoProcessorPool.java?rev=789164&r1=789163&r2=789164&view=diff
> ==============================================================================
> --- mina/trunk/core/src/main/java/org/apache/mina/core/service/SimpleIoProcessorPool.java (original)
> +++ mina/trunk/core/src/main/java/org/apache/mina/core/service/SimpleIoProcessorPool.java Sun Jun 28 22:30:02 2009
> @@ -19,6 +19,7 @@
> */
> package org.apache.mina.core.service;
>
> +import java.lang.reflect.Constructor;
> import java.util.concurrent.Executor;
> import java.util.concurrent.ExecutorService;
> import java.util.concurrent.Executors;
> @@ -73,44 +74,57 @@
> * @param <T> the type of the {@link IoSession} to be managed by the specified
> * {@link IoProcessor}.
> */
> -public class SimpleIoProcessorPool<T extends AbstractIoSession> implements IoProcessor<T> {
> -
> - private static final int DEFAULT_SIZE = Runtime.getRuntime().availableProcessors() + 1;
> - private static final AttributeKey PROCESSOR = new AttributeKey(SimpleIoProcessorPool.class, "processor");
> -
> - private final static Logger LOGGER = LoggerFactory.getLogger(SimpleIoProcessorPool.class);
> +public class SimpleIoProcessorPool<T extends AbstractIoSession> implements
> + IoProcessor<T> {
> +
> + private static final int DEFAULT_SIZE = Runtime.getRuntime()
> + .availableProcessors() + 1;
> +
> + private static final AttributeKey PROCESSOR = new AttributeKey(
> + SimpleIoProcessorPool.class, "processor");
> +
> + private final static Logger LOGGER = LoggerFactory
> + .getLogger(SimpleIoProcessorPool.class);
>
> private final IoProcessor<T>[] pool;
> +
> private final AtomicInteger processorDistributor = new AtomicInteger();
> +
> private final Executor executor;
> +
> private final boolean createdExecutor;
> -
> +
> private final Object disposalLock = new Object();
> +
> private volatile boolean disposing;
> +
> private volatile boolean disposed;
> -
> +
> public SimpleIoProcessorPool(Class<? extends IoProcessor<T>> processorType) {
> this(processorType, null, DEFAULT_SIZE);
> }
> -
> - public SimpleIoProcessorPool(Class<? extends IoProcessor<T>> processorType, int size) {
> +
> + public SimpleIoProcessorPool(Class<? extends IoProcessor<T>> processorType,
> + int size) {
> this(processorType, null, size);
> }
>
> - public SimpleIoProcessorPool(Class<? extends IoProcessor<T>> processorType, Executor executor) {
> + public SimpleIoProcessorPool(Class<? extends IoProcessor<T>> processorType,
> + Executor executor) {
> this(processorType, executor, DEFAULT_SIZE);
> }
> -
> +
> @SuppressWarnings("unchecked")
> - public SimpleIoProcessorPool(Class<? extends IoProcessor<T>> processorType, Executor executor, int size) {
> + public SimpleIoProcessorPool(Class<? extends IoProcessor<T>> processorType,
> + Executor executor, int size) {
> if (processorType == null) {
> throw new NullPointerException("processorType");
> }
> if (size <= 0) {
> - throw new IllegalArgumentException(
> - "size: " + size + " (expected: positive integer)");
> + throw new IllegalArgumentException("size: " + size
> + + " (expected: positive integer)");
> }
> -
> +
> if (executor == null) {
> this.executor = executor = Executors.newCachedThreadPool();
> this.createdExecutor = true;
> @@ -118,56 +132,72 @@
> this.executor = executor;
> this.createdExecutor = false;
> }
> -
> +
> pool = new IoProcessor[size];
> -
> +
> boolean success = false;
> + Constructor<? extends IoProcessor<T>> processorConstructor = null;
> + boolean usesExecutorArg = true;
> +
> try {
> - for (int i = 0; i < pool.length; i ++) {
> - IoProcessor<T> processor = null;
> -
> - // Try to create a new processor with a proper constructor.
> + // We create at least one processor
> + try {
> try {
> - try {
> - processor = processorType.getConstructor(ExecutorService.class).newInstance(executor);
> - } catch (NoSuchMethodException e) {
> - // To the next step...
> - }
> -
> - if (processor == null) {
> - try {
> - processor = processorType.getConstructor(Executor.class).newInstance(executor);
> - } catch (NoSuchMethodException e) {
> - // To the next step...
> - }
> - }
> -
> - if (processor == null) {
> - try {
> - processor = processorType.getConstructor().newInstance();
> - } catch (NoSuchMethodException e) {
> - // To the next step...
> - }
> - }
> - } catch (RuntimeException e) {
> - throw e;
> - } catch (Exception e) {
> - throw new RuntimeIoException(
> - "Failed to create a new instance of " + processorType.getName(), e);
> + processorConstructor = processorType
> + .getConstructor(ExecutorService.class);
> + pool[0] = processorConstructor.newInstance(executor);
> + } catch (NoSuchMethodException e) {
> + // To the next step...
> }
> -
> +
> + try {
> + processorConstructor = processorType
> + .getConstructor(Executor.class);
> + pool[0] = processorConstructor.newInstance(executor);
> + } catch (NoSuchMethodException e) {
> + // To the next step...
> + }
> +
> + try {
> + processorConstructor = processorType.getConstructor();
> + usesExecutorArg = false;
> + pool[0] = processorConstructor.newInstance();
> + } catch (NoSuchMethodException e) {
> + // To the next step...
> + }
> + } catch (RuntimeException e) {
> + throw e;
> + } catch (Exception e) {
> + throw new RuntimeIoException(
> + "Failed to create a new instance of "
> + + processorType.getName(), e);
> + }
> +
> + if (processorConstructor == null) {
> // Raise an exception if no proper constructor is found.
> - if (processor == null) {
> - throw new IllegalArgumentException(
> - String.valueOf(processorType) + " must have a public constructor " +
> - "with one " + ExecutorService.class.getSimpleName() + " parameter, " +
> - "a public constructor with one " + Executor.class.getSimpleName() +
> - " parameter or a public default constructor.");
> + throw new IllegalArgumentException(String
> + .valueOf(processorType)
> + + " must have a public constructor "
> + + "with one "
> + + ExecutorService.class.getSimpleName()
> + + " parameter, "
> + + "a public constructor with one "
> + + Executor.class.getSimpleName()
> + + " parameter or a public default constructor.");
> + }
> +
> + // Constructor found now use it for all subsequent instantiations
> + for (int i = 1; i < pool.length; i++) {
> + try {
> + if (usesExecutorArg) {
> + pool[i] = processorConstructor.newInstance(executor);
> + } else {
> + pool[i] = processorConstructor.newInstance();
> + }
> + } catch (Exception e) {
> + // Won't happen because it has been done previously
> }
> -
> - pool[i] = processor;
> }
> -
> success = true;
> } finally {
> if (!success) {
> @@ -175,7 +205,7 @@
> }
> }
> }
> -
> +
> public final void add(T session) {
> getProcessor(session).add(session);
> }
> @@ -191,7 +221,7 @@
> public final void updateTrafficControl(T session) {
> getProcessor(session).updateTrafficControl(session);
> }
> -
> +
> public boolean isDisposed() {
> return disposed;
> }
> @@ -208,7 +238,7 @@
> synchronized (disposalLock) {
> if (!disposing) {
> disposing = true;
> - for (int i = pool.length - 1; i >= 0; i --) {
> + for (int i = pool.length - 1; i >= 0; i--) {
> if (pool[i] == null || pool[i].isDisposing()) {
> continue;
> }
> @@ -216,15 +246,14 @@
> try {
> pool[i].dispose();
> } catch (Exception e) {
> - LOGGER.warn(
> - "Failed to dispose a " +
> - pool[i].getClass().getSimpleName() +
> - " at index " + i + ".", e);
> + LOGGER.warn("Failed to dispose a "
> + + pool[i].getClass().getSimpleName()
> + + " at index " + i + ".", e);
> } finally {
> pool[i] = null;
> }
> }
> -
> +
> if (createdExecutor) {
> ((ExecutorService) executor).shutdown();
> }
> @@ -233,30 +262,28 @@
>
> disposed = true;
> }
> -
> +
> @SuppressWarnings("unchecked")
> private IoProcessor<T> getProcessor(T session) {
> IoProcessor<T> p = (IoProcessor<T>) session.getAttribute(PROCESSOR);
> if (p == null) {
> p = nextProcessor();
> - IoProcessor<T> oldp =
> - (IoProcessor<T>) session.setAttributeIfAbsent(PROCESSOR, p);
> + IoProcessor<T> oldp = (IoProcessor<T>) session
> + .setAttributeIfAbsent(PROCESSOR, p);
> if (oldp != null) {
> p = oldp;
> }
> }
> -
> +
> return p;
> }
>
> private IoProcessor<T> nextProcessor() {
> - checkDisposal();
> - return pool[Math.abs(processorDistributor.getAndIncrement()) % pool.length];
> - }
> -
> - private void checkDisposal() {
> if (disposed) {
> - throw new IllegalStateException("A disposed processor cannot be accessed.");
> + throw new IllegalStateException(
> + "A disposed processor cannot be accessed.");
> }
> + return pool[Math.abs(processorDistributor.getAndIncrement())
> + % pool.length];
> }
> }
>
>
>