Avoid CancellationException construction in DefaultPromise (#9534)
Motivation #9152 reverted some static exception reuse optimizations due to the problem with Throwable#addSuppressed() raised in #9151. This introduced a performance issue when promises are cancelled at a high frequency due to the construction cost of CancellationException at the time that DefaultPromise#cancel() is called. Modifications - Reinstate the prior static CANCELLATION_CAUSE_HOLDER but use it just as a sentinel to indicate cancellation, constructing a new CancellationException only if/when one needs to be explicitly returned/thrown - Subclass CancellationException, overriding fillInStackTrace() to minimize the construction cost in these cases Result Promises are much cheaper to cancel. Fixes #9522.
This commit is contained in:
parent
d446765b84
commit
768a825035
@ -19,6 +19,7 @@ import io.netty.util.internal.InternalThreadLocalMap;
|
|||||||
import io.netty.util.internal.PlatformDependent;
|
import io.netty.util.internal.PlatformDependent;
|
||||||
import io.netty.util.internal.StringUtil;
|
import io.netty.util.internal.StringUtil;
|
||||||
import io.netty.util.internal.SystemPropertyUtil;
|
import io.netty.util.internal.SystemPropertyUtil;
|
||||||
|
import io.netty.util.internal.ThrowableUtil;
|
||||||
import io.netty.util.internal.logging.InternalLogger;
|
import io.netty.util.internal.logging.InternalLogger;
|
||||||
import io.netty.util.internal.logging.InternalLoggerFactory;
|
import io.netty.util.internal.logging.InternalLoggerFactory;
|
||||||
|
|
||||||
@ -40,6 +41,9 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
|
|||||||
AtomicReferenceFieldUpdater.newUpdater(DefaultPromise.class, Object.class, "result");
|
AtomicReferenceFieldUpdater.newUpdater(DefaultPromise.class, Object.class, "result");
|
||||||
private static final Object SUCCESS = new Object();
|
private static final Object SUCCESS = new Object();
|
||||||
private static final Object UNCANCELLABLE = new Object();
|
private static final Object UNCANCELLABLE = new Object();
|
||||||
|
private static final CauseHolder CANCELLATION_CAUSE_HOLDER = new CauseHolder(ThrowableUtil.unknownStackTrace(
|
||||||
|
new CancellationException(), DefaultPromise.class, "cancel(...)"));
|
||||||
|
private static final StackTraceElement[] CANCELLATION_STACK = CANCELLATION_CAUSE_HOLDER.cause.getStackTrace();
|
||||||
|
|
||||||
private volatile Object result;
|
private volatile Object result;
|
||||||
private final EventExecutor executor;
|
private final EventExecutor executor;
|
||||||
@ -131,11 +135,33 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
|
|||||||
return result == null;
|
return result == null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private static final class LeanCancellationException extends CancellationException {
|
||||||
|
private static final long serialVersionUID = 2794674970981187807L;
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public Throwable fillInStackTrace() {
|
||||||
|
setStackTrace(CANCELLATION_STACK);
|
||||||
|
return this;
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public String toString() {
|
||||||
|
return CancellationException.class.getName();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public Throwable cause() {
|
public Throwable cause() {
|
||||||
Object result = this.result;
|
Object result = this.result;
|
||||||
|
if (result != CANCELLATION_CAUSE_HOLDER) {
|
||||||
return (result instanceof CauseHolder) ? ((CauseHolder) result).cause : null;
|
return (result instanceof CauseHolder) ? ((CauseHolder) result).cause : null;
|
||||||
}
|
}
|
||||||
|
CancellationException ce = new LeanCancellationException();
|
||||||
|
if (RESULT_UPDATER.compareAndSet(this, CANCELLATION_CAUSE_HOLDER, new CauseHolder(ce))) {
|
||||||
|
return ce;
|
||||||
|
}
|
||||||
|
return ((CauseHolder) this.result).cause;
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public Promise<V> addListener(GenericFutureListener<? extends Future<? super V>> listener) {
|
public Promise<V> addListener(GenericFutureListener<? extends Future<? super V>> listener) {
|
||||||
@ -301,8 +327,7 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
|
|||||||
*/
|
*/
|
||||||
@Override
|
@Override
|
||||||
public boolean cancel(boolean mayInterruptIfRunning) {
|
public boolean cancel(boolean mayInterruptIfRunning) {
|
||||||
if (RESULT_UPDATER.get(this) == null &&
|
if (RESULT_UPDATER.compareAndSet(this, null, CANCELLATION_CAUSE_HOLDER)) {
|
||||||
RESULT_UPDATER.compareAndSet(this, null, new CauseHolder(new CancellationException()))) {
|
|
||||||
if (checkNotifyWaiters()) {
|
if (checkNotifyWaiters()) {
|
||||||
notifyListeners();
|
notifyListeners();
|
||||||
}
|
}
|
||||||
|
@ -37,6 +37,7 @@ import java.util.concurrent.TimeoutException;
|
|||||||
import java.util.concurrent.atomic.AtomicInteger;
|
import java.util.concurrent.atomic.AtomicInteger;
|
||||||
|
|
||||||
import static java.lang.Math.max;
|
import static java.lang.Math.max;
|
||||||
|
import static org.hamcrest.Matchers.instanceOf;
|
||||||
import static org.hamcrest.Matchers.lessThan;
|
import static org.hamcrest.Matchers.lessThan;
|
||||||
import static org.junit.Assert.*;
|
import static org.junit.Assert.*;
|
||||||
|
|
||||||
@ -70,7 +71,7 @@ public class DefaultPromiseTest {
|
|||||||
Mockito.when(executor.inEventLoop()).thenReturn(false);
|
Mockito.when(executor.inEventLoop()).thenReturn(false);
|
||||||
|
|
||||||
Promise<Void> promise = new DefaultPromise<Void>(executor);
|
Promise<Void> promise = new DefaultPromise<Void>(executor);
|
||||||
promise.cancel(false);
|
assertTrue(promise.cancel(false));
|
||||||
Mockito.verify(executor, Mockito.never()).execute(Mockito.any(Runnable.class));
|
Mockito.verify(executor, Mockito.never()).execute(Mockito.any(Runnable.class));
|
||||||
assertTrue(promise.isCancelled());
|
assertTrue(promise.isCancelled());
|
||||||
}
|
}
|
||||||
@ -102,7 +103,7 @@ public class DefaultPromiseTest {
|
|||||||
@Test(expected = CancellationException.class)
|
@Test(expected = CancellationException.class)
|
||||||
public void testCancellationExceptionIsThrownWhenBlockingGet() throws InterruptedException, ExecutionException {
|
public void testCancellationExceptionIsThrownWhenBlockingGet() throws InterruptedException, ExecutionException {
|
||||||
final Promise<Void> promise = new DefaultPromise<Void>(ImmediateEventExecutor.INSTANCE);
|
final Promise<Void> promise = new DefaultPromise<Void>(ImmediateEventExecutor.INSTANCE);
|
||||||
promise.cancel(false);
|
assertTrue(promise.cancel(false));
|
||||||
promise.get();
|
promise.get();
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -110,10 +111,18 @@ public class DefaultPromiseTest {
|
|||||||
public void testCancellationExceptionIsThrownWhenBlockingGetWithTimeout() throws InterruptedException,
|
public void testCancellationExceptionIsThrownWhenBlockingGetWithTimeout() throws InterruptedException,
|
||||||
ExecutionException, TimeoutException {
|
ExecutionException, TimeoutException {
|
||||||
final Promise<Void> promise = new DefaultPromise<Void>(ImmediateEventExecutor.INSTANCE);
|
final Promise<Void> promise = new DefaultPromise<Void>(ImmediateEventExecutor.INSTANCE);
|
||||||
promise.cancel(false);
|
assertTrue(promise.cancel(false));
|
||||||
promise.get(1, TimeUnit.SECONDS);
|
promise.get(1, TimeUnit.SECONDS);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testCancellationExceptionIsReturnedAsCause() throws InterruptedException,
|
||||||
|
ExecutionException, TimeoutException {
|
||||||
|
final Promise<Void> promise = new DefaultPromise<Void>(ImmediateEventExecutor.INSTANCE);
|
||||||
|
assertTrue(promise.cancel(false));
|
||||||
|
assertThat(promise.cause(), instanceOf(CancellationException.class));
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testStackOverflowWithImmediateEventExecutorA() throws Exception {
|
public void testStackOverflowWithImmediateEventExecutorA() throws Exception {
|
||||||
testStackOverFlowChainedFuturesA(stackOverflowTestDepth(), ImmediateEventExecutor.INSTANCE, true);
|
testStackOverFlowChainedFuturesA(stackOverflowTestDepth(), ImmediateEventExecutor.INSTANCE, true);
|
||||||
|
Loading…
Reference in New Issue
Block a user