Skip to content

Commit

Permalink
Eliminates unnecessary overhead (#1786)
Browse files Browse the repository at this point in the history
* Refactor AsyncResponseHandler

* Extract ResponseHandler from AsyncResponseHandler

* Use ResponseHandler on SynchronousMethodHandler

Eliminates unnecessary overhead caused by CompletableFutre on synchronous operation.

* Modify `AsyncResponseHandler.handleResponse` method signature

* Fix code format

Co-authored-by: Marvin Froeder <velo@users.noreply.github.com>
Co-authored-by: Marvin Froeder <marvin.froeder@dovetailstudios.com>
  • Loading branch information
3 people committed Oct 11, 2022
1 parent 542312e commit 2b141a0
Show file tree
Hide file tree
Showing 4 changed files with 167 additions and 108 deletions.
94 changes: 19 additions & 75 deletions core/src/main/java/feign/AsyncResponseHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,9 @@
*/
package feign;

import static feign.FeignException.errorReading;
import static feign.Util.ensureClosed;
import feign.Logger.Level;
import feign.codec.Decoder;
import feign.codec.ErrorDecoder;
import java.io.IOException;
import java.lang.reflect.Type;
import java.util.concurrent.CompletableFuture;

Expand All @@ -28,93 +25,40 @@
*/
@Experimental
class AsyncResponseHandler {

private static final long MAX_RESPONSE_BUFFER_SIZE = 8192L;

private final Level logLevel;
private final Logger logger;

private final Decoder decoder;
private final ErrorDecoder errorDecoder;
private final boolean dismiss404;
private final boolean closeAfterDecode;

private final ResponseInterceptor responseInterceptor;
private final ResponseHandler responseHandler;

AsyncResponseHandler(Level logLevel, Logger logger, Decoder decoder,
ErrorDecoder errorDecoder, boolean dismiss404, boolean closeAfterDecode,
ResponseInterceptor responseInterceptor) {
super();
this.logLevel = logLevel;
this.logger = logger;
this.decoder = decoder;
this.errorDecoder = errorDecoder;
this.dismiss404 = dismiss404;
this.closeAfterDecode = closeAfterDecode;
this.responseInterceptor = responseInterceptor;
this.responseHandler = new ResponseHandler(
logLevel, logger, decoder,
errorDecoder, dismiss404, closeAfterDecode,
responseInterceptor);
}

boolean isVoidType(Type returnType) {
return Void.class == returnType || void.class == returnType
|| returnType.getTypeName().equals("kotlin.Unit");
public CompletableFuture<Object> handleResponse(String configKey,
Response response,
Type returnType,
long elapsedTime) {
CompletableFuture<Object> resultFuture = new CompletableFuture<>();
handleResponse(resultFuture, configKey, response, returnType, elapsedTime);
return resultFuture;
}

/**
* @deprecated use {@link #handleResponse(String, Response, Type, long)} instead.
*/
@Deprecated()
void handleResponse(CompletableFuture<Object> resultFuture,
String configKey,
Response response,
Type returnType,
long elapsedTime) {
// copied fairly liberally from SynchronousMethodHandler
boolean shouldClose = true;

try {
if (logLevel != Level.NONE) {
response = logger.logAndRebufferResponse(configKey, logLevel, response,
elapsedTime);
}
if (Response.class == returnType) {
if (response.body() == null) {
resultFuture.complete(response);
} else if (response.body().length() == null
|| response.body().length() > MAX_RESPONSE_BUFFER_SIZE) {
shouldClose = false;
resultFuture.complete(response);
} else {
// Ensure the response body is disconnected
final byte[] bodyData = Util.toByteArray(response.body().asInputStream());
resultFuture.complete(response.toBuilder().body(bodyData).build());
}
} else if (response.status() >= 200 && response.status() < 300) {
if (isVoidType(returnType)) {
resultFuture.complete(null);
} else {
final Object result = decode(response, returnType);
shouldClose = closeAfterDecode;
resultFuture.complete(result);
}
} else if (dismiss404 && response.status() == 404 && !isVoidType(returnType)) {
final Object result = decode(response, returnType);
shouldClose = closeAfterDecode;
resultFuture.complete(result);
} else {
resultFuture.completeExceptionally(errorDecoder.decode(configKey, response));
}
} catch (final IOException e) {
if (logLevel != Level.NONE) {
logger.logIOException(configKey, logLevel, e, elapsedTime);
}
resultFuture.completeExceptionally(errorReading(response.request(), response, e));
} catch (final Exception e) {
resultFuture.complete(
this.responseHandler.handleResponse(configKey, response, returnType, elapsedTime));
} catch (Exception e) {
resultFuture.completeExceptionally(e);
} finally {
if (shouldClose) {
ensureClosed(response.body());
}
}

}

Object decode(Response response, Type type) throws IOException {
return responseInterceptor.aroundDecode(new InvocationContext(decoder, type, response));
}
}
12 changes: 2 additions & 10 deletions core/src/main/java/feign/AsynchronousMethodHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -188,16 +188,8 @@ private static Response ensureRequestIsSet(Response response,
}

private CompletableFuture<Object> handleResponse(Response response, long elapsedTime) {
CompletableFuture<Object> resultFuture = new CompletableFuture<>();

asyncResponseHandler.handleResponse(resultFuture, metadata.configKey(), response,
methodInfo.underlyingReturnType(), elapsedTime);

if (!resultFuture.isDone()) {
resultFuture.completeExceptionally(new IllegalStateException("Response handling not done"));
}

return resultFuture;
return asyncResponseHandler.handleResponse(
metadata.configKey(), response, methodInfo.underlyingReturnType(), elapsedTime);
}

private long elapsedTime(long start) {
Expand Down
141 changes: 141 additions & 0 deletions core/src/main/java/feign/ResponseHandler.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
/*
* Copyright 2012-2022 The Feign Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
package feign;

import feign.Logger.Level;
import feign.codec.Decoder;
import feign.codec.ErrorDecoder;
import java.io.IOException;
import java.lang.reflect.Type;
import static feign.FeignException.errorReading;
import static feign.Util.ensureClosed;

/**
* The response handler that is used to provide synchronous support on top of standard response
* handling
*/
public class ResponseHandler {

private static final long MAX_RESPONSE_BUFFER_SIZE = 8192L;

private final Level logLevel;
private final Logger logger;

private final Decoder decoder;
private final ErrorDecoder errorDecoder;
private final boolean dismiss404;
private final boolean closeAfterDecode;

private final ResponseInterceptor responseInterceptor;

public ResponseHandler(Level logLevel, Logger logger, Decoder decoder,
ErrorDecoder errorDecoder, boolean dismiss404, boolean closeAfterDecode,
ResponseInterceptor responseInterceptor) {
super();
this.logLevel = logLevel;
this.logger = logger;
this.decoder = decoder;
this.errorDecoder = errorDecoder;
this.dismiss404 = dismiss404;
this.closeAfterDecode = closeAfterDecode;
this.responseInterceptor = responseInterceptor;
}

public Object handleResponse(String configKey,
Response response,
Type returnType,
long elapsedTime)
throws Exception {
try {
response = logAndRebufferResponseIfNeeded(configKey, response, elapsedTime);
if (returnType == Response.class) {
return disconnectResponseBodyIfNeeded(response);
}

final boolean shouldDecodeResponseBody = (response.status() >= 200 && response.status() < 300)
|| (response.status() == 404 && dismiss404 && !isVoidType(returnType));

if (!shouldDecodeResponseBody) {
throw decodeError(configKey, response);
}

return decode(response, returnType);
} catch (final IOException e) {
if (logLevel != Level.NONE) {
logger.logIOException(configKey, logLevel, e, elapsedTime);
}
throw errorReading(response.request(), response, e);
}
}

private boolean isVoidType(Type returnType) {
return returnType == Void.class
|| returnType == void.class
|| returnType.getTypeName().equals("kotlin.Unit");
}

private Response logAndRebufferResponseIfNeeded(String configKey,
Response response,
long elapsedTime)
throws IOException {
if (logLevel == Level.NONE) {
return response;
}

return logger.logAndRebufferResponse(configKey, logLevel, response, elapsedTime);
}

private static Response disconnectResponseBodyIfNeeded(Response response) throws IOException {
final boolean shouldDisconnectResponseBody = response.body() != null
&& response.body().length() != null
&& response.body().length() <= MAX_RESPONSE_BUFFER_SIZE;
if (!shouldDisconnectResponseBody) {
return response;
}

try {
final byte[] bodyData = Util.toByteArray(response.body().asInputStream());
return response.toBuilder().body(bodyData).build();
} finally {
ensureClosed(response.body());
}
}

private Object decode(Response response, Type type) throws IOException {
if (isVoidType(type)) {
ensureClosed(response.body());
return null;
}

try {
final Object result = responseInterceptor.aroundDecode(
new InvocationContext(decoder, type, response));
if (closeAfterDecode) {
ensureClosed(response.body());
}
return result;
} catch (Exception e) {
ensureClosed(response.body());
throw e;
}
}

private Exception decodeError(String methodKey, Response response) {
try {
return errorDecoder.decode(methodKey, response);
} finally {
ensureClosed(response.body());
}
}
}
28 changes: 5 additions & 23 deletions core/src/main/java/feign/SynchronousMethodHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,27 +22,22 @@
import feign.codec.ErrorDecoder;
import java.io.IOException;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;
import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;

final class SynchronousMethodHandler implements MethodHandler {

private static final long MAX_RESPONSE_BUFFER_SIZE = 8192L;

private final MethodMetadata metadata;
private final Target<?> target;
private final Client client;
private final Retryer retryer;
private final List<RequestInterceptor> requestInterceptors;
private final ResponseInterceptor responseInterceptor;
private final Logger logger;
private final Logger.Level logLevel;
private final RequestTemplate.Factory buildTemplateFromArgs;
private final Options options;
private final ExceptionPropagationPolicy propagationPolicy;
private final AsyncResponseHandler asyncResponseHandler;
private final ResponseHandler responseHandler;


private SynchronousMethodHandler(Target<?> target, Client client, Retryer retryer,
Expand All @@ -63,8 +58,7 @@ private SynchronousMethodHandler(Target<?> target, Client client, Retryer retrye
this.buildTemplateFromArgs = checkNotNull(buildTemplateFromArgs, "metadata for %s", target);
this.options = checkNotNull(options, "options for %s", target);
this.propagationPolicy = propagationPolicy;
this.responseInterceptor = responseInterceptor;
this.asyncResponseHandler = new AsyncResponseHandler(logLevel, logger, decoder, errorDecoder,
this.responseHandler = new ResponseHandler(logLevel, logger, decoder, errorDecoder,
dismiss404, closeAfterDecode, responseInterceptor);
}

Expand Down Expand Up @@ -117,22 +111,10 @@ Object executeAndDecode(RequestTemplate template, Options options) throws Throwa
}
throw errorExecuting(request, e);
}
long elapsedTime = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);

CompletableFuture<Object> resultFuture = new CompletableFuture<>();
asyncResponseHandler.handleResponse(resultFuture, metadata.configKey(), response,
metadata.returnType(), elapsedTime);

try {
if (!resultFuture.isDone())
throw new IllegalStateException("Response handling not done");
return resultFuture.join();
} catch (CompletionException e) {
Throwable cause = e.getCause();
if (cause != null)
throw cause;
throw e;
}
long elapsedTime = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
return responseHandler.handleResponse(
metadata.configKey(), response, metadata.returnType(), elapsedTime);
}

long elapsedTime(long start) {
Expand Down

0 comments on commit 2b141a0

Please sign in to comment.