-
Notifications
You must be signed in to change notification settings - Fork 1k
Optimize json marshaller #6857
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
Merged
Merged
Optimize json marshaller #6857
Changes from 15 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
2605aaf
Optimize json marshaller (first pass for testing)
alextwoods 3a48144
Refactor code to reduce duplication from switch statement
alextwoods 9be8184
Change approaches - move cache into the marshaller instead of SDKFields
alextwoods a837836
Merge branch 'master' into alexwoo/json_marshall_opt
alextwoods 6f5bf03
Optimize byte buffer/output stream
alextwoods 6776dc9
Optimize binary marshalling
alextwoods c3993ac
Try new more dynamic output stream approach
alextwoods 23cf86e
Merge branch 'master' into alexwoo/json_marshall_opt
alextwoods b4ae8a7
Cleanups
alextwoods e93c62f
Improve testing
alextwoods 6f264af
Merge branch 'master' into alexwoo/json_marshall_opt
alextwoods 5198d8e
Remove computeIfAbsent from the warm path
alextwoods b71bc56
Extract SdkByteArrayOutputStream and GC optimizations to separate branch
alextwoods 58a9d99
Move large SDK_BYTES test to alexwoo/json_huge_gc_opt branch
alextwoods a3feb6c
Move writeBinaryValue and SDK_BYTES asByteArrayUnsafe optimization to…
alextwoods 0066051
Merge branch 'master' into alexwoo/json_marshall_opt
aws-sdk-java-automation 6ba98cb
Merge branch 'master' into alexwoo/json_marshall_opt
alextwoods File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| { | ||
| "type": "feature", | ||
| "category": "AWS SDK for Java v2", | ||
| "contributor": "", | ||
| "description": "Optimized JSON marshalling performance for JSON RPC, REST JSON and RPCv2 Cbor protocols." | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
168 changes: 168 additions & 0 deletions
168
...tware/amazon/awssdk/protocols/json/internal/marshall/CachedNonPayloadMarshallingTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,168 @@ | ||
| /* | ||
| * 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.protocols.json.internal.marshall; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| import java.net.URI; | ||
| import java.util.Arrays; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import org.junit.jupiter.api.Test; | ||
| import software.amazon.awssdk.core.SdkField; | ||
| import software.amazon.awssdk.core.SdkPojo; | ||
| import software.amazon.awssdk.core.protocol.MarshallLocation; | ||
| import software.amazon.awssdk.core.protocol.MarshallingType; | ||
| import software.amazon.awssdk.core.traits.LocationTrait; | ||
| import software.amazon.awssdk.http.SdkHttpFullRequest; | ||
| import software.amazon.awssdk.http.SdkHttpMethod; | ||
| import software.amazon.awssdk.protocols.core.OperationInfo; | ||
| import software.amazon.awssdk.protocols.core.ProtocolMarshaller; | ||
| import software.amazon.awssdk.protocols.json.AwsJsonProtocol; | ||
| import software.amazon.awssdk.protocols.json.AwsJsonProtocolMetadata; | ||
| import software.amazon.awssdk.protocols.json.internal.AwsStructuredPlainJsonFactory; | ||
|
|
||
| /** | ||
| * Tests that the cached non-payload marshalling path in | ||
| * {@link JsonProtocolMarshaller#marshallFieldViaRegistry} produces correct output | ||
| * and that the cache is populated after the first call. | ||
| */ | ||
| class CachedNonPayloadMarshallingTest { | ||
|
|
||
| private static final URI ENDPOINT = URI.create("http://localhost"); | ||
| private static final String CONTENT_TYPE = "application/x-amz-json-1.0"; | ||
| private static final OperationInfo OP_INFO = OperationInfo.builder() | ||
| .httpMethod(SdkHttpMethod.POST) | ||
| .hasImplicitPayloadMembers(true) | ||
| .build(); | ||
| private static final AwsJsonProtocolMetadata METADATA = | ||
| AwsJsonProtocolMetadata.builder() | ||
| .protocol(AwsJsonProtocol.AWS_JSON) | ||
| .contentType(CONTENT_TYPE) | ||
| .build(); | ||
|
|
||
| // ---- HEADER tests ---- | ||
|
|
||
| @Test | ||
| void header_string_producesCorrectHeader() { | ||
| SdkField<String> field = headerField("x-custom-header", obj -> "headerValue"); | ||
| SdkPojo pojo = new SimplePojo(field); | ||
|
|
||
| SdkHttpFullRequest result = createMarshaller().marshall(pojo); | ||
|
|
||
| assertThat(result.firstMatchingHeader("x-custom-header")) | ||
| .isPresent() | ||
| .hasValue("headerValue"); | ||
| } | ||
|
|
||
| @Test | ||
| void header_string_secondCall_usesCachedMarshaller() { | ||
| // Use the SAME SdkField instance for both calls so the cache is shared | ||
| SdkField<String> field = headerField("x-custom-header", obj -> "headerValue"); | ||
|
|
||
| // First call — populates the internal marshaller cache | ||
| SdkPojo pojo1 = new SimplePojo(field); | ||
| SdkHttpFullRequest result1 = createMarshaller().marshall(pojo1); | ||
|
|
||
| // Second call — should use cached marshaller | ||
| SdkPojo pojo2 = new SimplePojo(field); | ||
| SdkHttpFullRequest result2 = createMarshaller().marshall(pojo2); | ||
|
|
||
| // Both calls produce identical header output, confirming the cached path works | ||
| assertThat(result1.firstMatchingHeader("x-custom-header")) | ||
| .isPresent() | ||
| .hasValue("headerValue"); | ||
| assertThat(result2.firstMatchingHeader("x-custom-header")) | ||
| .isPresent() | ||
| .hasValue("headerValue"); | ||
| } | ||
|
|
||
| // ---- QUERY_PARAM tests ---- | ||
|
|
||
| @Test | ||
| void queryParam_string_producesCorrectQueryParam() { | ||
| SdkField<String> field = queryParamField("myParam", obj -> "paramValue"); | ||
| SdkPojo pojo = new SimplePojo(field); | ||
|
|
||
| SdkHttpFullRequest result = createMarshaller().marshall(pojo); | ||
|
|
||
| assertThat(result.rawQueryParameters().get("myParam")) | ||
| .isNotNull() | ||
| .containsExactly("paramValue"); | ||
| } | ||
|
|
||
| private static SdkField<String> headerField(String headerName, | ||
| java.util.function.Function<Object, String> getter) { | ||
| return SdkField.<String>builder(MarshallingType.STRING) | ||
| .memberName(headerName) | ||
| .getter(getter) | ||
| .setter((obj, val) -> { }) | ||
| .traits(LocationTrait.builder() | ||
| .location(MarshallLocation.HEADER) | ||
| .locationName(headerName) | ||
| .build()) | ||
| .build(); | ||
| } | ||
|
|
||
| private static SdkField<String> queryParamField(String paramName, | ||
| java.util.function.Function<Object, String> getter) { | ||
| return SdkField.<String>builder(MarshallingType.STRING) | ||
| .memberName(paramName) | ||
| .getter(getter) | ||
| .setter((obj, val) -> { }) | ||
| .traits(LocationTrait.builder() | ||
| .location(MarshallLocation.QUERY_PARAM) | ||
| .locationName(paramName) | ||
| .build()) | ||
| .build(); | ||
| } | ||
|
|
||
| private static ProtocolMarshaller<SdkHttpFullRequest> createMarshaller() { | ||
| return JsonProtocolMarshallerBuilder.create() | ||
| .endpoint(ENDPOINT) | ||
| .jsonGenerator(AwsStructuredPlainJsonFactory | ||
| .SDK_JSON_FACTORY.createWriter(CONTENT_TYPE)) | ||
| .contentType(CONTENT_TYPE) | ||
| .operationInfo(OP_INFO) | ||
| .sendExplicitNullForPayload(false) | ||
| .protocolMetadata(METADATA) | ||
| .build(); | ||
| } | ||
|
|
||
| private static final class SimplePojo implements SdkPojo { | ||
| private final List<SdkField<?>> fields; | ||
|
|
||
| SimplePojo(SdkField<?>... fields) { | ||
| this.fields = Arrays.asList(fields); | ||
| } | ||
|
|
||
| @Override | ||
| public List<SdkField<?>> sdkFields() { | ||
| return fields; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equalsBySdkFields(Object other) { | ||
| return other instanceof SimplePojo; | ||
| } | ||
|
|
||
| @Override | ||
| public Map<String, SdkField<?>> sdkFieldNameToField() { | ||
| return Collections.emptyMap(); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This technically doesnt prove that the cache is used 🙃
I'd suggest either changing the test name, or making
MARSHALLER_CACHEpackage private so we can assert something likecache.size() > 0