Skip to content

Commit 1eaba50

Browse files
millemsmillems
authored
Various performance improvements: (#3178)
1. Replace uses of String.replace with StringUtils.replace and StringUtils.replaceEach to avoid the overhead of regex parsing the search term. 2. Remove uses of SdkHttpRequest and SdkHttpResponse's headers and rawQueryParameters calls in favor of more specific methods that do not require data copying. Added spotbugs rules to prevent uses of these methods in the future. 3. Update SdkHttpRequest and SdkHttpResponse to not copy data in headers and rawQueryParameters when round-tripped to/from builder unless it is actually modified. 4. Reduce string copies performed during message signing. 5. Remove uses of String.format that were performance-impacting on profiler output. 6. Remove construction of log strings within the netty client unless the logging is enabled. 7. Move most user-agent construction to client creation time instead of at request time. 8. Initialize collection/map capacities to prevent resizes that will always occur. 9. Prevent creation of RateLimitingTokenBucket unless adaptive retries are enabled.
1 parent 9515f77 commit 1eaba50

File tree

86 files changed

+1623
-736
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

86 files changed

+1623
-736
lines changed

‎build-tools/pom.xml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
</configuration>
6161
</plugin>
6262
</plugins>
63+
6364
</build>
6465

6566
<dependencies>
@@ -68,6 +69,11 @@
6869
<artifactId>checkstyle</artifactId>
6970
<version>8.42</version>
7071
</dependency>
72+
<dependency>
73+
<groupId>com.github.spotbugs</groupId>
74+
<artifactId>spotbugs</artifactId>
75+
<version>4.2.3</version>
76+
</dependency>
7177
</dependencies>
7278

7379
</project>

‎build-tools/src/main/java/software/amazon/awssdk/buildtools/findbugs/DisallowMethodCall.java

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.buildtools.findbugs;
17+
18+
import edu.umd.cs.findbugs.BugInstance;
19+
import edu.umd.cs.findbugs.BugReporter;
20+
import edu.umd.cs.findbugs.ba.SignatureParser;
21+
import edu.umd.cs.findbugs.bcel.OpcodeStackDetector;
22+
import edu.umd.cs.findbugs.classfile.MethodDescriptor;
23+
import java.util.AbstractMap.SimpleEntry;
24+
import java.util.HashSet;
25+
import java.util.Map.Entry;
26+
import java.util.Set;
27+
import org.apache.bcel.Const;
28+
29+
/**
30+
* Blocks usage of disallowed methods in the SDK.
31+
*/
32+
public class DisallowMethodCall extends OpcodeStackDetector {
33+
private static final Set<Entry<String, String>> PROHIBITED_METHODS = new HashSet<>();
34+
private final BugReporter bugReporter;
35+
36+
static {
37+
PROHIBITED_METHODS.add(new SimpleEntry<>("software/amazon/awssdk/http/SdkHttpHeaders", "headers"));
38+
PROHIBITED_METHODS.add(new SimpleEntry<>("software/amazon/awssdk/http/SdkHttpResponse", "headers"));
39+
PROHIBITED_METHODS.add(new SimpleEntry<>("software/amazon/awssdk/http/SdkHttpRequest", "headers"));
40+
PROHIBITED_METHODS.add(new SimpleEntry<>("software/amazon/awssdk/http/SdkHttpFullRequest", "headers"));
41+
PROHIBITED_METHODS.add(new SimpleEntry<>("software/amazon/awssdk/http/SdkHttpFullResponse", "headers"));
42+
PROHIBITED_METHODS.add(new SimpleEntry<>("software/amazon/awssdk/http/SdkHttpFullRequest$Builder", "headers"));
43+
PROHIBITED_METHODS.add(new SimpleEntry<>("software/amazon/awssdk/http/SdkHttpFullResponse$Builder", "headers"));
44+
PROHIBITED_METHODS.add(new SimpleEntry<>("software/amazon/awssdk/http/SdkHttpRequest", "rawQueryParameters"));
45+
PROHIBITED_METHODS.add(new SimpleEntry<>("software/amazon/awssdk/http/SdkHttpFullRequest", "rawQueryParameters"));
46+
PROHIBITED_METHODS.add(new SimpleEntry<>("software/amazon/awssdk/http/SdkHttpFullRequest$Builder", "rawQueryParameters"));
47+
}
48+
49+
public DisallowMethodCall(BugReporter bugReporter) {
50+
this.bugReporter = bugReporter;
51+
}
52+
53+
@Override
54+
public void sawOpcode(int code) {
55+
switch (code) {
56+
case Const.INVOKEVIRTUAL:
57+
case Const.INVOKESPECIAL:
58+
case Const.INVOKESTATIC:
59+
case Const.INVOKEINTERFACE:
60+
handleMethodCall(code);
61+
return;
62+
default:
63+
// Ignore - not a method call.
64+
}
65+
}
66+
67+
private void handleMethodCall(int code) {
68+
MethodDescriptor method = getMethodDescriptorOperand();
69+
SignatureParser signature = new SignatureParser(method.getSignature());
70+
Entry<String, String> calledMethod = new SimpleEntry<>(method.getSlashedClassName(), method.getName());
71+
if (PROHIBITED_METHODS.contains(calledMethod) && signature.getNumParameters() == 0) {
72+
bugReporter.reportBug(new BugInstance(this, "SDK_BAD_METHOD_CALL", NORMAL_PRIORITY)
73+
.addClassAndMethod(this)
74+
.addSourceLine(this, getPC()));
75+
}
76+
}
77+
}

‎build-tools/src/main/resources/findbugs.xml

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<!--
2+
~ Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
~
4+
~ Licensed under the Apache License, Version 2.0 (the "License").
5+
~ You may not use this file except in compliance with the License.
6+
~ A copy of the License is located at
7+
~
8+
~ http://aws.amazon.com/apache2.0
9+
~
10+
~ or in the "license" file accompanying this file. This file is distributed
11+
~ on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
~ express or implied. See the License for the specific language governing
13+
~ permissions and limitations under the License.
14+
-->
15+
16+
<FindbugsPlugin>
17+
<Detector class="software.amazon.awssdk.buildtools.findbugs.DisallowMethodCall" speed="fast" />
18+
<BugPattern abbrev="BM" type="SDK_BAD_METHOD_CALL" category="PERFORMANCE" />
19+
</FindbugsPlugin>

‎build-tools/src/main/resources/messages.xml

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
<!--
2+
~ Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
~
4+
~ Licensed under the Apache License, Version 2.0 (the "License").
5+
~ You may not use this file except in compliance with the License.
6+
~ A copy of the License is located at
7+
~
8+
~ http://aws.amazon.com/apache2.0
9+
~
10+
~ or in the "license" file accompanying this file. This file is distributed
11+
~ on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
~ express or implied. See the License for the specific language governing
13+
~ permissions and limitations under the License.
14+
-->
15+
16+
<MessageCollection>
17+
18+
<Detector class="software.amazon.awssdk.buildtools.findbugs.DisallowMethodCall" >
19+
<Details>This detector checks for method calls that are not allowed for use.</Details>
20+
</Detector>
21+
22+
<BugPattern type="SDK_BAD_METHOD_CALL">
23+
<ShortDescription>Bad method call</ShortDescription>
24+
25+
<LongDescription>
26+
<![CDATA[
27+
{1} uses a method that is prohibited.
28+
The headers() and rawQueryParameters() methods create a deep copy of the data, which can result in
29+
significant memory pressure. Instead of retrieving a copy of the headers to look at it, use methods like
30+
hasHeaders(), firstMatchingHeader(...) or forEachHeader(..) which may be optimized in the type to avoid
31+
copying the data.
32+
]]>
33+
</LongDescription>
34+
35+
<Details>
36+
<![CDATA[
37+
The headers() and rawQueryParameters() methods create a deep copy of the data, which can result in
38+
significant memory pressure. Instead of retrieving a copy of the headers to look at it, use methods like
39+
hasHeaders(), firstMatchingHeader(...) or forEachHeader(..) which may be optimized in the type to avoid
40+
copying the data.
41+
]]>
42+
</Details>
43+
</BugPattern>
44+
45+
<BugCode abbrev="BM">Bad method</BugCode>
46+
</MessageCollection>

‎build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,4 +230,13 @@
230230
<Match>
231231
<Bug pattern="DMI_RANDOM_USED_ONLY_ONCE" />
232232
</Match>
233+
234+
<!-- Uses these methods legitimately. -->
235+
<Match>
236+
<Or>
237+
<Class name="software.amazon.awssdk.http.SdkHttpHeaders"/>
238+
<Class name="software.amazon.awssdk.http.SdkHttpRequest"/>
239+
</Or>
240+
<Bug pattern="SDK_BAD_METHOD_CALL"/>
241+
</Match>
233242
</FindBugsFilter>

‎codegen-lite-maven-plugin/pom.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@
5757
<groupId>software.amazon.awssdk</groupId>
5858
<version>${awsjavasdk.version}</version>
5959
</dependency>
60+
<dependency>
61+
<artifactId>utils</artifactId>
62+
<groupId>software.amazon.awssdk</groupId>
63+
<version>${awsjavasdk.version}</version>
64+
</dependency>
6065
<dependency>
6166
<artifactId>maven-plugin-api</artifactId>
6267
<groupId>org.apache.maven</groupId>

‎codegen-lite-maven-plugin/src/main/java/software/amazon/awssdk/codegen/lite/maven/plugin/DefaultsModeGenerationMojo.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import software.amazon.awssdk.codegen.lite.defaultsmode.DefaultsLoader;
2828
import software.amazon.awssdk.codegen.lite.defaultsmode.DefaultsModeConfigurationGenerator;
2929
import software.amazon.awssdk.codegen.lite.defaultsmode.DefaultsModeGenerator;
30+
import software.amazon.awssdk.utils.StringUtils;
3031

3132
/**
3233
* The Maven mojo to generate defaults mode related classes.
@@ -61,12 +62,12 @@ public void execute() {
6162
}
6263

6364
public void generateDefaultsModeClass(Path baseSourcesDirectory, DefaultConfiguration configuration) {
64-
Path sourcesDirectory = baseSourcesDirectory.resolve(DEFAULTS_MODE_BASE.replace(".", "/"));
65+
Path sourcesDirectory = baseSourcesDirectory.resolve(StringUtils.replace(DEFAULTS_MODE_BASE, ".", "/"));
6566
new CodeGenerator(sourcesDirectory.toString(), new DefaultsModeGenerator(DEFAULTS_MODE_BASE, configuration)).generate();
6667
}
6768

6869
public void generateDefaultsModeConfiguartionClass(Path baseSourcesDirectory, DefaultConfiguration configuration) {
69-
Path sourcesDirectory = baseSourcesDirectory.resolve(DEFAULTS_MODE_CONFIGURATION_BASE.replace(".", "/"));
70+
Path sourcesDirectory = baseSourcesDirectory.resolve(StringUtils.replace(DEFAULTS_MODE_CONFIGURATION_BASE, ".", "/"));
7071
new CodeGenerator(sourcesDirectory.toString(), new DefaultsModeConfigurationGenerator(DEFAULTS_MODE_CONFIGURATION_BASE,
7172
DEFAULTS_MODE_BASE,
7273
configuration)).generate();

‎codegen-lite-maven-plugin/src/main/java/software/amazon/awssdk/codegen/lite/maven/plugin/RegionGenerationMojo.java

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import software.amazon.awssdk.codegen.lite.regions.ServiceMetadataGenerator;
3737
import software.amazon.awssdk.codegen.lite.regions.ServiceMetadataProviderGenerator;
3838
import software.amazon.awssdk.codegen.lite.regions.model.Partitions;
39+
import software.amazon.awssdk.utils.StringUtils;
3940

4041
/**
4142
* The Maven mojo to generate Java client code using software.amazon.awssdk:codegen module.
@@ -79,7 +80,7 @@ public void execute() throws MojoExecutionException {
7980
}
8081

8182
public void generatePartitionMetadataClass(Path baseSourcesDirectory, Partitions partitions) {
82-
Path sourcesDirectory = baseSourcesDirectory.resolve(PARTITION_METADATA_BASE.replace(".", "/"));
83+
Path sourcesDirectory = baseSourcesDirectory.resolve(StringUtils.replace(PARTITION_METADATA_BASE, ".", "/"));
8384
partitions.getPartitions()
8485
.forEach(p -> new CodeGenerator(sourcesDirectory.toString(),
8586
new PartitionMetadataGenerator(p,
@@ -88,12 +89,12 @@ public void generatePartitionMetadataClass(Path baseSourcesDirectory, Partitions
8889
}
8990

9091
public void generateRegionClass(Path baseSourcesDirectory, Partitions partitions) {
91-
Path sourcesDirectory = baseSourcesDirectory.resolve(REGION_BASE.replace(".", "/"));
92+
Path sourcesDirectory = baseSourcesDirectory.resolve(StringUtils.replace(REGION_BASE, ".", "/"));
9293
new CodeGenerator(sourcesDirectory.toString(), new RegionGenerator(partitions, REGION_BASE)).generate();
9394
}
9495

9596
public void generateServiceMetadata(Path baseSourcesDirectory, Partitions partitions) {
96-
Path sourcesDirectory = baseSourcesDirectory.resolve(SERVICE_METADATA_BASE.replace(".", "/"));
97+
Path sourcesDirectory = baseSourcesDirectory.resolve(StringUtils.replace(SERVICE_METADATA_BASE, ".", "/"));
9798
Set<String> services = new HashSet<>();
9899
partitions.getPartitions().forEach(p -> services.addAll(p.getServices().keySet()));
99100

@@ -105,7 +106,7 @@ public void generateServiceMetadata(Path baseSourcesDirectory, Partitions partit
105106
}
106107

107108
public void generateRegions(Path baseSourcesDirectory, Partitions partitions) {
108-
Path sourcesDirectory = baseSourcesDirectory.resolve(REGION_METADATA_BASE.replace(".", "/"));
109+
Path sourcesDirectory = baseSourcesDirectory.resolve(StringUtils.replace(REGION_METADATA_BASE, ".", "/"));
109110
partitions.getPartitions()
110111
.forEach(p -> p.getRegions().forEach((k, v) ->
111112
new CodeGenerator(sourcesDirectory.toString(),
@@ -118,31 +119,31 @@ public void generateRegions(Path baseSourcesDirectory, Partitions partitions) {
118119
}
119120

120121
public void generatePartitionProvider(Path baseSourcesDirectory, Partitions partitions) {
121-
Path sourcesDirectory = baseSourcesDirectory.resolve(REGION_BASE.replace(".", "/"));
122+
Path sourcesDirectory = baseSourcesDirectory.resolve(StringUtils.replace(REGION_BASE, ".", "/"));
122123
new CodeGenerator(sourcesDirectory.toString(), new PartitionMetadataProviderGenerator(partitions,
123124
PARTITION_METADATA_BASE,
124125
REGION_BASE))
125126
.generate();
126127
}
127128

128129
public void generateRegionProvider(Path baseSourcesDirectory, Partitions partitions) {
129-
Path sourcesDirectory = baseSourcesDirectory.resolve(REGION_BASE.replace(".", "/"));
130+
Path sourcesDirectory = baseSourcesDirectory.resolve(StringUtils.replace(REGION_BASE, ".", "/"));
130131
new CodeGenerator(sourcesDirectory.toString(), new RegionMetadataProviderGenerator(partitions,
131132
REGION_METADATA_BASE,
132133
REGION_BASE))
133134
.generate();
134135
}
135136

136137
public void generateServiceProvider(Path baseSourcesDirectory, Partitions partitions) {
137-
Path sourcesDirectory = baseSourcesDirectory.resolve(REGION_BASE.replace(".", "/"));
138+
Path sourcesDirectory = baseSourcesDirectory.resolve(StringUtils.replace(REGION_BASE, ".", "/"));
138139
new CodeGenerator(sourcesDirectory.toString(), new ServiceMetadataProviderGenerator(partitions,
139140
SERVICE_METADATA_BASE,
140141
REGION_BASE))
141142
.generate();
142143
}
143144

144145
public void generateEndpointTags(Path baseSourcesDirectory, Partitions partitions) {
145-
Path sourcesDirectory = baseSourcesDirectory.resolve(REGION_BASE.replace(".", "/"));
146+
Path sourcesDirectory = baseSourcesDirectory.resolve(StringUtils.replace(REGION_BASE, ".", "/"));
146147
new CodeGenerator(sourcesDirectory.toString(), new EndpointTagGenerator(partitions, REGION_BASE)).generate();
147148
}
148149
}

‎core/auth-crt/src/main/java/software/amazon/awssdk/authcrt/signer/internal/CrtHttpRequestConverter.java

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
package software.amazon.awssdk.authcrt.signer.internal;
1717

1818
import static java.lang.Math.min;
19-
import static software.amazon.awssdk.utils.CollectionUtils.isNullOrEmpty;
2019

2120
import java.io.ByteArrayInputStream;
2221
import java.io.IOException;
@@ -26,7 +25,6 @@
2625
import java.nio.ByteBuffer;
2726
import java.util.ArrayList;
2827
import java.util.List;
29-
import java.util.Map;
3028
import java.util.Optional;
3129
import software.amazon.awssdk.annotations.SdkInternalApi;
3230
import software.amazon.awssdk.crt.auth.signing.AwsSigningResult;
@@ -52,9 +50,7 @@ public HttpRequest requestToCrt(SdkHttpFullRequest inputRequest) {
5250
String method = inputRequest.method().name();
5351
String encodedPath = encodedPathToCrtFormat(inputRequest.encodedPath());
5452

55-
String encodedQueryString = SdkHttpUtils.encodeAndFlattenQueryParameters(inputRequest.rawQueryParameters())
56-
.map(value -> "?" + value)
57-
.orElse("");
53+
String encodedQueryString = inputRequest.encodedQueryParameters().map(value -> "?" + value).orElse("");
5854

5955
HttpHeader[] crtHeaderArray = createHttpHeaderArray(inputRequest);
6056

@@ -129,20 +125,20 @@ public HttpRequestBodyStream toCrtStream(byte[] data) {
129125
}
130126

131127
private HttpHeader[] createHttpHeaderArray(SdkHttpFullRequest request) {
132-
List<HttpHeader> crtHeaderList = new ArrayList<>(request.headers().size() + 2);
128+
List<HttpHeader> crtHeaderList = new ArrayList<>(request.numHeaders() + 2);
133129

134130
// Set Host Header if needed
135-
if (isNullOrEmpty(request.headers().get(HOST_HEADER))) {
131+
if (!request.firstMatchingHeader(HOST_HEADER).isPresent()) {
136132
crtHeaderList.add(new HttpHeader(HOST_HEADER, request.host()));
137133
}
138134

139135
// Add the rest of the Headers
140-
for (Map.Entry<String, List<String>> headerList: request.headers().entrySet()) {
141-
for (String val: headerList.getValue()) {
142-
HttpHeader h = new HttpHeader(headerList.getKey(), val);
136+
request.forEachHeader((name, values) -> {
137+
for (String val : values) {
138+
HttpHeader h = new HttpHeader(name, val);
143139
crtHeaderList.add(h);
144140
}
145-
}
141+
});
146142

147143
return crtHeaderList.toArray(new HttpHeader[0]);
148144
}

‎core/auth-crt/src/main/java/software/amazon/awssdk/authcrt/signer/internal/SigningUtils.java

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
import java.nio.charset.StandardCharsets;
1919
import java.time.Clock;
2020
import java.time.Duration;
21-
import java.util.List;
22-
import java.util.Map;
2321
import java.util.Optional;
2422
import java.util.Set;
2523
import java.util.TreeSet;
@@ -116,11 +114,11 @@ public static SdkHttpFullRequest sanitizeSdkRequestForCrtSigning(SdkHttpFullRequ
116114
builder.clearHeaders();
117115

118116
// Filter headers that will cause signing to fail
119-
for (Map.Entry<String, List<String>> header: request.headers().entrySet()) {
120-
if (!FORBIDDEN_HEADERS.contains(header.getKey())) {
121-
builder.putHeader(header.getKey(), header.getValue());
117+
request.forEachHeader((name, value) -> {
118+
if (!FORBIDDEN_HEADERS.contains(name)) {
119+
builder.putHeader(name, value);
122120
}
123-
}
121+
});
124122

125123
// Add host, which must be signed. We ignore any pre-existing Host header to match the behavior of the SigV4 signer.
126124
String hostHeader = SdkHttpUtils.isUsingStandardPort(request.protocol(), request.port())
@@ -131,12 +129,11 @@ public static SdkHttpFullRequest sanitizeSdkRequestForCrtSigning(SdkHttpFullRequ
131129
builder.clearQueryParameters();
132130

133131
// Filter query parameters that will cause signing to fail
134-
Map<String, List<String>> params = request.rawQueryParameters();
135-
for (Map.Entry<String, List<String>> param: params.entrySet()) {
136-
if (!FORBIDDEN_PARAMS.contains(param.getKey())) {
137-
builder.putRawQueryParameter(param.getKey(), param.getValue());
132+
request.forEachRawQueryParameter((key, value) -> {
133+
if (!FORBIDDEN_PARAMS.contains(key)) {
134+
builder.putRawQueryParameter(key, value);
138135
}
139-
}
136+
});
140137

141138
return builder.build();
142139
}

‎core/auth/src/main/java/software/amazon/awssdk/auth/signer/AsyncAws4Signer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public CompletableFuture<SdkHttpFullRequest> signWithBody(SdkHttpFullRequest req
6969
Aws4SignerRequestParams requestParams = new Aws4SignerRequestParams(signingParams);
7070

7171
SdkHttpFullRequest.Builder builder = doSign(request, requestParams, signingParams,
72-
new ContentChecksum(digestHex, sdkChecksum));
72+
new ContentChecksum(digestHex, sdkChecksum));
7373

7474
return builder.build();
7575
});

0 commit comments

Comments
 (0)