Use shaded dependency on JCTools instead of copy and paste

Motivation: JCTools supports both non-unsafe, unsafe versions of queues and JDK6 which allows us to shade the library in netty-common allowing it to stay "zero dependency". Modifications: - Remove copy paste JCTools code and shade the library (dependencies that are shaded should be removed from the <dependencies> section of the generated POM). - Remove usage of OneTimeTask and remove it all together. Result: Less code to maintain and easier to update JCTools and less GC pressure as the queue implementation nt creates so much garbage
luffyke · Jun 10, 2016 · c3abb91 · c3abb91
1 parent 398efb1
commit c3abb91
Show file tree

Hide file tree

Showing 64 changed files with 228 additions and 2,498 deletions.
diff --git a/.gitignore b/.gitignore
@@ -30,3 +30,4 @@
 # JVM crash logs
 hs_err_pid*.log
 
+dependency-reduced-pom.xml
diff --git a/...-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker.java b/...-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker.java
@@ -34,7 +34,6 @@
 import io.netty.handler.codec.http.HttpResponseDecoder;
 import io.netty.util.ReferenceCountUtil;
 import io.netty.util.internal.EmptyArrays;
-import io.netty.util.internal.OneTimeTask;
 import io.netty.util.internal.StringUtil;
 
 import java.net.URI;
@@ -273,7 +272,7 @@ public final void finishHandshake(Channel channel, FullHttpResponse response) {
             // Delay the removal of the decoder so the user can setup the pipeline if needed to handle
             // WebSocketFrame messages.
             // See https://github.com/netty/netty/issues/4533
-            channel.eventLoop().execute(new OneTimeTask() {
+            channel.eventLoop().execute(new Runnable() {
                 @Override
                 public void run() {
                     p.remove(codec);
@@ -290,7 +289,7 @@ public void run() {
             // Delay the removal of the decoder so the user can setup the pipeline if needed to handle
             // WebSocketFrame messages.
             // See https://github.com/netty/netty/issues/4533
-            channel.eventLoop().execute(new OneTimeTask() {
+            channel.eventLoop().execute(new Runnable() {
                 @Override
                 public void run() {
                     p.remove(context.handler());

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2StreamChannel.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2StreamChannel.java
@@ -29,7 +29,6 @@
 import io.netty.util.ReferenceCountUtil;
 import io.netty.util.concurrent.EventExecutor;
 import io.netty.util.internal.EmptyArrays;
-import io.netty.util.internal.OneTimeTask;
 
 import java.net.SocketAddress;
 import java.nio.channels.ClosedChannelException;
@@ -201,7 +200,7 @@ protected final void doWrite(ChannelOutboundBuffer in) throws Exception {
                 in.remove();
             }
 
-            preferredExecutor.execute(new OneTimeTask() {
+            preferredExecutor.execute(new Runnable() {
                 @Override
                 public void run() {
                     for (Object msg : msgsCopy) {
@@ -248,7 +247,7 @@ protected void fireChildRead(final Object msg) {
         if (eventLoop().inEventLoop()) {
             fireChildRead0(msg);
         } else {
-            eventLoop().execute(new OneTimeTask() {
+            eventLoop().execute(new Runnable() {
                 @Override
                 public void run() {
                     fireChildRead0(msg);

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java
@@ -25,7 +25,6 @@
 import io.netty.handler.codec.http2.Http2Exception.CompositeStreamException;
 import io.netty.handler.codec.http2.Http2Exception.StreamException;
 import io.netty.util.concurrent.ScheduledFuture;
-import io.netty.util.internal.OneTimeTask;
 import io.netty.util.internal.UnstableApi;
 import io.netty.util.internal.logging.InternalLogger;
 import io.netty.util.internal.logging.InternalLoggerFactory;
@@ -766,7 +765,7 @@ private static final class ClosingChannelFutureListener implements ChannelFuture
                                      long timeout, TimeUnit unit) {
             this.ctx = ctx;
             this.promise = promise;
-            timeoutTask = ctx.executor().schedule(new OneTimeTask() {
+            timeoutTask = ctx.executor().schedule(new Runnable() {
                 @Override
                 public void run() {
                     ctx.close(promise);

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodec.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodec.java
@@ -31,7 +31,6 @@
 import io.netty.handler.codec.http.HttpServerUpgradeHandler.UpgradeEvent;
 import io.netty.util.ReferenceCountUtil;
 import io.netty.util.concurrent.EventExecutor;
-import io.netty.util.internal.OneTimeTask;
 import io.netty.util.internal.UnstableApi;
 
 import java.util.ArrayList;
@@ -212,7 +211,7 @@ void writeFromStreamChannel(final Object msg, final boolean flush) {
             writeFromStreamChannel0(msg, flush, promise);
         } else {
             try {
-                executor.execute(new OneTimeTask() {
+                executor.execute(new Runnable() {
                     @Override
                     public void run() {
                         writeFromStreamChannel0(msg, flush, promise);
@@ -344,7 +343,7 @@ public void onStreamClosed(Http2Stream stream) {
                 if (eventLoop.inEventLoop()) {
                     onStreamClosed0(streamInfo);
                 } else {
-                    eventLoop.execute(new OneTimeTask() {
+                    eventLoop.execute(new Runnable() {
                         @Override
                         public void run() {
                             onStreamClosed0(streamInfo);
@@ -381,7 +380,7 @@ public boolean visit(Http2Stream stream) {
                 if (executor.inEventLoop()) {
                     exceptionCaught(ctx, t);
                 } else {
-                    executor.execute(new OneTimeTask() {
+                    executor.execute(new Runnable() {
                         @Override
                         public void run() {
                             exceptionCaught(ctx, t);
@@ -518,7 +517,7 @@ protected void bytesConsumed(final int bytes) {
             if (executor.inEventLoop()) {
                 bytesConsumed0(bytes);
             } else {
-                executor.execute(new OneTimeTask() {
+                executor.execute(new Runnable() {
                     @Override
                     public void run() {
                         bytesConsumed0(bytes);

diff --git a/codec/src/main/java/io/netty/handler/codec/compression/JZlibEncoder.java b/codec/src/main/java/io/netty/handler/codec/compression/JZlibEncoder.java
@@ -26,7 +26,6 @@
 import io.netty.channel.ChannelPromiseNotifier;
 import io.netty.util.concurrent.EventExecutor;
 import io.netty.util.internal.EmptyArrays;
-import io.netty.util.internal.OneTimeTask;
 
 import java.util.concurrent.TimeUnit;
 
@@ -253,7 +252,7 @@ public ChannelFuture close(final ChannelPromise promise) {
             return finishEncode(ctx, promise);
         } else {
             final ChannelPromise p = ctx.newPromise();
-            executor.execute(new OneTimeTask() {
+            executor.execute(new Runnable() {
                 @Override
                 public void run() {
                     ChannelFuture f = finishEncode(ctx(), p);
@@ -352,7 +351,7 @@ public void operationComplete(ChannelFuture f) throws Exception {
 
         if (!f.isDone()) {
             // Ensure the channel is closed even if the write operation completes in time.
-            ctx.executor().schedule(new OneTimeTask() {
+            ctx.executor().schedule(new Runnable() {
                 @Override
                 public void run() {
                     ctx.close(promise);

diff --git a/codec/src/main/java/io/netty/handler/codec/compression/JdkZlibEncoder.java b/codec/src/main/java/io/netty/handler/codec/compression/JdkZlibEncoder.java
@@ -22,7 +22,6 @@
 import io.netty.channel.ChannelPromise;
 import io.netty.channel.ChannelPromiseNotifier;
 import io.netty.util.concurrent.EventExecutor;
-import io.netty.util.internal.OneTimeTask;
 
 import java.util.concurrent.TimeUnit;
 import java.util.zip.CRC32;
@@ -164,7 +163,7 @@ public ChannelFuture close(final ChannelPromise promise) {
             return finishEncode(ctx, promise);
         } else {
             final ChannelPromise p = ctx.newPromise();
-            executor.execute(new OneTimeTask() {
+            executor.execute(new Runnable() {
                 @Override
                 public void run() {
                     ChannelFuture f = finishEncode(ctx(), p);
@@ -260,7 +259,7 @@ public void operationComplete(ChannelFuture f) throws Exception {
 
         if (!f.isDone()) {
             // Ensure the channel is closed even if the write operation completes in time.
-            ctx.executor().schedule(new OneTimeTask() {
+            ctx.executor().schedule(new Runnable() {
                 @Override
                 public void run() {
                     ctx.close(promise);

diff --git a/common/pom.xml b/common/pom.xml
@@ -44,6 +44,12 @@
       <scope>compile</scope> <!-- override the 'test' scope defined at parent pom.xml -->
       <optional>true</optional>
     </dependency>
+    <dependency>
+      <groupId>org.jctools</groupId>
+      <artifactId>jctools-core</artifactId>
+      <!-- Mark as optional as otherwise the bundle plugin will add strict import statements and so fail in OSGI containers-->
+      <optional>true</optional>
+    </dependency>
 
     <!-- Logging frameworks - completely optional -->
     <dependency>
@@ -75,6 +81,31 @@
 
   <build>
     <plugins>
+      <plugin>
+        <artifactId>maven-shade-plugin</artifactId>
+        <executions>
+          <execution>
+            <phase>package</phase>
+            <goals>
+              <goal>shade</goal>
+            </goals>
+            <configuration>
+              <artifactSet>
+                <includes>
+                  <include>org.jctools</include>
+                </includes>
+              </artifactSet>
+              <relocations>
+                <relocation>
+                  <pattern>org.jctools.</pattern>
+                  <shadedPattern>io.netty.util.internal.shaded.org.jctools.</shadedPattern>
+                </relocation>
+              </relocations>
+              <minimizeJar>true</minimizeJar>
+            </configuration>
+          </execution>
+        </executions>
+      </plugin>
       <!-- Add generated collection sources. -->
       <plugin>
         <groupId>org.codehaus.mojo</groupId>
@@ -151,6 +182,7 @@
             </goals>
             <configuration>
               <instructions>
+                <!-- Exclude org.jctools.* as we shade it -->
                 <!-- NativeLibraryLoader can be used to manually load native libraries from other bundles that this bundle does not depend on,
                 hence use DynamicImport-Package instruction to ensure the loading is successful -->
                 <DynamicImport-Package>*</DynamicImport-Package>

diff --git a/common/src/main/java/io/netty/util/HashedWheelTimer.java b/common/src/main/java/io/netty/util/HashedWheelTimer.java
@@ -15,7 +15,6 @@
  */
 package io.netty.util;
 
-import io.netty.util.internal.MpscLinkedQueueNode;
 import io.netty.util.internal.PlatformDependent;
 import io.netty.util.internal.StringUtil;
 import io.netty.util.internal.logging.InternalLogger;
@@ -106,7 +105,7 @@ public class HashedWheelTimer implements Timer {
     private final int mask;
     private final CountDownLatch startTimeInitialized = new CountDownLatch(1);
     private final Queue<HashedWheelTimeout> timeouts = PlatformDependent.newMpscQueue();
-    private final Queue<Runnable> cancelledTimeouts = PlatformDependent.newMpscQueue();
+    private final Queue<HashedWheelTimeout> cancelledTimeouts = PlatformDependent.newMpscQueue();
 
     private volatile long startTime;
 
@@ -412,13 +411,13 @@ private void transferTimeoutsToBuckets() {
 
         private void processCancelledTasks() {
             for (;;) {
-                Runnable task = cancelledTimeouts.poll();
-                if (task == null) {
+                HashedWheelTimeout timeout = cancelledTimeouts.poll();
+                if (timeout == null) {
                     // all processed
                     break;
                 }
                 try {
-                    task.run();
+                    timeout.remove();
                 } catch (Throwable t) {
                     if (logger.isWarnEnabled()) {
                         logger.warn("An exception was thrown while process a cancellation task", t);
@@ -472,8 +471,7 @@ public Set<Timeout> unprocessedTimeouts() {
         }
     }
 
-    private static final class HashedWheelTimeout extends MpscLinkedQueueNode<Timeout>
-            implements Timeout {
+    private static final class HashedWheelTimeout implements Timeout {
 
         private static final int ST_INIT = 0;
         private static final int ST_CANCELLED = 1;
@@ -530,25 +528,20 @@ public boolean cancel() {
             if (!compareAndSetState(ST_INIT, ST_CANCELLED)) {
                 return false;
             }
-            // If a task should be canceled we create a new Runnable for this to another queue which will
-            // be processed on each tick. So this means that we will have a GC latency of max. 1 tick duration
-            // which is good enough. This way we can make again use of our MpscLinkedQueue and so minimize the
-            // locking / overhead as much as possible.
-            //
-            // It is important that we not just add the HashedWheelTimeout itself again as it extends
-            // MpscLinkedQueueNode and so may still be used as tombstone.
-            timer.cancelledTimeouts.add(new Runnable() {
-                @Override
-                public void run() {
-                    HashedWheelBucket bucket = HashedWheelTimeout.this.bucket;
-                    if (bucket != null) {
-                        bucket.remove(HashedWheelTimeout.this);
-                    }
-                }
-            });
+            // If a task should be canceled we put this to another queue which will be processed on each tick.
+            // So this means that we will have a GC latency of max. 1 tick duration which is good enough. This way
+            // we can make again use of our MpscLinkedQueue and so minimize the locking / overhead as much as possible.
+            timer.cancelledTimeouts.add(this);
             return true;
         }
 
+        void remove() {
+            HashedWheelBucket bucket = this.bucket;
+            if (bucket != null) {
+                bucket.remove(this);
+            }
+        }
+
         public boolean compareAndSetState(int expected, int state) {
             return STATE_UPDATER.compareAndSet(this, expected, state);
         }
@@ -567,11 +560,6 @@ public boolean isExpired() {
             return state() == ST_EXPIRED;
         }
 
-        @Override
-        public HashedWheelTimeout value() {
-            return this;
-        }
-
         public void expire() {
             if (!compareAndSetState(ST_INIT, ST_EXPIRED)) {
                 return;

diff --git a/common/src/main/java/io/netty/util/ThreadDeathWatcher.java b/common/src/main/java/io/netty/util/ThreadDeathWatcher.java
@@ -17,7 +17,6 @@
 package io.netty.util;
 
 import io.netty.util.concurrent.DefaultThreadFactory;
-import io.netty.util.internal.MpscLinkedQueueNode;
 import io.netty.util.internal.PlatformDependent;
 import io.netty.util.internal.StringUtil;
 import io.netty.util.internal.SystemPropertyUtil;
@@ -213,7 +212,7 @@ private void notifyWatchees() {
         }
     }
 
-    private static final class Entry extends MpscLinkedQueueNode<Entry> {
+    private static final class Entry {
         final Thread thread;
         final Runnable task;
         final boolean isWatch;
@@ -224,11 +223,6 @@ private static final class Entry extends MpscLinkedQueueNode<Entry> {
             this.isWatch = isWatch;
         }
 
-        @Override
-        public Entry value() {
-            return this;
-        }
-
         @Override
         public int hashCode() {
             return thread.hashCode() ^ task.hashCode();

diff --git a/common/src/main/java/io/netty/util/concurrent/AbstractScheduledEventExecutor.java b/common/src/main/java/io/netty/util/concurrent/AbstractScheduledEventExecutor.java
@@ -16,7 +16,6 @@
 package io.netty.util.concurrent;
 
 import io.netty.util.internal.ObjectUtil;
-import io.netty.util.internal.OneTimeTask;
 
 import java.util.PriorityQueue;
 import java.util.Queue;
@@ -195,7 +194,7 @@ <V> ScheduledFuture<V> schedule(final ScheduledFutureTask<V> task) {
         if (inEventLoop()) {
             scheduledTaskQueue().add(task);
         } else {
-            execute(new OneTimeTask() {
+            execute(new Runnable() {
                 @Override
                 public void run() {
                     scheduledTaskQueue().add(task);
@@ -210,7 +209,7 @@ final void removeScheduled(final ScheduledFutureTask<?> task) {
         if (inEventLoop()) {
             scheduledTaskQueue().remove(task);
         } else {
-            execute(new OneTimeTask() {
+            execute(new Runnable() {
                 @Override
                 public void run() {
                     removeScheduled(task);
Original file line number	Diff line number	Diff line change
Expand Up		@@ -30,3 +30,4 @@
		# JVM crash logs
		hs_err_pid*.log

		dependency-reduced-pom.xml