Avoid redundant volatile read in DefaultPromise#get() (#9547)

Motivation

Currently every call to get() on a promise results in two reads of the
volatile result field when one would suffice. Maybe this is optimized
away but it seems sensible not to rely on that.

Modification

Reimplement get() and get(...) in DefaultPromise to reduce volatile access.

Result

Fewer volatile reads.
This commit is contained in:
Nick Hill 2019-09-09 00:54:38 -07:00 committed by Norman Maurer
parent 572b6a8f19
commit 629eae2082

View File

@ -24,7 +24,9 @@ import io.netty.util.internal.logging.InternalLogger;
import io.netty.util.internal.logging.InternalLoggerFactory; import io.netty.util.internal.logging.InternalLoggerFactory;
import java.util.concurrent.CancellationException; import java.util.concurrent.CancellationException;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
import static io.netty.util.internal.ObjectUtil.checkNotNull; import static io.netty.util.internal.ObjectUtil.checkNotNull;
@ -152,15 +154,21 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
@Override @Override
public Throwable cause() { public Throwable cause() {
Object result = this.result; return cause0(result);
if (result != CANCELLATION_CAUSE_HOLDER) { }
return (result instanceof CauseHolder) ? ((CauseHolder) result).cause : null;
private Throwable cause0(Object result) {
if (!(result instanceof CauseHolder)) {
return null;
} }
CancellationException ce = new LeanCancellationException(); if (result == CANCELLATION_CAUSE_HOLDER) {
if (RESULT_UPDATER.compareAndSet(this, CANCELLATION_CAUSE_HOLDER, new CauseHolder(ce))) { CancellationException ce = new LeanCancellationException();
return ce; if (RESULT_UPDATER.compareAndSet(this, CANCELLATION_CAUSE_HOLDER, new CauseHolder(ce))) {
return ce;
}
result = this.result;
} }
return ((CauseHolder) this.result).cause; return ((CauseHolder) result).cause;
} }
@Override @Override
@ -320,6 +328,50 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
return (V) result; return (V) result;
} }
@SuppressWarnings("unchecked")
@Override
public V get() throws InterruptedException, ExecutionException {
Object result = this.result;
if (!isDone0(result)) {
await();
result = this.result;
}
if (result == SUCCESS || result == UNCANCELLABLE) {
return null;
}
Throwable cause = cause0(result);
if (cause == null) {
return (V) result;
}
if (cause instanceof CancellationException) {
throw (CancellationException) cause;
}
throw new ExecutionException(cause);
}
@SuppressWarnings("unchecked")
@Override
public V get(long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException {
Object result = this.result;
if (!isDone0(result)) {
if (!await(timeout, unit)) {
throw new TimeoutException();
}
result = this.result;
}
if (result == SUCCESS || result == UNCANCELLABLE) {
return null;
}
Throwable cause = cause0(result);
if (cause == null) {
return (V) result;
}
if (cause instanceof CancellationException) {
throw (CancellationException) cause;
}
throw new ExecutionException(cause);
}
/** /**
* {@inheritDoc} * {@inheritDoc}
* *