Improve PresignedRequest throughput by avoiding URI construction

core/aws-core/src/main/java/software/amazon/awssdk/awscore/presigner/PresignedRequest.java

@@ -27,6 +27,7 @@
 import software.amazon.awssdk.core.SdkBytes;
 import software.amazon.awssdk.http.SdkHttpRequest;
 import software.amazon.awssdk.utils.Validate;
+import software.amazon.awssdk.utils.http.SdkHttpUtils;
 
 
 /**
@@ -49,7 +50,13 @@ protected PresignedRequest(DefaultBuilder<?> builder) {
         this.signedHeaders = Validate.notEmpty(builder.signedHeaders, "signedHeaders");
         this.signedPayload = builder.signedPayload;
         this.httpRequest = Validate.notNull(builder.httpRequest, "httpRequest");
-        this.url = invokeSafely(httpRequest.getUri()::toURL);
+        this.url = toUrl(httpRequest);
+    }
+
+    private static URL toUrl(SdkHttpRequest request) {
+        String queryString = request.encodedQueryParameters().map(query -> "?" + query).orElse("");
+        int port = SdkHttpUtils.isUsingStandardPort(request.protocol(), request.port()) ? -1 : request.port();
+        return invokeSafely(() -> new URL(request.protocol(), request.host(), port, request.encodedPath() + queryString));
     }
 
     /**
@@ -230,4 +237,4 @@ private B thisBuilder() {
             return (B) this;
         }
     }
-}
+}

core/aws-core/src/test/java/software/amazon/awssdk/awscore/presigner/PresignedRequestTest.java

@@ -0,0 +1,163 @@
+/*
+ * 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.awscore.presigner;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.net.URI;
+import java.time.Instant;
+import java.util.Collections;
+import org.junit.jupiter.api.Test;
+import software.amazon.awssdk.http.SdkHttpFullRequest;
+import software.amazon.awssdk.http.SdkHttpMethod;
+import software.amazon.awssdk.http.SdkHttpRequest;
+
+class PresignedRequestTest {
+    @Test
+    void url_withStandardHttpsPort_omitsPort() throws Exception {
+        SdkHttpRequest httpRequest = requestBuilder()
+            .protocol("https")
+            .host("example.com")
+            .port(443)
+            .encodedPath("/resource")
+            .build();
+
+        assertEquivalentToUriConversion(httpRequest);
+        assertThat(presignedRequest(httpRequest).url().getPort()).isEqualTo(-1);
+        assertThat(presignedRequest(httpRequest).url()).hasToString("https://example.com/resource");
+    }
+
+    @Test
+    void url_withStandardHttpPort_omitsPort() throws Exception {
+        SdkHttpRequest httpRequest = requestBuilder()
+            .protocol("http")
+            .host("example.com")
+            .port(80)
+            .encodedPath("/resource")
+            .build();
+
+        assertEquivalentToUriConversion(httpRequest);
+        assertThat(presignedRequest(httpRequest).url().getPort()).isEqualTo(-1);
+        assertThat(presignedRequest(httpRequest).url()).hasToString("http://example.com/resource");
+    }
+
+    @Test
+    void url_withImplicitDefaultPort_omitsPort() throws Exception {
+        SdkHttpRequest httpRequest = requestBuilder()
+            .protocol("https")
+            .host("example.com")
+            .encodedPath("/resource")
+            .build();
+
+        assertThat(httpRequest.port()).isEqualTo(443);
+        assertEquivalentToUriConversion(httpRequest);
+        assertThat(presignedRequest(httpRequest).url().getPort()).isEqualTo(-1);
+    }
+
+    @Test
+    void url_withCustomPort_preservesPort() throws Exception {
+        SdkHttpRequest httpRequest = requestBuilder()
+            .protocol("https")
+            .host("example.com")
+            .port(8443)
+            .encodedPath("/resource")
+            .build();
+
+        assertEquivalentToUriConversion(httpRequest);
+        assertThat(presignedRequest(httpRequest).url().getPort()).isEqualTo(8443);
+        assertThat(presignedRequest(httpRequest).url()).hasToString("https://example.com:8443/resource");
+    }
+
+    @Test
+    void url_withEncodedPathAndQuery_preservesEncodedValues() throws Exception {
+        SdkHttpRequest httpRequest = requestBuilder()
+            .protocol("https")
+            .host("example.com")
+            .encodedPath("/a%20path/%2Fvalue%3Fquery%23fragment%25percent")
+            .putRawQueryParameter("key with space", "value/with?#%characters")
+            .build();
+
+        assertEquivalentToUriConversion(httpRequest);
+        assertThat(presignedRequest(httpRequest).url())
+            .hasToString("https://example.com/a%20path/%2Fvalue%3Fquery%23fragment%25percent"
+                         + "?key%20with%20space=value%2Fwith%3F%23%25characters");
+    }
+
+    @Test
+    void url_withEmptyPath_preservesEmptyPath() throws Exception {
+        SdkHttpRequest httpRequest = requestBuilder()
+            .protocol("https")
+            .host("example.com")
+            .build();
+
+        assertEquivalentToUriConversion(httpRequest);
+        assertThat(presignedRequest(httpRequest).url()).hasToString("https://example.com");
+    }
+
+    @Test
+    void url_withEmptyPathAndQuery_preservesQuery() throws Exception {
+        SdkHttpRequest httpRequest = requestBuilder()
+            .protocol("https")
+            .host("example.com")
+            .putRawQueryParameter("foo", "bar")
+            .build();
+
+        assertEquivalentToUriConversion(httpRequest);
+        assertThat(presignedRequest(httpRequest).url()).hasToString("https://example.com?foo=bar");
+    }
+
+    @Test
+    void url_withIpv6Host_preservesHost() throws Exception {
+        SdkHttpRequest httpRequest = requestBuilder()
+            .uri(URI.create("https://[2001:db8::1]:8443/resource"))
+            .build();
+
+        assertEquivalentToUriConversion(httpRequest);
+        assertThat(presignedRequest(httpRequest).url())
+            .hasToString("https://[2001:db8::1]:8443/resource");
+    }
+
+    private static void assertEquivalentToUriConversion(SdkHttpRequest httpRequest) throws Exception {
+        assertThat(presignedRequest(httpRequest).url().toExternalForm())
+            .isEqualTo(httpRequest.getUri().toURL().toExternalForm());
+    }
+
+    private static SdkHttpFullRequest.Builder requestBuilder() {
+        return SdkHttpFullRequest.builder().method(SdkHttpMethod.GET);
+    }
+
+    private static TestPresignedRequest presignedRequest(SdkHttpRequest httpRequest) {
+        return new TestBuilder()
+            .expiration(Instant.EPOCH)
+            .isBrowserExecutable(true)
+            .signedHeaders(Collections.singletonMap("host", Collections.singletonList(httpRequest.host())))
+            .httpRequest(httpRequest)
+            .build();
+    }
+
+    private static final class TestPresignedRequest extends PresignedRequest {
+        private TestPresignedRequest(TestBuilder builder) {
+            super(builder);
+        }
+    }
+
+    private static final class TestBuilder extends PresignedRequest.DefaultBuilder<TestBuilder> {
+        @Override
+        public TestPresignedRequest build() {
+            return new TestPresignedRequest(this);
+        }
+    }
+}

test/sdk-benchmarks/pom.xml

@@ -118,6 +118,11 @@
             <artifactId>cloudfront</artifactId>
             <version>${awsjavasdk.version}</version>
         </dependency>
+        <dependency>
+            <groupId>software.amazon.awssdk</groupId>
+            <artifactId>s3</artifactId>
+            <version>${awsjavasdk.version}</version>
+        </dependency>
         <dependency>
             <groupId>software.amazon.awssdk</groupId>
             <artifactId>sts</artifactId>

test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/presigner/S3PresignerBenchmark.java

@@ -0,0 +1,80 @@
+/*
+ * 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.benchmark.presigner;
+
+import java.time.Duration;
+import java.util.concurrent.TimeUnit;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Level;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.TearDown;
+import org.openjdk.jmh.annotations.Warmup;
+import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
+import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
+import software.amazon.awssdk.regions.Region;
+import software.amazon.awssdk.services.s3.model.GetObjectRequest;
+import software.amazon.awssdk.services.s3.presigner.S3Presigner;
+import software.amazon.awssdk.services.s3.presigner.model.GetObjectPresignRequest;
+import software.amazon.awssdk.services.s3.presigner.model.PresignedGetObjectRequest;
+
+@State(Scope.Thread)
+@Warmup(iterations = 5)
+@Measurement(iterations = 5)
+@Fork(2)
+@BenchmarkMode(Mode.Throughput)
+public class S3PresignerBenchmark {
+    private static final String BUCKET = "bucket";
+    private static final String KEY = "key";
+
+    private S3Presigner presigner;
+    private GetObjectPresignRequest request;
+
+    @Setup(Level.Trial)
+    public void setup() {
+        presigner = S3Presigner.builder()
+                               .region(Region.US_EAST_1)
+                               .credentialsProvider(StaticCredentialsProvider.create(
+                                     AwsBasicCredentials.create("dummykey", "dummysecret")))
+                               .build();
+
+        GetObjectRequest getObjectRequest =
+            GetObjectRequest.builder()
+                            .bucket(BUCKET)
+                            .key(KEY)
+                            .build();
+
+        request = GetObjectPresignRequest.builder()
+                                         .signatureDuration(Duration.ofMinutes(15))
+                                         .getObjectRequest(getObjectRequest)
+                                         .build();
+    }
+
+    @TearDown(Level.Trial)
+    public void tearDown() {
+        presigner.close();
+    }
+
+    @Benchmark
+    public PresignedGetObjectRequest presignGetObject() {
+        return presigner.presignGetObject(request);
+    }
+}