Skip to content

Commit 3ecd693

Browse files
alextwoodsaws-sdk-java-automation
alextwoods
and
aws-sdk-java-automation
authored
Optimize json marshaller (#6857)
* Optimize json marshaller (first pass for testing) * Refactor code to reduce duplication from switch statement * Change approaches - move cache into the marshaller instead of SDKFields * Optimize byte buffer/output stream * Optimize binary marshalling * Try new more dynamic output stream approach * Cleanups * Improve testing * Remove computeIfAbsent from the warm path * Extract SdkByteArrayOutputStream and GC optimizations to separate branch Remove optimizations 3 (SdkByteArrayOutputStream for G1GC humongous allocation avoidance) and 4 (contentStreamProvider/contentSize zero-copy streaming) from this branch. These will be reviewed separately on alexwoo/json_huge_gc_opt. This branch now contains only: - Optimization 1: Megamorphic interface dispatch fix (switch-based marshallPayloadField) - Optimization 2: Redundant registry lookup caching (marshallFieldViaRegistry with ConcurrentHashMap cache) - writeBinaryValue addition (used by SDK_BYTES switch case) * Move large SDK_BYTES test to alexwoo/json_huge_gc_opt branch The marshallPayloadField_withLargeSdkBytesValue_producesCorrectBase64 test exercises the SdkByteArrayOutputStream overflow path (200 KB payload), which belongs with the GC optimization changes. * Move writeBinaryValue and SDK_BYTES asByteArrayUnsafe optimization to GC branch Revert SDK_BYTES switch case to use the original writeValue(asByteBuffer()) path, matching the existing SimpleTypeJsonMarshaller.SDK_BYTES behavior. Remove writeBinaryValue from StructuredJsonGenerator and SdkJsonGenerator since it is no longer used on this branch. The writeBinaryValue optimization (bypassing ByteBuffer allocation and copyBytesFrom) will be introduced on alexwoo/json_huge_gc_opt alongside the other allocation-reduction changes. --------- Co-authored-by: aws-sdk-java-automation <43143862+aws-sdk-java-automation@users.noreply.github.com>
1 parent bac3224 commit 3ecd693

6 files changed

Lines changed: 1032 additions & 7 deletions

File tree

‎.changes/next-release/feature-AWSSDKforJavav2-439f346.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "feature",
3+
"category": "AWS SDK for Java v2",
4+
"contributor": "",
5+
"description": "Optimized JSON marshalling performance for JSON RPC, REST JSON and RPCv2 Cbor protocols."
6+
}

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,21 @@
530530
whose NULL marshallers handle null validation. -->
531531
<Match>
532532
<Class name="software.amazon.awssdk.protocols.json.internal.marshall.JsonProtocolMarshaller"/>
533-
<Method name="doMarshall"/>
533+
<Or>
534+
<Method name="doMarshall"/>
535+
<Method name="marshallFieldViaRegistry"/>
536+
</Or>
534537
<Bug pattern="NP_LOAD_OF_KNOWN_NULL_VALUE"/>
535538
</Match>
539+
540+
<!-- Intentional benign-race get-then-put on ConcurrentHashMap. SdkField instances are
541+
static final, and the registry always returns the same marshaller for a given
542+
(location, marshallingType) pair, so concurrent puts are idempotent. Using get()
543+
instead of computeIfAbsent() avoids the latter's bucket-level synchronization
544+
overhead on every call. -->
545+
<Match>
546+
<Class name="software.amazon.awssdk.protocols.json.internal.marshall.JsonProtocolMarshaller"/>
547+
<Method name="marshallFieldViaRegistry"/>
548+
<Bug pattern="AT_OPERATION_SEQUENCE_ON_CONCURRENT_ABSTRACTION"/>
549+
</Match>
536550
</FindBugsFilter>

‎core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java‎

Lines changed: 128 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,22 @@
2222
import static software.amazon.awssdk.http.Header.TRANSFER_ENCODING;
2323

2424
import java.io.ByteArrayInputStream;
25+
import java.math.BigDecimal;
2526
import java.net.URI;
2627
import java.nio.charset.StandardCharsets;
2728
import java.time.Instant;
2829
import java.util.Collections;
2930
import java.util.EnumMap;
31+
import java.util.List;
3032
import java.util.Map;
33+
import java.util.concurrent.ConcurrentHashMap;
3134
import software.amazon.awssdk.annotations.SdkInternalApi;
3235
import software.amazon.awssdk.core.SdkBytes;
3336
import software.amazon.awssdk.core.SdkField;
3437
import software.amazon.awssdk.core.SdkPojo;
38+
import software.amazon.awssdk.core.document.Document;
3539
import software.amazon.awssdk.core.protocol.MarshallLocation;
40+
import software.amazon.awssdk.core.protocol.MarshallingKnownType;
3641
import software.amazon.awssdk.core.protocol.MarshallingType;
3742
import software.amazon.awssdk.core.traits.PayloadTrait;
3843
import software.amazon.awssdk.core.traits.RequiredTrait;
@@ -61,6 +66,14 @@ public class JsonProtocolMarshaller implements ProtocolMarshaller<SdkHttpFullReq
6166

6267
private static final JsonMarshallerRegistry MARSHALLER_REGISTRY = createMarshallerRegistry();
6368

69+
// Caches the resolved marshaller for non-PAYLOAD fields, keyed by SdkField identity.
70+
// SdkField instances are static final per generated model class, so identity-based lookup is correct.
71+
// The cache is effectively bounded by the total number of non-payload SdkField instances across all
72+
// loaded service models — each SdkField is inserted at most once, and no eviction is needed.
73+
// ConcurrentHashMap is used for thread safety; the one-time put per SdkField is negligible.
74+
private static final ConcurrentHashMap<SdkField<?>, JsonMarshaller<Object>> MARSHALLER_CACHE =
75+
new ConcurrentHashMap<>();
76+
6477
private final URI endpoint;
6578
private final StructuredJsonGenerator jsonGenerator;
6679
private final SdkHttpFullRequest.Builder request;
@@ -214,17 +227,21 @@ void doMarshall(SdkPojo pojo) {
214227
} else if (isExplicitPayloadMember(field)) {
215228
marshallExplicitJsonPayload(field, val);
216229
} else if (val != null) {
217-
marshallField(field, val);
230+
if (field.location() == MarshallLocation.PAYLOAD) {
231+
// HOT PATH: switch-based dispatch, no registry, no interface dispatch
232+
marshallPayloadField(field, val);
233+
} else {
234+
// WARM PATH: cached registry lookup + interface dispatch
235+
marshallFieldViaRegistry(field, val);
236+
}
218237
} else if (field.location() != MarshallLocation.PAYLOAD) {
219-
// Null payload fields that aren't required are no-op in the marshaller registry.
220-
// We short circuit to avoid the registry lookup and dispatch overhead.
221-
// Non payload locations (path, header, query) have null marshallers with
222-
// different behavior, so they must still go through marshallField.
223-
marshallField(field, val);
238+
// Null non-payload: must go through registry (null marshallers vary by location)
239+
marshallFieldViaRegistry(field, val);
224240
} else if (field.containsTrait(RequiredTrait.class, TraitType.REQUIRED_TRAIT)) {
225241
throw new IllegalArgumentException(
226242
String.format("Parameter '%s' must not be null", field.locationName()));
227243
}
244+
// else: null payload field, not required → no-op
228245
}
229246
}
230247

@@ -312,6 +329,111 @@ private SdkHttpFullRequest finishMarshalling() {
312329
return request.build();
313330
}
314331

332+
/**
333+
* Marshalls a PAYLOAD-location field using a switch on {@link MarshallingKnownType} instead of
334+
* registry lookup and interface dispatch. Each case is a monomorphic call site that the JIT can inline.
335+
*/
336+
@SuppressWarnings("unchecked")
337+
private void marshallPayloadField(SdkField<?> field, Object val) {
338+
MarshallingKnownType knownType = field.marshallingType().getKnownType();
339+
if (knownType == null) {
340+
marshallFieldViaRegistry(field, val);
341+
return;
342+
}
343+
344+
StructuredJsonGenerator gen = marshallerContext.jsonGenerator();
345+
String fieldName = field.locationName();
346+
347+
switch (knownType) {
348+
case STRING:
349+
gen.writeFieldName(fieldName);
350+
gen.writeValue((String) val);
351+
break;
352+
case INTEGER:
353+
gen.writeFieldName(fieldName);
354+
gen.writeValue((int) (Integer) val);
355+
break;
356+
case LONG:
357+
gen.writeFieldName(fieldName);
358+
gen.writeValue((long) (Long) val);
359+
break;
360+
case SHORT:
361+
gen.writeFieldName(fieldName);
362+
gen.writeValue((short) (Short) val);
363+
break;
364+
case BYTE:
365+
gen.writeFieldName(fieldName);
366+
gen.writeValue((byte) (Byte) val);
367+
break;
368+
case FLOAT:
369+
gen.writeFieldName(fieldName);
370+
gen.writeValue((float) (Float) val);
371+
break;
372+
case DOUBLE:
373+
gen.writeFieldName(fieldName);
374+
gen.writeValue((double) (Double) val);
375+
break;
376+
case BIG_DECIMAL:
377+
gen.writeFieldName(fieldName);
378+
gen.writeValue((BigDecimal) val);
379+
break;
380+
case BOOLEAN:
381+
gen.writeFieldName(fieldName);
382+
gen.writeValue((boolean) (Boolean) val);
383+
break;
384+
case INSTANT:
385+
// Delegate to existing INSTANT marshaller to preserve TimestampFormatTrait handling.
386+
// Note: INSTANT marshaller writes the field name itself.
387+
SimpleTypeJsonMarshaller.INSTANT.marshall((Instant) val, marshallerContext,
388+
fieldName, (SdkField<Instant>) field);
389+
break;
390+
case SDK_BYTES:
391+
gen.writeFieldName(fieldName);
392+
gen.writeValue(((SdkBytes) val).asByteBuffer());
393+
break;
394+
case SDK_POJO:
395+
SimpleTypeJsonMarshaller.SDK_POJO.marshall((SdkPojo) val, marshallerContext,
396+
fieldName, (SdkField<SdkPojo>) field);
397+
break;
398+
case LIST:
399+
SimpleTypeJsonMarshaller.LIST.marshall((List<?>) val, marshallerContext,
400+
fieldName, (SdkField<List<?>>) field);
401+
break;
402+
case MAP:
403+
SimpleTypeJsonMarshaller.MAP.marshall((Map<String, ?>) val, marshallerContext,
404+
fieldName, (SdkField<Map<String, ?>>) field);
405+
break;
406+
case DOCUMENT:
407+
SimpleTypeJsonMarshaller.DOCUMENT.marshall((Document) val, marshallerContext,
408+
fieldName, (SdkField<Document>) field);
409+
break;
410+
default:
411+
// Unknown type — fall back to registry lookup
412+
marshallFieldViaRegistry(field, val);
413+
break;
414+
}
415+
}
416+
417+
@SuppressWarnings("unchecked")
418+
private void marshallFieldViaRegistry(SdkField<?> field, Object val) {
419+
if (val == null) {
420+
MARSHALLER_REGISTRY.getMarshaller(field.location(), field.marshallingType(), val)
421+
.marshall(val, marshallerContext, field.locationName(), (SdkField<Object>) field);
422+
return;
423+
}
424+
// Use get-before-put instead of computeIfAbsent. ConcurrentHashMap.get() is a single lock-free
425+
// volatile read, whereas computeIfAbsent() has additional overhead even on cache hits (bucket-level
426+
// synchronization bookkeeping). The benign-race on first access is safe: SdkField instances are
427+
// static final, and the registry always returns the same marshaller for a given (location, type) pair,
428+
// so concurrent puts are idempotent.
429+
JsonMarshaller<Object> marshaller = MARSHALLER_CACHE.get(field);
430+
if (marshaller == null) {
431+
marshaller = MARSHALLER_REGISTRY.getMarshaller(field.location(), field.marshallingType(), val);
432+
MARSHALLER_CACHE.put(field, marshaller);
433+
}
434+
marshaller.marshall(val, marshallerContext, field.locationName(), (SdkField<Object>) field);
435+
}
436+
315437
private void marshallField(SdkField<?> field, Object val) {
316438
MARSHALLER_REGISTRY.getMarshaller(field.location(), field.marshallingType(), val)
317439
.marshall(val, marshallerContext, field.locationName(), (SdkField<Object>) field);

‎core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/CachedNonPayloadMarshallingTest.java‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
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.protocols.json.internal.marshall;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
20+
import java.net.URI;
21+
import java.util.Arrays;
22+
import java.util.Collections;
23+
import java.util.List;
24+
import java.util.Map;
25+
import org.junit.jupiter.api.Test;
26+
import software.amazon.awssdk.core.SdkField;
27+
import software.amazon.awssdk.core.SdkPojo;
28+
import software.amazon.awssdk.core.protocol.MarshallLocation;
29+
import software.amazon.awssdk.core.protocol.MarshallingType;
30+
import software.amazon.awssdk.core.traits.LocationTrait;
31+
import software.amazon.awssdk.http.SdkHttpFullRequest;
32+
import software.amazon.awssdk.http.SdkHttpMethod;
33+
import software.amazon.awssdk.protocols.core.OperationInfo;
34+
import software.amazon.awssdk.protocols.core.ProtocolMarshaller;
35+
import software.amazon.awssdk.protocols.json.AwsJsonProtocol;
36+
import software.amazon.awssdk.protocols.json.AwsJsonProtocolMetadata;
37+
import software.amazon.awssdk.protocols.json.internal.AwsStructuredPlainJsonFactory;
38+
39+
/**
40+
* Tests that the cached non-payload marshalling path in
41+
* {@link JsonProtocolMarshaller#marshallFieldViaRegistry} produces correct output
42+
* and that the cache is populated after the first call.
43+
*/
44+
class CachedNonPayloadMarshallingTest {
45+
46+
private static final URI ENDPOINT = URI.create("http://localhost");
47+
private static final String CONTENT_TYPE = "application/x-amz-json-1.0";
48+
private static final OperationInfo OP_INFO = OperationInfo.builder()
49+
.httpMethod(SdkHttpMethod.POST)
50+
.hasImplicitPayloadMembers(true)
51+
.build();
52+
private static final AwsJsonProtocolMetadata METADATA =
53+
AwsJsonProtocolMetadata.builder()
54+
.protocol(AwsJsonProtocol.AWS_JSON)
55+
.contentType(CONTENT_TYPE)
56+
.build();
57+
58+
// ---- HEADER tests ----
59+
60+
@Test
61+
void header_string_producesCorrectHeader() {
62+
SdkField<String> field = headerField("x-custom-header", obj -> "headerValue");
63+
SdkPojo pojo = new SimplePojo(field);
64+
65+
SdkHttpFullRequest result = createMarshaller().marshall(pojo);
66+
67+
assertThat(result.firstMatchingHeader("x-custom-header"))
68+
.isPresent()
69+
.hasValue("headerValue");
70+
}
71+
72+
@Test
73+
void header_string_secondCall_usesCachedMarshaller() {
74+
// Use the SAME SdkField instance for both calls so the cache is shared
75+
SdkField<String> field = headerField("x-custom-header", obj -> "headerValue");
76+
77+
// First call — populates the internal marshaller cache
78+
SdkPojo pojo1 = new SimplePojo(field);
79+
SdkHttpFullRequest result1 = createMarshaller().marshall(pojo1);
80+
81+
// Second call — should use cached marshaller
82+
SdkPojo pojo2 = new SimplePojo(field);
83+
SdkHttpFullRequest result2 = createMarshaller().marshall(pojo2);
84+
85+
// Both calls produce identical header output, confirming the cached path works
86+
assertThat(result1.firstMatchingHeader("x-custom-header"))
87+
.isPresent()
88+
.hasValue("headerValue");
89+
assertThat(result2.firstMatchingHeader("x-custom-header"))
90+
.isPresent()
91+
.hasValue("headerValue");
92+
}
93+
94+
// ---- QUERY_PARAM tests ----
95+
96+
@Test
97+
void queryParam_string_producesCorrectQueryParam() {
98+
SdkField<String> field = queryParamField("myParam", obj -> "paramValue");
99+
SdkPojo pojo = new SimplePojo(field);
100+
101+
SdkHttpFullRequest result = createMarshaller().marshall(pojo);
102+
103+
assertThat(result.rawQueryParameters().get("myParam"))
104+
.isNotNull()
105+
.containsExactly("paramValue");
106+
}
107+
108+
private static SdkField<String> headerField(String headerName,
109+
java.util.function.Function<Object, String> getter) {
110+
return SdkField.<String>builder(MarshallingType.STRING)
111+
.memberName(headerName)
112+
.getter(getter)
113+
.setter((obj, val) -> { })
114+
.traits(LocationTrait.builder()
115+
.location(MarshallLocation.HEADER)
116+
.locationName(headerName)
117+
.build())
118+
.build();
119+
}
120+
121+
private static SdkField<String> queryParamField(String paramName,
122+
java.util.function.Function<Object, String> getter) {
123+
return SdkField.<String>builder(MarshallingType.STRING)
124+
.memberName(paramName)
125+
.getter(getter)
126+
.setter((obj, val) -> { })
127+
.traits(LocationTrait.builder()
128+
.location(MarshallLocation.QUERY_PARAM)
129+
.locationName(paramName)
130+
.build())
131+
.build();
132+
}
133+
134+
private static ProtocolMarshaller<SdkHttpFullRequest> createMarshaller() {
135+
return JsonProtocolMarshallerBuilder.create()
136+
.endpoint(ENDPOINT)
137+
.jsonGenerator(AwsStructuredPlainJsonFactory
138+
.SDK_JSON_FACTORY.createWriter(CONTENT_TYPE))
139+
.contentType(CONTENT_TYPE)
140+
.operationInfo(OP_INFO)
141+
.sendExplicitNullForPayload(false)
142+
.protocolMetadata(METADATA)
143+
.build();
144+
}
145+
146+
private static final class SimplePojo implements SdkPojo {
147+
private final List<SdkField<?>> fields;
148+
149+
SimplePojo(SdkField<?>... fields) {
150+
this.fields = Arrays.asList(fields);
151+
}
152+
153+
@Override
154+
public List<SdkField<?>> sdkFields() {
155+
return fields;
156+
}
157+
158+
@Override
159+
public boolean equalsBySdkFields(Object other) {
160+
return other instanceof SimplePojo;
161+
}
162+
163+
@Override
164+
public Map<String, SdkField<?>> sdkFieldNameToField() {
165+
return Collections.emptyMap();
166+
}
167+
}
168+
}

0 commit comments

Comments
 (0)