Skip to content

Commit

Permalink
Refactor OkHttpCall to use CAS instead of synchronized (square#4297)
Browse files Browse the repository at this point in the history
Replace synchronized blocks with atomic compare-and-swap logic. It prevent virtual thread pinning and reduce contention.
See square#4297 for more details.
  • Loading branch information
qwexter committed Feb 21, 2025
1 parent e673ec7 commit 86cde28
Showing 1 changed file with 41 additions and 53 deletions.
94 changes: 41 additions & 53 deletions retrofit/src/main/java/retrofit2/OkHttpCall.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@

import java.io.IOException;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import javax.annotation.Nullable;
import javax.annotation.concurrent.GuardedBy;
import okhttp3.MediaType;
import okhttp3.Request;
import okhttp3.ResponseBody;
Expand All @@ -37,16 +38,11 @@ final class OkHttpCall<T> implements Call<T> {
private final okhttp3.Call.Factory callFactory;
private final Converter<ResponseBody, T> responseConverter;

private volatile boolean canceled;

@GuardedBy("this")
private @Nullable okhttp3.Call rawCall;

@GuardedBy("this") // Either a RuntimeException, non-fatal Error, or IOException.
private @Nullable Throwable creationFailure;

@GuardedBy("this")
private boolean executed;
private final AtomicBoolean canceled = new AtomicBoolean(false);
private final AtomicBoolean executed = new AtomicBoolean(false);
private final AtomicReference<okhttp3.Call> rawCallRef = new AtomicReference<>();
// Either a RuntimeException, non-fatal Error, or IOException.
private final AtomicReference<Throwable> creationFailureRef = new AtomicReference<>();

OkHttpCall(
RequestFactory requestFactory,
Expand All @@ -68,7 +64,7 @@ public OkHttpCall<T> clone() {
}

@Override
public synchronized Request request() {
public Request request() {
try {
return getRawCall().request();
} catch (IOException e) {
Expand All @@ -77,7 +73,7 @@ public synchronized Request request() {
}

@Override
public synchronized Timeout timeout() {
public Timeout timeout() {
try {
return getRawCall().timeout();
} catch (IOException e) {
Expand All @@ -89,12 +85,12 @@ public synchronized Timeout timeout() {
* Returns the raw call, initializing it if necessary. Throws if initializing the raw call throws,
* or has thrown in previous attempts to create it.
*/
@GuardedBy("this")
private okhttp3.Call getRawCall() throws IOException {
okhttp3.Call call = rawCall;
okhttp3.Call call = rawCallRef.get();
if (call != null) return call;

// Re-throw previous failures if this isn't the first attempt.
Throwable creationFailure = creationFailureRef.get();
if (creationFailure != null) {
if (creationFailure instanceof IOException) {
throw (IOException) creationFailure;
Expand All @@ -107,10 +103,12 @@ private okhttp3.Call getRawCall() throws IOException {

// Create and remember either the success or the failure.
try {
return rawCall = createRawCall();
final okhttp3.Call newCall = createRawCall();
rawCallRef.compareAndSet(null, newCall);
return rawCallRef.get();
} catch (RuntimeException | Error | IOException e) {
throwIfFatal(e); // Do not assign a fatal error to creationFailure.
creationFailure = e;
creationFailureRef.set(e);
throw e;
}
}
Expand All @@ -119,22 +117,19 @@ private okhttp3.Call getRawCall() throws IOException {
public void enqueue(final Callback<T> callback) {
Objects.requireNonNull(callback, "callback == null");

okhttp3.Call call;
Throwable failure;

synchronized (this) {
if (executed) throw new IllegalStateException("Already executed.");
executed = true;

call = rawCall;
failure = creationFailure;
if (call == null && failure == null) {
try {
call = rawCall = createRawCall();
} catch (Throwable t) {
throwIfFatal(t);
failure = creationFailure = t;
}
if (!executed.compareAndSet(false, true)) throw new IllegalStateException("Already executed.");

okhttp3.Call call = rawCallRef.get();
Throwable failure = creationFailureRef.get();

if (call == null && failure == null) {
try {
call = createRawCall();
rawCallRef.compareAndSet(null, call);
} catch (Throwable t) {
throwIfFatal(t);
creationFailureRef.compareAndSet(null, t);
failure = t;
}
}

Expand All @@ -143,7 +138,7 @@ public void enqueue(final Callback<T> callback) {
return;
}

if (canceled) {
if (canceled.get()) {
call.cancel();
}

Expand Down Expand Up @@ -185,22 +180,19 @@ private void callFailure(Throwable e) {
}

@Override
public synchronized boolean isExecuted() {
return executed;
public boolean isExecuted() {
return executed.get();
}

@Override
public Response<T> execute() throws IOException {
okhttp3.Call call;

synchronized (this) {
if (executed) throw new IllegalStateException("Already executed.");
executed = true;

call = getRawCall();
if (!executed.compareAndSet(false, true)) {
throw new IllegalStateException("Already executed.");
}

if (canceled) {
okhttp3.Call call = getRawCall();

if (canceled.get()) {
call.cancel();
}

Expand Down Expand Up @@ -255,25 +247,21 @@ Response<T> parseResponse(okhttp3.Response rawResponse) throws IOException {

@Override
public void cancel() {
canceled = true;
canceled.set(true);

okhttp3.Call call;
synchronized (this) {
call = rawCall;
}
okhttp3.Call call = rawCallRef.get();
if (call != null) {
call.cancel();
}
}

@Override
public boolean isCanceled() {
if (canceled) {
if (canceled.get()) {
return true;
}
synchronized (this) {
return rawCall != null && rawCall.isCanceled();
}
okhttp3.Call call = rawCallRef.get();
return call != null && call.isCanceled();
}

static final class NoContentResponseBody extends ResponseBody {
Expand Down

0 comments on commit 86cde28

Please sign in to comment.