Fix query/ec2 protocol perf regression (#7367)

* Fix query/ec2 protocol perf regression

* fix: Restore HTTP_REQUEST_URI_BEFORE_MODIFY as a lazy attribute

The previous commit removed HTTP_REQUEST_URI_BEFORE_MODIFY outright, which
broke customers who had started reading it despite it being internal API.

Restore it as a deprecated derived view over a new HTTP_REQUEST_BEFORE_MODIFY
attribute, which holds the marshalled request itself. Derived attributes apply
their read mapping on each read, so the URI is only built if something actually
asks for it, and the value is identical to before including the query string.

EndpointResolutionStage reads the endpoint components straight off the
snapshotted request, so the request path no longer builds a URI at all. This
keeps the query/ec2 protocol regression fixed: those protocols still carry the
entire payload in the raw query parameters at this point in the execution, so
building a URI cost two passes over the whole payload per API call.

* fix: Make HTTP_REQUEST_URI_BEFORE_MODIFY read-only

There is no write usage of this attribute, so setting it now throws
UnsupportedOperationException rather than projecting the URI back onto the
snapshotted request. This matches UnmodifiableExecutionAttributes, and is safe
because ExecutionAttributes copies, merges and putAbsentAttributes all duplicate
the backing map directly and never invoke a derived attribute's write mapping.

* Use new perf-improvement type

* fix: Snapshot endpoint components instead of the marshalled request

Holding the marshalled request kept its raw query parameters reachable until the
end of the API call, because the signing stage swaps the interceptor context
over to the signed request and LowCopyListMap#clear installs a fresh map rather
than mutating the original. For query and ec2 that map is the request payload:
measured at ~114KB retained for a 33KB payload, against ~69KB for the URI that
2.47.0 retained.

Snapshot only the endpoint components instead, which retains ~130 bytes. The
deprecated HTTP_REQUEST_URI_BEFORE_MODIFY becomes a view over those components,
so it now carries no query string and always renders the port. The only known
consumer parses the path for a "/invocations" suffix, which is unaffected;
EndpointUrl#toUri also memoizes, so repeated reads no longer rebuild the URI.

Guarded end to end in SyncClientHandlerTest: an interceptor reads the snapshot
during modifyHttpRequest and asserts the URI has no query string, which fails if
the snapshot is ever built from getUri() again.

* chore: Restore perf-improvement changelog type

* fix: Remove unused SdkHttpRequest import

The javadoc reference that used it was removed, and Checkstyle's UnusedImports
does not process javadoc, so the import became a build error.

* fix: Make HTTP_REQUEST_URI_BEFORE_MODIFY writable again

A customer unit test sets this attribute, so throwing on write is too strict.
Writing now replaces the backing endpoint snapshot via EndpointUrl#fromUri,
which also pre-populates the cached URI, so a written value reads back exactly
as written including its query string.

* test: Add E2E coverage for endpointOverride with interceptors

S3 is the only service here whose endpoint rules rewrite the host taken from an
endpointOverride (virtual-host addressing resolves {Bucket}.{url#authority}), so
it is the only place these assertions can tell "the resolved host was applied"
apart from "the interceptor's host was preserved".

Extend EndpointOverrideEndpointResolutionTest with overrides that spell out the
protocol's default port, and add EndpointOverrideInterceptorResolutionTest
covering the override x modifyHttpRequest matrix: an interceptor that changes
the host, scheme or port wins and the resolved path is still applied; one that
leaves the endpoint alone, adds a header, or rebuilds the request from its own
values does not suppress endpoint resolution.

Verified these fail on the pre-fix snapshot: 6 failures, including an
interceptor that merely restates the endpoint, which turns the builder's raw
port from null into an explicit 443 and so used to look like a change.

Author: alextwoods

.changes/next-release/bugfix-AWSSDKforJavav2-e181119.json

@@ -0,0 +1,6 @@
+{
+    "type": "perf-improvement",
+    "category": "AWS SDK for Java v2",
+    "contributor": "",
+    "description": "Fix request-side performance regression that scaled with payload size for `query` and `ec2` protocol services where the request endpoint was re-parsed as a URI. The internal `HTTP_REQUEST_URI_BEFORE_MODIFY` execution attribute is now deprecated and computed on demand from `HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY`; the value the SDK records carries the scheme, host, port and path but no query string."
+}

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

@@ -38,6 +38,7 @@
 import software.amazon.awssdk.core.useragent.BusinessMetricCollection;
 import software.amazon.awssdk.endpoints.Endpoint;
 import software.amazon.awssdk.endpoints.EndpointProvider;
+import software.amazon.awssdk.endpoints.EndpointUrl;
 import software.amazon.awssdk.http.SdkHttpExecutionAttributes;
 import software.amazon.awssdk.http.auth.spi.scheme.AuthScheme;
 import software.amazon.awssdk.http.auth.spi.scheme.AuthSchemeProvider;
@@ -212,11 +213,29 @@ public final class SdkInternalExecutionAttribute extends SdkExecutionAttribute {
         new ExecutionAttribute<>("EndpointResolver");
 
     /**
-     * The HTTP request URI captured before modifyHttpRequest interceptors run.
-     * Used by EndpointResolutionStage to detect if a customer interceptor modified the URL.
+     * The HTTP request endpoint (scheme, host, port and path) captured before modifyHttpRequest interceptors run.
+     * Used by EndpointResolutionStage to detect if a customer interceptor modified the endpoint.
      */
+    public static final ExecutionAttribute<EndpointUrl> HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY =
+        new ExecutionAttribute<>("HttpRequestEndpointBeforeModify");
+
+    /**
+     * The HTTP request URI captured before modifyHttpRequest interceptors run. Writing this replaces
+     * {@link #HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY} with the written URI's components.
+     *
+     * @deprecated Use {@link #HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY} instead. This is a view over that attribute, so the
+     * value recorded by the SDK carries only the scheme, host, port and path: the query string is absent, and the port
+     * is present even when it is the protocol's default. Reading it builds a {@link URI}, which
+     * {@link #HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY} lets you avoid.
+     */
+    @Deprecated
     public static final ExecutionAttribute<URI> HTTP_REQUEST_URI_BEFORE_MODIFY =
-        new ExecutionAttribute<>("HttpRequestUriBeforeModify");
+        ExecutionAttribute.derivedBuilder("HttpRequestUriBeforeModify",
+                                          URI.class,
+                                          () -> HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY)
+                          .readMapping(endpoint -> endpoint != null ? endpoint.toUri() : null)
+                          .writeMapping((endpoint, uri) -> uri != null ? EndpointUrl.fromUri(uri) : null)
+                          .build();
 
     /**
      * The selected auth scheme for a request.

core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseClientHandler.java

@@ -41,6 +41,7 @@
 import software.amazon.awssdk.core.metrics.CoreMetric;
 import software.amazon.awssdk.core.signer.Signer;
 import software.amazon.awssdk.core.sync.RequestBody;
+import software.amazon.awssdk.endpoints.EndpointUrl;
 import software.amazon.awssdk.http.ContentStreamProvider;
 import software.amazon.awssdk.http.SdkHttpFullRequest;
 import software.amazon.awssdk.http.SdkHttpFullResponse;
@@ -81,11 +82,19 @@ static <InputT extends SdkRequest, OutputT> InterceptorContext finalizeSdkHttpFu
         addHttpRequest(executionContext, request);
         runAfterMarshallingInterceptors(executionContext);
 
-        // Snapshot the HTTP request URI before modifyHttpRequest interceptors run.
-        // EndpointResolutionStage uses this to detect if a customer interceptor modified the URL.
+        // Snapshot the HTTP request endpoint before modifyHttpRequest interceptors run.
+        // EndpointResolutionStage uses this to detect if a customer interceptor modified the endpoint.
+        //
+        // Use the optimized EndpointUrl instead of an expensive URI to avoid the cost of parsing/re-parsing it.
+        // Query/EC2 protocols have the entire request payload in query params at this point which increases the time
+        // to build the URI in proportion to the size of the request.
+        SdkHttpRequest marshalledRequest = executionContext.interceptorContext().httpRequest();
         executionContext.executionAttributes().putAttribute(
-            SdkInternalExecutionAttribute.HTTP_REQUEST_URI_BEFORE_MODIFY,
-            executionContext.interceptorContext().httpRequest().getUri());
+            SdkInternalExecutionAttribute.HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY,
+            EndpointUrl.fromComponents(marshalledRequest.protocol(),
+                                       marshalledRequest.host(),
+                                       marshalledRequest.port(),
+                                       marshalledRequest.encodedPath()));
 
         return runModifyHttpRequestAndHttpContentInterceptors(executionContext);
     }

core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/EndpointResolutionStage.java

@@ -110,20 +110,23 @@ public SdkHttpFullRequest.Builder execute(SdkHttpFullRequest.Builder request, Re
     }
 
     /**
-     * Detects if an interceptor modified the HTTP request URL in modifyHttpRequest().
-     * Compares the current request's host and scheme against the snapshot taken before interceptors ran.
+     * Detects if an interceptor modified the HTTP request endpoint in modifyHttpRequest().
+     * Compares the current request's host, scheme and port against the snapshot taken before interceptors ran.
      */
     private static boolean interceptorModifiedEndpoint(SdkHttpFullRequest.Builder request, ExecutionAttributes attrs) {
-        URI preModifyUri = attrs.getAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_URI_BEFORE_MODIFY);
-        if (preModifyUri == null) {
+        EndpointUrl preModifyEndpoint =
+            attrs.getAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY);
+        if (preModifyEndpoint == null) {
             return false;
         }
         String requestHost = request.host();
+        if (requestHost == null) {
+            return false;
+        }
         Integer requestPort = request.port();
-        return requestHost != null
-            && (!requestHost.equals(preModifyUri.getHost())
-                || !String.valueOf(request.protocol()).equals(preModifyUri.getScheme())
-                || (requestPort != null && requestPort != preModifyUri.getPort()));
+        return !requestHost.equals(preModifyEndpoint.host())
+               || !String.valueOf(request.protocol()).equals(preModifyEndpoint.scheme())
+               || (requestPort != null && requestPort != preModifyEndpoint.port());
     }
 
     /**

core/sdk-core/src/test/java/software/amazon/awssdk/core/client/handler/SyncClientHandlerTest.java

@@ -15,13 +15,16 @@
 
 package software.amazon.awssdk.core.client.handler;
 
+import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
 import java.io.ByteArrayInputStream;
+import java.net.URI;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
@@ -41,14 +44,20 @@
 import software.amazon.awssdk.core.exception.RetryableException;
 import software.amazon.awssdk.core.exception.SdkServiceException;
 import software.amazon.awssdk.core.http.HttpResponseHandler;
+import software.amazon.awssdk.core.interceptor.Context;
+import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
+import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
+import software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute;
 import software.amazon.awssdk.core.protocol.VoidSdkResponse;
 import software.amazon.awssdk.core.runtime.transform.Marshaller;
 import software.amazon.awssdk.core.sync.ResponseTransformer;
+import software.amazon.awssdk.endpoints.EndpointUrl;
 import software.amazon.awssdk.http.AbortableInputStream;
 import software.amazon.awssdk.http.HttpExecuteResponse;
 import software.amazon.awssdk.http.ExecutableHttpRequest;
 import software.amazon.awssdk.http.SdkHttpClient;
 import software.amazon.awssdk.http.SdkHttpFullRequest;
+import software.amazon.awssdk.http.SdkHttpRequest;
 import software.amazon.awssdk.http.SdkHttpResponse;
 import software.amazon.awssdk.retries.DefaultRetryStrategy;
 import utils.HttpTestUtils;
@@ -189,6 +198,69 @@ private void mockSuccessfulApiCall() throws Exception {
         when(responseHandler.handle(any(), any())).thenReturn(VoidSdkResponse.builder().build());
     }
 
+    /**
+     * The pre-modify snapshot must never encode the marshalled request's query parameters. For the query and ec2
+     * protocols those parameters still hold the entire request payload when the snapshot is taken, so encoding them
+     * costs a full pass over the payload on every API call, and retaining them keeps the payload alive for the rest of
+     * the call. Guards the regression fixed in this change.
+     */
+    @Test
+    public void snapshottedEndpoint_doesNotEncodeOrRetainQueryParameters() throws Exception {
+        SdkHttpFullRequest.Builder marshalled =
+            ValidSdkObjects.sdkHttpFullRequest(8080)
+                           .encodedPath("/2015-03-31/functions/my-function/invocations")
+                           .putRawQueryParameter("Qualifier", "prod");
+        // A handful of payload-shaped parameters is enough: the assertion is that the query string is absent
+        // entirely, so any of these appearing in the snapshot is a failure.
+        for (int i = 0; i < 20; i++) {
+            marshalled.putRawQueryParameter("MetricData.member." + i + ".Value", "12345.6789");
+        }
+
+        List<EndpointUrl> capturedEndpoint = new ArrayList<>();
+        List<URI> capturedUri = new ArrayList<>();
+        ExecutionInterceptor interceptor = new ExecutionInterceptor() {
+            @Override
+            public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context, ExecutionAttributes attrs) {
+                capturedEndpoint.add(
+                    attrs.getAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY));
+                capturedUri.add(attrs.getAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_URI_BEFORE_MODIFY));
+                return context.httpRequest();
+            }
+        };
+
+        SdkSyncClientHandler handler = new SdkSyncClientHandler(
+            clientConfiguration().toBuilder()
+                                 .option(SdkClientOption.EXECUTION_INTERCEPTORS, singletonList(interceptor))
+                                 .build());
+
+        when(marshaller.marshall(request)).thenReturn(marshalled.build());
+        when(httpClient.prepareRequest(any())).thenReturn(httpClientCall);
+        when(httpClientCall.call()).thenReturn(HttpExecuteResponse.builder()
+                                                                 .response(SdkHttpResponse.builder()
+                                                                                          .statusCode(200)
+                                                                                          .build())
+                                                                 .build());
+        when(responseHandler.handle(any(), any())).thenReturn(VoidSdkResponse.builder().build());
+
+        handler.execute(clientExecutionParams());
+
+        // The endpoint snapshot holds components only; nothing references the query parameters.
+        assertThat(capturedEndpoint).hasSize(1);
+        assertThat(capturedEndpoint.get(0).host()).isEqualTo("localhost");
+        assertThat(capturedEndpoint.get(0).encodedPath()).isEqualTo("/2015-03-31/functions/my-function/invocations");
+
+        // The deprecated URI view carries no query string, so the payload was never encoded.
+        URI uri = capturedUri.get(0);
+        assertThat(uri.getQuery()).isNull();
+        assertThat(uri.toString()).isEqualTo("http://localhost:8080/2015-03-31/functions/my-function/invocations");
+
+        // The known consumer pattern: extract the path suffix following "/invocations".
+        String path = uri.toString();
+        int idx = path.indexOf("/invocations");
+        assertThat(idx).isGreaterThanOrEqualTo(0);
+        assertThat(path.substring(idx + "/invocations".length())).isEmpty();
+    }
+
     private void expectRetrievalFromMocks() {
         when(marshaller.marshall(request)).thenReturn(marshalledRequest);
         when(httpClient.prepareRequest(any())).thenReturn(httpClientCall);

core/sdk-core/src/test/java/software/amazon/awssdk/core/interceptor/SdkInternalExecutionAttributeTest.java

@@ -0,0 +1,126 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file 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 software.amazon.awssdk.core.interceptor;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.net.URI;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import software.amazon.awssdk.endpoints.EndpointUrl;
+
+/**
+ * Tests for the deprecated {@link SdkInternalExecutionAttribute#HTTP_REQUEST_URI_BEFORE_MODIFY}, which is a derived
+ * view over {@link SdkInternalExecutionAttribute#HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY}.
+ */
+class SdkInternalExecutionAttributeTest {
+
+    private static final EndpointUrl ENDPOINT =
+        EndpointUrl.fromComponents("https", "lambda.us-east-1.amazonaws.com", 443,
+                                   "/2015-03-31/functions/my-function/invocations");
+
+    private ExecutionAttributes attributes;
+
+    @BeforeEach
+    void setup() {
+        attributes = new ExecutionAttributes();
+    }
+
+    @Test
+    void httpRequestUriBeforeModify_noSnapshot_isNull() {
+        assertThat(attributes.getAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_URI_BEFORE_MODIFY)).isNull();
+    }
+
+    @Test
+    void httpRequestUriBeforeModify_rendersSnapshottedEndpoint() {
+        attributes.putAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY, ENDPOINT);
+
+        assertThat(attributes.getAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_URI_BEFORE_MODIFY))
+            .isEqualTo(URI.create("https://lambda.us-east-1.amazonaws.com:443"
+                                  + "/2015-03-31/functions/my-function/invocations"));
+    }
+
+    @Test
+    void httpRequestUriBeforeModify_neverCarriesQueryString() {
+        // The snapshot holds components only, so the query string is always absent. This is what keeps the query and
+        // ec2 protocols cheap: for those the raw query parameters are the entire request payload at this point.
+        attributes.putAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY, ENDPOINT);
+
+        URI uri = attributes.getAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_URI_BEFORE_MODIFY);
+
+        assertThat(uri.getQuery()).isNull();
+        assertThat(uri.getRawPath()).isEqualTo("/2015-03-31/functions/my-function/invocations");
+    }
+
+    @Test
+    void httpRequestUriBeforeModify_repeatedReadsShareTheSameUri() {
+        attributes.putAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY, ENDPOINT);
+
+        assertThat(attributes.getAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_URI_BEFORE_MODIFY))
+            .isSameAs(attributes.getAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_URI_BEFORE_MODIFY));
+    }
+
+    @Test
+    void httpRequestUriBeforeModify_writeReplacesTheEndpoint() {
+        attributes.putAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY, ENDPOINT);
+
+        attributes.putAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_URI_BEFORE_MODIFY,
+                                URI.create("http://custom.example.com:8443/my-path"));
+
+        EndpointUrl endpoint =
+            attributes.getAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY);
+        assertThat(endpoint.scheme()).isEqualTo("http");
+        assertThat(endpoint.host()).isEqualTo("custom.example.com");
+        assertThat(endpoint.port()).isEqualTo(8443);
+        assertThat(endpoint.encodedPath()).isEqualTo("/my-path");
+    }
+
+    @Test
+    void httpRequestUriBeforeModify_writeRoundTripsExactly() {
+        // A written URI reads back unchanged, query string included, so a caller that sets this attribute sees
+        // precisely what it set.
+        URI written = URI.create("https://custom.example.com/my-path?Qualifier=prod");
+
+        attributes.putAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_URI_BEFORE_MODIFY, written);
+
+        assertThat(attributes.getAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_URI_BEFORE_MODIFY))
+            .isEqualTo(written);
+    }
+
+    @Test
+    void httpRequestUriBeforeModify_writeNull_clearsTheEndpoint() {
+        attributes.putAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY, ENDPOINT);
+
+        attributes.putAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_URI_BEFORE_MODIFY, null);
+
+        assertThat(attributes.getAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY)).isNull();
+        assertThat(attributes.getAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_URI_BEFORE_MODIFY)).isNull();
+    }
+
+    @Test
+    void httpRequestEndpointBeforeModify_survivesCopy() {
+        attributes.putAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY, ENDPOINT);
+
+        // Copies duplicate the backing map rather than re-setting each attribute, so the read-only derived view must
+        // still resolve against the copy.
+        ExecutionAttributes copy = attributes.copy();
+
+        assertThat(copy.getAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_ENDPOINT_BEFORE_MODIFY))
+            .isSameAs(ENDPOINT);
+        assertThat(copy.getAttribute(SdkInternalExecutionAttribute.HTTP_REQUEST_URI_BEFORE_MODIFY))
+            .isEqualTo(ENDPOINT.toUri());
+    }
+}