Repository: netty/netty
Issue: MultithreadEventExecutorGroup.next() should use bitwise operations when possible
Labels: improvement

At the moment MultithreadEventExecutorGroup.next() uses modulo operation to calculate the index of the next EventExecutor to use. This is slow compared to bitwise operations which can be used if the array is power of two. 

So if the array is power of two we should better use bitwise otherwise use modulo for best performance.


Issue comments:
I was interested in the performance of this, so I ran a little JMH benchmark (disclaimer: I don't know the first thing about benchmarking) on my laptop (too lazy to connect to Azure :)). Well, this is what I got https://gist.github.com/bucjac/9401713 and here is the code https://gist.github.com/bucjac/9401700.

I am not sure if this would bring any performance gains?

That's interesting. I just ran it on a Ubuntu VM on Windows Azure (AMD Opteron) and the bitwise optimization resulted in a 2.13 speedup, while for non-poweroftwos it was only 4% slower. https://gist.github.com/bucjac/9450506

It only will work with power of two

> Am 09.03.2014 um 17:48 schrieb Jakob Buchgraber notifications@github.com:
> 
> That's interesting. I just ran it on a Ubuntu VM on Windows Azure (AMD Opteron) and the bitwise optimization resulted in a 2.13 speedup, while for non-poweroftwos it was only 4%. https://gist.github.com/bucjac/9450506
> 
> —
> Reply to this email directly or view it on GitHub.

Yeah, but on my laptop it didn't run any faster for power of twos, while on azure it was twice as fast?

Fixed by #2299


Pull request review comments:

Patch that resolved the issue:
diff --git a/common/src/main/java/io/netty/util/concurrent/MultithreadEventExecutorGroup.java b/common/src/main/java/io/netty/util/concurrent/MultithreadEventExecutorGroup.java
index 38efd4912ed..449598442a3 100644
--- a/common/src/main/java/io/netty/util/concurrent/MultithreadEventExecutorGroup.java
+++ b/common/src/main/java/io/netty/util/concurrent/MultithreadEventExecutorGroup.java
@@ -33,6 +33,7 @@ public abstract class MultithreadEventExecutorGroup extends AbstractEventExecuto
     private final AtomicInteger childIndex = new AtomicInteger();
     private final AtomicInteger terminatedChildren = new AtomicInteger();
     private final Promise<?> terminationFuture = new DefaultPromise(GlobalEventExecutor.INSTANCE);
+    private final EventExecutorChooser chooser;
 
     /**
      * Create a new instance.
@@ -51,6 +52,12 @@ protected MultithreadEventExecutorGroup(int nThreads, ThreadFactory threadFactor
         }
 
         children = new SingleThreadEventExecutor[nThreads];
+        if (isPowerOfTwo(children.length)) {
+            chooser = new PowerOfTwoEventExecutorChooser();
+        } else {
+            chooser = new GenericEventExecutorChooser();
+        }
+
         for (int i = 0; i < nThreads; i ++) {
             boolean success = false;
             try {
@@ -100,7 +107,7 @@ protected ThreadFactory newDefaultThreadFactory() {
 
     @Override
     public EventExecutor next() {
-        return children[Math.abs(childIndex.getAndIncrement() % children.length)];
+        return chooser.next();
     }
 
     @Override
@@ -201,4 +208,26 @@ public boolean awaitTermination(long timeout, TimeUnit unit)
         }
         return isTerminated();
     }
+
+    private static boolean isPowerOfTwo(int val) {
+        return (val & -val) == val;
+    }
+
+    private interface EventExecutorChooser {
+        EventExecutor next();
+    }
+
+    private final class PowerOfTwoEventExecutorChooser implements EventExecutorChooser {
+        @Override
+        public EventExecutor next() {
+            return children[childIndex.getAndIncrement() & children.length - 1];
+        }
+    }
+
+    private final class GenericEventExecutorChooser implements EventExecutorChooser {
+        @Override
+        public EventExecutor next() {
+            return children[Math.abs(childIndex.getAndIncrement() % children.length)];
+        }
+    }
 }


Test patch: