Skip to content

Commit 3b86c3b

Browse files
authored
Performance optimizations in ExecutionInterceptorChain (#4863)
1. Removed apply*Hack method. This method wasn't doing anything, because its side effects were immediately overridden. 2. Do not modify the execution context in the modify* methods unless the interceptor requests a modification. The time added by the comparisons are less than the cost of always modifying the interceptor context, because modifying the execution context is relatively rare.
1 parent 5c2529f commit 3b86c3b

File tree

1 file changed

+32
-49
lines changed

1 file changed

+32
-49
lines changed

core/sdk-core/src/main/java/software/amazon/awssdk/core/interceptor/ExecutionInterceptorChain.java

Lines changed: 32 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import java.util.ArrayList;
2121
import java.util.List;
2222
import java.util.Objects;
23-
import java.util.Optional;
2423
import java.util.function.Consumer;
2524
import org.reactivestreams.Publisher;
2625
import software.amazon.awssdk.annotations.SdkProtectedApi;
@@ -29,8 +28,6 @@
2928
import software.amazon.awssdk.core.async.AsyncRequestBody;
3029
import software.amazon.awssdk.core.internal.interceptor.DefaultFailedExecutionContext;
3130
import software.amazon.awssdk.core.sync.RequestBody;
32-
import software.amazon.awssdk.http.ContentStreamProvider;
33-
import software.amazon.awssdk.http.SdkHttpFullRequest;
3431
import software.amazon.awssdk.http.SdkHttpRequest;
3532
import software.amazon.awssdk.http.SdkHttpResponse;
3633
import software.amazon.awssdk.utils.Logger;
@@ -66,9 +63,11 @@ public InterceptorContext modifyRequest(InterceptorContext context, ExecutionAtt
6663
InterceptorContext result = context;
6764
for (ExecutionInterceptor interceptor : interceptors) {
6865
SdkRequest interceptorResult = interceptor.modifyRequest(result, executionAttributes);
69-
validateInterceptorResult(result.request(), interceptorResult, interceptor, "modifyRequest");
7066

71-
result = result.copy(b -> b.request(interceptorResult));
67+
if (interceptorResult != result.request()) {
68+
validateInterceptorResult(result.request(), interceptorResult, interceptor, "modifyRequest");
69+
result = result.copy(b -> b.request(interceptorResult));
70+
}
7271
}
7372
return result;
7473
}
@@ -88,46 +87,20 @@ public InterceptorContext modifyHttpRequestAndHttpContent(InterceptorContext con
8887
AsyncRequestBody asyncRequestBody = interceptor.modifyAsyncHttpContent(result, executionAttributes).orElse(null);
8988
RequestBody requestBody = interceptor.modifyHttpContent(result, executionAttributes).orElse(null);
9089
SdkHttpRequest interceptorResult = interceptor.modifyHttpRequest(result, executionAttributes);
91-
validateInterceptorResult(result.httpRequest(), interceptorResult, interceptor, "modifyHttpRequest");
9290

93-
InterceptorContext.Builder builder = result.toBuilder();
91+
if (asyncRequestBody != result.asyncRequestBody().orElse(null) ||
92+
requestBody != result.requestBody().orElse(null) ||
93+
interceptorResult != result.httpRequest()) {
9494

95-
applySdkHttpFullRequestHack(result, builder);
96-
97-
result = builder.httpRequest(interceptorResult)
98-
.asyncRequestBody(asyncRequestBody)
99-
.requestBody(requestBody)
100-
.build();
95+
validateInterceptorResult(result.httpRequest(), interceptorResult, interceptor, "modifyHttpRequest");
96+
result = result.copy(r -> r.httpRequest(interceptorResult)
97+
.asyncRequestBody(asyncRequestBody)
98+
.requestBody(requestBody));
99+
}
101100
}
102101
return result;
103102
}
104103

105-
private void applySdkHttpFullRequestHack(InterceptorContext context, InterceptorContext.Builder builder) {
106-
// Someone thought it would be a great idea to allow interceptors to return SdkHttpFullRequest to modify the payload
107-
// instead of using the modifyPayload method. This is for backwards-compatibility with those interceptors.
108-
// TODO: Update interceptors to use the proper payload-modifying method so that this code path is only used for older
109-
// client versions. Maybe if we ever decide to break @SdkProtectedApis (if we stop using Jackson?!) we can even remove
110-
// this hack!
111-
SdkHttpFullRequest sdkHttpFullRequest = (SdkHttpFullRequest) context.httpRequest();
112-
113-
if (context.requestBody().isPresent()) {
114-
return;
115-
}
116-
117-
Optional<ContentStreamProvider> contentStreamProvider = sdkHttpFullRequest.contentStreamProvider();
118-
119-
if (!contentStreamProvider.isPresent()) {
120-
return;
121-
}
122-
123-
long contentLength = Long.parseLong(sdkHttpFullRequest.firstMatchingHeader("Content-Length").orElse("0"));
124-
String contentType = sdkHttpFullRequest.firstMatchingHeader("Content-Type").orElse("");
125-
RequestBody requestBody = RequestBody.fromContentProvider(contentStreamProvider.get(),
126-
contentLength,
127-
contentType);
128-
builder.requestBody(requestBody);
129-
}
130-
131104
public void beforeTransmission(Context.BeforeTransmission context, ExecutionAttributes executionAttributes) {
132105
interceptors.forEach(i -> i.beforeTransmission(context, executionAttributes));
133106
}
@@ -143,11 +116,15 @@ public InterceptorContext modifyHttpResponse(InterceptorContext context,
143116
for (int i = interceptors.size() - 1; i >= 0; i--) {
144117
SdkHttpResponse interceptorResult =
145118
interceptors.get(i).modifyHttpResponse(result, executionAttributes);
146-
validateInterceptorResult(result.httpResponse(), interceptorResult, interceptors.get(i), "modifyHttpResponse");
147-
148119
InputStream response = interceptors.get(i).modifyHttpResponseContent(result, executionAttributes).orElse(null);
149120

150-
result = result.toBuilder().httpResponse(interceptorResult).responseBody(response).build();
121+
if (interceptorResult != result.httpResponse() || response != result.responseBody().orElse(null)) {
122+
validateInterceptorResult(result.httpResponse(), interceptorResult, interceptors.get(i), "modifyHttpResponse");
123+
result = result.copy(r -> r.httpResponse(interceptorResult)
124+
.responseBody(response));
125+
}
126+
127+
151128
}
152129

153130
return result;
@@ -163,9 +140,9 @@ public InterceptorContext modifyAsyncHttpResponse(InterceptorContext context,
163140
Publisher<ByteBuffer> newResponsePublisher =
164141
interceptor.modifyAsyncHttpResponseContent(result, executionAttributes).orElse(null);
165142

166-
result = result.toBuilder()
167-
.responsePublisher(newResponsePublisher)
168-
.build();
143+
if (newResponsePublisher != result.responsePublisher().orElse(null)) {
144+
result = result.copy(r -> r.responsePublisher(newResponsePublisher));
145+
}
169146
}
170147

171148
return result;
@@ -183,9 +160,11 @@ public InterceptorContext modifyResponse(InterceptorContext context, ExecutionAt
183160
InterceptorContext result = context;
184161
for (int i = interceptors.size() - 1; i >= 0; i--) {
185162
SdkResponse interceptorResult = interceptors.get(i).modifyResponse(result, executionAttributes);
186-
validateInterceptorResult(result.response(), interceptorResult, interceptors.get(i), "modifyResponse");
187163

188-
result = result.copy(b -> b.response(interceptorResult));
164+
if (interceptorResult != result.response()) {
165+
validateInterceptorResult(result.response(), interceptorResult, interceptors.get(i), "modifyResponse");
166+
result = result.copy(b -> b.response(interceptorResult));
167+
}
189168
}
190169

191170
return result;
@@ -200,8 +179,12 @@ public DefaultFailedExecutionContext modifyException(DefaultFailedExecutionConte
200179
DefaultFailedExecutionContext result = context;
201180
for (int i = interceptors.size() - 1; i >= 0; i--) {
202181
Throwable interceptorResult = interceptors.get(i).modifyException(result, executionAttributes);
203-
validateInterceptorResult(result.exception(), interceptorResult, interceptors.get(i), "modifyException");
204-
result = result.copy(b -> b.exception(interceptorResult));
182+
183+
if (interceptorResult != result.exception()) {
184+
validateInterceptorResult(result.exception(), interceptorResult,
185+
interceptors.get(i), "modifyException");
186+
result = result.copy(b -> b.exception(interceptorResult));
187+
}
205188
}
206189

207190
return result;

0 commit comments

Comments
 (0)