From 19b4adf79c2d82d14e645be0e5f3eca30067bf1b Mon Sep 17 00:00:00 2001 From: Nick Hill Date: Thu, 17 Oct 2019 07:01:53 -0700 Subject: [PATCH] Avoid wrapping scheduled Runnables in Callable adapter (#9666) Motivation Currently when future tasks are scheduled via schedule(Runnable, ...) methods, the supplied Runnable is wrapped in a newly allocated Callable adapter prior to being wrapped in a ScheduledFutureTask. This can be avoided which saves an object allocation per scheduled task. Modifications Change the Callable task field of ScheduledFutureTask to be of type Object so that it can hold/run Runnables directly in addition to Callables. An "adapter" is still used in the case a Runnable is scheduled with an explicit constant non-null completion value, assumed to be rare. Result Less garbage --- .../AbstractScheduledEventExecutor.java | 14 +++--- .../io/netty/util/concurrent/PromiseTask.java | 46 +++++++++++-------- .../util/concurrent/ScheduledFutureTask.java | 39 ++++++++++------ .../UnorderedThreadPoolEventExecutor.java | 4 +- 4 files changed, 61 insertions(+), 42 deletions(-) diff --git a/common/src/main/java/io/netty/util/concurrent/AbstractScheduledEventExecutor.java b/common/src/main/java/io/netty/util/concurrent/AbstractScheduledEventExecutor.java index a44ae521dd..33c27f2d3c 100644 --- a/common/src/main/java/io/netty/util/concurrent/AbstractScheduledEventExecutor.java +++ b/common/src/main/java/io/netty/util/concurrent/AbstractScheduledEventExecutor.java @@ -19,10 +19,11 @@ import io.netty.util.internal.DefaultPriorityQueue; import io.netty.util.internal.ObjectUtil; import io.netty.util.internal.PriorityQueue; +import static io.netty.util.concurrent.ScheduledFutureTask.deadlineNanos; + import java.util.Comparator; import java.util.Queue; import java.util.concurrent.Callable; -import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; /** @@ -169,7 +170,7 @@ public abstract class AbstractScheduledEventExecutor extends AbstractEventExecut validateScheduled0(delay, unit); return schedule(new ScheduledFutureTask( - this, command, null, ScheduledFutureTask.deadlineNanos(unit.toNanos(delay)))); + this, command, deadlineNanos(unit.toNanos(delay)))); } @Override @@ -181,8 +182,7 @@ public abstract class AbstractScheduledEventExecutor extends AbstractEventExecut } validateScheduled0(delay, unit); - return schedule(new ScheduledFutureTask( - this, callable, ScheduledFutureTask.deadlineNanos(unit.toNanos(delay)))); + return schedule(new ScheduledFutureTask(this, callable, deadlineNanos(unit.toNanos(delay)))); } @Override @@ -201,8 +201,7 @@ public abstract class AbstractScheduledEventExecutor extends AbstractEventExecut validateScheduled0(period, unit); return schedule(new ScheduledFutureTask( - this, Executors.callable(command, null), - ScheduledFutureTask.deadlineNanos(unit.toNanos(initialDelay)), unit.toNanos(period))); + this, command, deadlineNanos(unit.toNanos(initialDelay)), unit.toNanos(period))); } @Override @@ -222,8 +221,7 @@ public abstract class AbstractScheduledEventExecutor extends AbstractEventExecut validateScheduled0(delay, unit); return schedule(new ScheduledFutureTask( - this, Executors.callable(command, null), - ScheduledFutureTask.deadlineNanos(unit.toNanos(initialDelay)), -unit.toNanos(delay))); + this, command, deadlineNanos(unit.toNanos(initialDelay)), -unit.toNanos(delay))); } @SuppressWarnings("deprecation") diff --git a/common/src/main/java/io/netty/util/concurrent/PromiseTask.java b/common/src/main/java/io/netty/util/concurrent/PromiseTask.java index 116a9f13e7..0025815c52 100644 --- a/common/src/main/java/io/netty/util/concurrent/PromiseTask.java +++ b/common/src/main/java/io/netty/util/concurrent/PromiseTask.java @@ -20,10 +20,6 @@ import java.util.concurrent.RunnableFuture; class PromiseTask extends DefaultPromise implements RunnableFuture { - static Callable toCallable(Runnable runnable, T result) { - return new RunnableAdapter(runnable, result); - } - private static final class RunnableAdapter implements Callable { final Runnable task; final T result; @@ -45,21 +41,19 @@ class PromiseTask extends DefaultPromise implements RunnableFuture { } } - private static final Callable COMPLETED = new SentinelCallable("COMPLETED"); - private static final Callable CANCELLED = new SentinelCallable("CANCELLED"); - private static final Callable FAILED = new SentinelCallable("FAILED"); + private static final Runnable COMPLETED = new SentinelRunnable("COMPLETED"); + private static final Runnable CANCELLED = new SentinelRunnable("CANCELLED"); + private static final Runnable FAILED = new SentinelRunnable("FAILED"); - private static class SentinelCallable implements Callable { + private static class SentinelRunnable implements Runnable { private final String name; - SentinelCallable(String name) { + SentinelRunnable(String name) { this.name = name; } @Override - public T call() { - return null; - } + public void run() { } // no-op @Override public String toString() { @@ -67,10 +61,17 @@ class PromiseTask extends DefaultPromise implements RunnableFuture { } } - protected Callable task; + // Strictly of type Callable or Runnable + private Object task; PromiseTask(EventExecutor executor, Runnable runnable, V result) { - this(executor, toCallable(runnable, result)); + super(executor); + task = result == null ? runnable : new RunnableAdapter(runnable, result); + } + + PromiseTask(EventExecutor executor, Runnable runnable) { + super(executor); + task = runnable; } PromiseTask(EventExecutor executor, Callable callable) { @@ -88,11 +89,21 @@ class PromiseTask extends DefaultPromise implements RunnableFuture { return this == obj; } + @SuppressWarnings("unchecked") + final V runTask() throws Exception { + final Object task = this.task; + if (task instanceof Callable) { + return ((Callable) task).call(); + } + ((Runnable) task).run(); + return null; + } + @Override public void run() { try { if (setUncancellableInternal()) { - V result = task.call(); + V result = runTask(); setSuccessInternal(result); } } catch (Throwable e) { @@ -100,14 +111,13 @@ class PromiseTask extends DefaultPromise implements RunnableFuture { } } - @SuppressWarnings("unchecked") - private boolean clearTaskAfterCompletion(boolean done, Callable result) { + private boolean clearTaskAfterCompletion(boolean done, Runnable result) { if (done) { // The only time where it might be possible for the sentinel task // to be called is in the case of a periodic ScheduledFutureTask, // in which case it's a benign race with cancellation and the (null) // return value is not used. - task = (Callable) result; + task = result; } return done; } diff --git a/common/src/main/java/io/netty/util/concurrent/ScheduledFutureTask.java b/common/src/main/java/io/netty/util/concurrent/ScheduledFutureTask.java index ac77dc5d84..5ffe824119 100644 --- a/common/src/main/java/io/netty/util/concurrent/ScheduledFutureTask.java +++ b/common/src/main/java/io/netty/util/concurrent/ScheduledFutureTask.java @@ -51,27 +51,31 @@ final class ScheduledFutureTask extends PromiseTask implements ScheduledFu private int queueIndex = INDEX_NOT_IN_QUEUE; - ScheduledFutureTask( - AbstractScheduledEventExecutor executor, - Runnable runnable, V result, long nanoTime) { + ScheduledFutureTask(AbstractScheduledEventExecutor executor, + Runnable runnable, long nanoTime) { - this(executor, toCallable(runnable, result), nanoTime); + super(executor, runnable); + deadlineNanos = nanoTime; + periodNanos = 0; } - ScheduledFutureTask( - AbstractScheduledEventExecutor executor, + ScheduledFutureTask(AbstractScheduledEventExecutor executor, + Runnable runnable, long nanoTime, long period) { + + super(executor, runnable); + deadlineNanos = nanoTime; + periodNanos = validatePeriod(period); + } + + ScheduledFutureTask(AbstractScheduledEventExecutor executor, Callable callable, long nanoTime, long period) { super(executor, callable); - if (period == 0) { - throw new IllegalArgumentException("period: 0 (expected: != 0)"); - } deadlineNanos = nanoTime; - periodNanos = period; + periodNanos = validatePeriod(period); } - ScheduledFutureTask( - AbstractScheduledEventExecutor executor, + ScheduledFutureTask(AbstractScheduledEventExecutor executor, Callable callable, long nanoTime) { super(executor, callable); @@ -79,6 +83,13 @@ final class ScheduledFutureTask extends PromiseTask implements ScheduledFu periodNanos = 0; } + private static long validatePeriod(long period) { + if (period == 0) { + throw new IllegalArgumentException("period: 0 (expected: != 0)"); + } + return period; + } + ScheduledFutureTask setId(long id) { this.id = id; return this; @@ -136,13 +147,13 @@ final class ScheduledFutureTask extends PromiseTask implements ScheduledFu try { if (periodNanos == 0) { if (setUncancellableInternal()) { - V result = task.call(); + V result = runTask(); setSuccessInternal(result); } } else { // check if is done as it may was cancelled if (!isCancelled()) { - task.call(); + runTask(); if (!executor().isShutdown()) { if (periodNanos > 0) { deadlineNanos += periodNanos; diff --git a/common/src/main/java/io/netty/util/concurrent/UnorderedThreadPoolEventExecutor.java b/common/src/main/java/io/netty/util/concurrent/UnorderedThreadPoolEventExecutor.java index 4ed94da537..277c90322a 100644 --- a/common/src/main/java/io/netty/util/concurrent/UnorderedThreadPoolEventExecutor.java +++ b/common/src/main/java/io/netty/util/concurrent/UnorderedThreadPoolEventExecutor.java @@ -215,7 +215,7 @@ public final class UnorderedThreadPoolEventExecutor extends ScheduledThreadPoolE RunnableScheduledFutureTask(EventExecutor executor, Runnable runnable, RunnableScheduledFuture future) { - super(executor, runnable, null); + super(executor, runnable); this.future = future; } @@ -232,7 +232,7 @@ public final class UnorderedThreadPoolEventExecutor extends ScheduledThreadPoolE } else if (!isDone()) { try { // Its a periodic task so we need to ignore the return value - task.call(); + runTask(); } catch (Throwable cause) { if (!tryFailureInternal(cause)) { logger.warn("Failure during execution of task", cause);