From 85a663fa52ca07c8c282cd688966b921d35fb29f Mon Sep 17 00:00:00 2001 From: Nick Hill Date: Fri, 27 Sep 2019 00:54:42 -0700 Subject: [PATCH] Null out completed tasks to help with garbage collection (#9613) Motivation When ScheduledFutureTasks complete, there's no need to retain a ref to the wrapped task. Clearing it could help in particular with the case where many scheduled tasks have been cancelled but their queue removal delayed (since it is done lazily). Modifications This comprises just the PromiseTask changes from #9580. Upon completion, replace the task reference with a static sentinel depending on the type of completion (so that it will be reflected by toString). Result More expedient collection of cancelled task objects --- .../io/netty/util/concurrent/PromiseTask.java | 47 +++++++++++++++++-- 1 file changed, 44 insertions(+), 3 deletions(-) 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 8cb23c7861..116a9f13e7 100644 --- a/common/src/main/java/io/netty/util/concurrent/PromiseTask.java +++ b/common/src/main/java/io/netty/util/concurrent/PromiseTask.java @@ -45,7 +45,29 @@ class PromiseTask extends DefaultPromise implements RunnableFuture { } } - protected final Callable task; + 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 class SentinelCallable implements Callable { + private final String name; + + SentinelCallable(String name) { + this.name = name; + } + + @Override + public T call() { + return null; + } + + @Override + public String toString() { + return name; + } + } + + protected Callable task; PromiseTask(EventExecutor executor, Runnable runnable, V result) { this(executor, toCallable(runnable, result)); @@ -78,6 +100,18 @@ class PromiseTask extends DefaultPromise implements RunnableFuture { } } + @SuppressWarnings("unchecked") + private boolean clearTaskAfterCompletion(boolean done, Callable 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; + } + return done; + } + @Override public final Promise setFailure(Throwable cause) { throw new IllegalStateException(); @@ -85,6 +119,7 @@ class PromiseTask extends DefaultPromise implements RunnableFuture { protected final Promise setFailureInternal(Throwable cause) { super.setFailure(cause); + clearTaskAfterCompletion(true, FAILED); return this; } @@ -94,7 +129,7 @@ class PromiseTask extends DefaultPromise implements RunnableFuture { } protected final boolean tryFailureInternal(Throwable cause) { - return super.tryFailure(cause); + return clearTaskAfterCompletion(super.tryFailure(cause), FAILED); } @Override @@ -104,6 +139,7 @@ class PromiseTask extends DefaultPromise implements RunnableFuture { protected final Promise setSuccessInternal(V result) { super.setSuccess(result); + clearTaskAfterCompletion(true, COMPLETED); return this; } @@ -113,7 +149,7 @@ class PromiseTask extends DefaultPromise implements RunnableFuture { } protected final boolean trySuccessInternal(V result) { - return super.trySuccess(result); + return clearTaskAfterCompletion(super.trySuccess(result), COMPLETED); } @Override @@ -125,6 +161,11 @@ class PromiseTask extends DefaultPromise implements RunnableFuture { return super.setUncancellable(); } + @Override + public boolean cancel(boolean mayInterruptIfRunning) { + return clearTaskAfterCompletion(super.cancel(mayInterruptIfRunning), CANCELLED); + } + @Override protected StringBuilder toStringBuilder() { StringBuilder buf = super.toStringBuilder();