-
Notifications
You must be signed in to change notification settings - Fork 916
Various performance improvements #3175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
String placeholder = "not available"; | ||
String prefix = "Received " + (response.isSuccessful() ? "successful" : "failed") + | ||
" response: "; | ||
StringJoiner details = new StringJoiner(", ", prefix, ""); | ||
details.add("Status Code: " + response.statusCode()); | ||
String requestId = SdkHttpUtils.firstMatchingHeaderFromCollection(response.headers(), | ||
X_AMZN_REQUEST_ID_HEADERS) | ||
.orElse(placeholder); | ||
|
||
details.add("Request ID: " + requestId); | ||
|
||
response.firstMatchingHeader(X_AMZ_ID_2_HEADER).ifPresent(extendedRequestId -> | ||
details.add("Extended Request ID: " + extendedRequestId)); | ||
return details.toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we're worrying about efficiency, should we just use StringBuilder? This feels hard to read with StringJoiner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, makes sense
if (!VALUE_MAP.containsKey(normalizedValue)) { | ||
throw new IllegalArgumentException("The provided value is not a valid algorithm " + value); | ||
} | ||
return VALUE_MAP.get(normalizedValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're being picky about performance, should we just do a get and check to see if the result is null? (I don't think we need null to be a valid algorithm)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, updated.
Kudos, SonarCloud Quality Gate passed! |
* Various performance improvements * Address feedback and fix failed unit test * Fix test
S3 team had been, partly on our reports [1] working on numerous performance enhancements [2] [3] that Apache James IMAP workload would definitely benefit from. [1] aws/aws-sdk-java-v2#3162 [2] aws/aws-sdk-java-v2#3175 [3] aws/aws-sdk-java-v2#3178 Upgrade
S3 team had been, partly on our reports [1] working on numerous performance enhancements [2] [3] that Apache James IMAP workload would definitely benefit from. [1] aws/aws-sdk-java-v2#3162 [2] aws/aws-sdk-java-v2#3175 [3] aws/aws-sdk-java-v2#3178
Motivation and Context
Modifications
Various performance improvements