From 2605aaf6e81d512c03d4bd7e6f700d84e8e1289f Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Sat, 11 Apr 2026 10:44:25 -0700 Subject: [PATCH 01/12] Optimize json marshaller (first pass for testing) --- .../amazon/awssdk/spotbugs-suppressions.xml | 5 +- .../marshall/JsonProtocolMarshaller.java | 143 ++++- .../CachedNonPayloadMarshallingTest.java | 188 ++++++ .../PayloadMarshallingEquivalenceTest.java | 580 ++++++++++++++++++ ...knownMarshallingKnownTypeFallbackTest.java | 202 ++++++ .../software/amazon/awssdk/core/SdkField.java | 30 + .../core/SdkFieldCacheMarshallerTest.java | 117 ++++ 7 files changed, 1258 insertions(+), 7 deletions(-) create mode 100644 core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/CachedNonPayloadMarshallingTest.java create mode 100644 core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/PayloadMarshallingEquivalenceTest.java create mode 100644 core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/UnknownMarshallingKnownTypeFallbackTest.java create mode 100644 core/sdk-core/src/test/java/software/amazon/awssdk/core/SdkFieldCacheMarshallerTest.java diff --git a/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml b/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml index 69a7894bfe94..f6704386ca2a 100644 --- a/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml +++ b/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml @@ -528,7 +528,10 @@ whose NULL marshallers handle null validation. --> - + + + + diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java index c76aa851d997..d62a254d8cc0 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java @@ -22,22 +22,28 @@ import static software.amazon.awssdk.http.Header.TRANSFER_ENCODING; import java.io.ByteArrayInputStream; +import java.math.BigDecimal; import java.net.URI; import java.nio.charset.StandardCharsets; import java.time.Instant; import java.util.Collections; import java.util.EnumMap; +import java.util.List; import java.util.Map; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.core.SdkBytes; import software.amazon.awssdk.core.SdkField; import software.amazon.awssdk.core.SdkPojo; +import software.amazon.awssdk.core.document.Document; import software.amazon.awssdk.core.protocol.MarshallLocation; +import software.amazon.awssdk.core.protocol.MarshallingKnownType; import software.amazon.awssdk.core.protocol.MarshallingType; import software.amazon.awssdk.core.traits.PayloadTrait; import software.amazon.awssdk.core.traits.RequiredTrait; import software.amazon.awssdk.core.traits.TimestampFormatTrait; import software.amazon.awssdk.core.traits.TraitType; +import software.amazon.awssdk.core.util.SdkAutoConstructList; +import software.amazon.awssdk.core.util.SdkAutoConstructMap; import software.amazon.awssdk.http.SdkHttpFullRequest; import software.amazon.awssdk.protocols.core.InstantToString; import software.amazon.awssdk.protocols.core.OperationInfo; @@ -214,17 +220,21 @@ void doMarshall(SdkPojo pojo) { } else if (isExplicitPayloadMember(field)) { marshallExplicitJsonPayload(field, val); } else if (val != null) { - marshallField(field, val); + if (field.location() == MarshallLocation.PAYLOAD) { + // HOT PATH: switch-based dispatch, no registry, no interface dispatch + marshallPayloadField(field, val); + } else { + // WARM PATH: cached registry lookup + interface dispatch + marshallFieldViaRegistry(field, val); + } } else if (field.location() != MarshallLocation.PAYLOAD) { - // Null payload fields that aren't required are no-op in the marshaller registry. - // We short circuit to avoid the registry lookup and dispatch overhead. - // Non payload locations (path, header, query) have null marshallers with - // different behavior, so they must still go through marshallField. - marshallField(field, val); + // Null non-payload: must go through registry (null marshallers vary by location) + marshallFieldViaRegistry(field, val); } else if (field.containsTrait(RequiredTrait.class, TraitType.REQUIRED_TRAIT)) { throw new IllegalArgumentException( String.format("Parameter '%s' must not be null", field.locationName())); } + // else: null payload field, not required → no-op } } @@ -312,6 +322,127 @@ private SdkHttpFullRequest finishMarshalling() { return request.build(); } + /** + * Marshalls a PAYLOAD-location field using a switch on {@link MarshallingKnownType} instead of + * registry lookup and interface dispatch. Each case is a monomorphic call site that the JIT can inline. + */ + @SuppressWarnings("unchecked") + private void marshallPayloadField(SdkField field, Object val) { + MarshallingKnownType knownType = field.marshallingType().getKnownType(); + if (knownType == null) { + marshallFieldViaRegistry(field, val); + return; + } + + StructuredJsonGenerator gen = marshallerContext.jsonGenerator(); + String fieldName = field.locationName(); + + switch (knownType) { + case STRING: + gen.writeFieldName(fieldName); + gen.writeValue((String) val); + break; + case INTEGER: + gen.writeFieldName(fieldName); + gen.writeValue((int) (Integer) val); + break; + case LONG: + gen.writeFieldName(fieldName); + gen.writeValue((long) (Long) val); + break; + case SHORT: + gen.writeFieldName(fieldName); + gen.writeValue((short) (Short) val); + break; + case BYTE: + gen.writeFieldName(fieldName); + gen.writeValue((byte) (Byte) val); + break; + case FLOAT: + gen.writeFieldName(fieldName); + gen.writeValue((float) (Float) val); + break; + case DOUBLE: + gen.writeFieldName(fieldName); + gen.writeValue((double) (Double) val); + break; + case BIG_DECIMAL: + gen.writeFieldName(fieldName); + gen.writeValue((BigDecimal) val); + break; + case BOOLEAN: + gen.writeFieldName(fieldName); + gen.writeValue((boolean) (Boolean) val); + break; + case INSTANT: + // Delegate to existing INSTANT marshaller to preserve TimestampFormatTrait handling. + // Note: INSTANT marshaller writes the field name itself. + SimpleTypeJsonMarshaller.INSTANT.marshall((Instant) val, marshallerContext, + fieldName, (SdkField) field); + break; + case SDK_BYTES: + gen.writeFieldName(fieldName); + gen.writeValue(((SdkBytes) val).asByteBuffer()); + break; + case SDK_POJO: + gen.writeFieldName(fieldName); + gen.writeStartObject(); + doMarshall((SdkPojo) val); + gen.writeEndObject(); + break; + case LIST: + List list = (List) val; + if (list.isEmpty() && list instanceof SdkAutoConstructList) { + break; + } + gen.writeFieldName(fieldName); + gen.writeStartArray(list.size()); + for (Object item : list) { + marshallerContext.marshall(MarshallLocation.PAYLOAD, item); + } + gen.writeEndArray(); + break; + case MAP: + Map map = (Map) val; + if (map.isEmpty() && map instanceof SdkAutoConstructMap) { + break; + } + gen.writeFieldName(fieldName); + gen.writeStartObject(); + for (Map.Entry entry : map.entrySet()) { + if (entry.getValue() != null) { + gen.writeFieldName(entry.getKey()); + marshallerContext.marshall(MarshallLocation.PAYLOAD, entry.getValue()); + } + } + gen.writeEndObject(); + break; + case DOCUMENT: + gen.writeFieldName(fieldName); + ((Document) val).accept(new DocumentTypeJsonMarshaller(gen)); + break; + default: + // Unknown type — fall back to registry lookup + marshallFieldViaRegistry(field, val); + break; + } + } + + @SuppressWarnings("unchecked") + private void marshallFieldViaRegistry(SdkField field, Object val) { + if (val == null) { + MARSHALLER_REGISTRY.getMarshaller(field.location(), field.marshallingType(), val) + .marshall(val, marshallerContext, field.locationName(), (SdkField) field); + return; + } + JsonMarshaller marshaller = field.cachedMarshaller(MARSHALLER_REGISTRY); + if (marshaller == null) { + marshaller = MARSHALLER_REGISTRY.getMarshaller(field.location(), field.marshallingType(), val); + field.cacheMarshaller(MARSHALLER_REGISTRY, marshaller); + } + marshaller.marshall(val, marshallerContext, field.locationName(), (SdkField) field); + } + private void marshallField(SdkField field, Object val) { MARSHALLER_REGISTRY.getMarshaller(field.location(), field.marshallingType(), val) .marshall(val, marshallerContext, field.locationName(), (SdkField) field); diff --git a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/CachedNonPayloadMarshallingTest.java b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/CachedNonPayloadMarshallingTest.java new file mode 100644 index 000000000000..2b4e559a518e --- /dev/null +++ b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/CachedNonPayloadMarshallingTest.java @@ -0,0 +1,188 @@ +/* + * 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. + * + *

Validates: Property 3 — Cached non-payload marshalling equivalence

+ *

Validates: Requirements 7.3, 7.4

+ */ +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 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 field = headerField("x-custom-header", obj -> "headerValue"); + + // First call — populates the cache + SdkPojo pojo1 = new SimplePojo(field); + SdkHttpFullRequest result1 = createMarshaller().marshall(pojo1); + + // After first marshalling, the cache should be populated on the SdkField. + // We can't access the exact registry key, but we can verify the field has + // a non-null cached marshaller by checking that a second marshalling produces + // identical output. + + // Second call — should use cached marshaller + SdkPojo pojo2 = new SimplePojo(field); + SdkHttpFullRequest result2 = createMarshaller().marshall(pojo2); + + // Both calls produce identical header output + assertThat(result1.firstMatchingHeader("x-custom-header")) + .isPresent() + .hasValue("headerValue"); + assertThat(result2.firstMatchingHeader("x-custom-header")) + .isPresent() + .hasValue("headerValue"); + + // Verify the cache was populated: the field should have a non-null cached + // marshaller for at least one registry key. Since we can't access the private + // MARSHALLER_REGISTRY, we verify indirectly: the field's cachedMarshaller + // with a dummy key returns null (different key), but the fact that both calls + // succeeded with identical output confirms the cached path works. + Object cachedWithDifferentKey = field.cachedMarshaller(new Object()); + assertThat(cachedWithDifferentKey) + .as("Different registry key should return null") + .isNull(); + } + + // ---- QUERY_PARAM tests ---- + + @Test + void queryParam_string_producesCorrectQueryParam() { + SdkField field = queryParamField("myParam", obj -> "paramValue"); + SdkPojo pojo = new SimplePojo(field); + + SdkHttpFullRequest result = createMarshaller().marshall(pojo); + + assertThat(result.rawQueryParameters().get("myParam")) + .isNotNull() + .containsExactly("paramValue"); + } + + // ---- Helper methods ---- + + private static SdkField headerField(String headerName, + java.util.function.Function getter) { + return SdkField.builder(MarshallingType.STRING) + .memberName(headerName) + .getter(getter) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.HEADER) + .locationName(headerName) + .build()) + .build(); + } + + private static SdkField queryParamField(String paramName, + java.util.function.Function getter) { + return SdkField.builder(MarshallingType.STRING) + .memberName(paramName) + .getter(getter) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.QUERY_PARAM) + .locationName(paramName) + .build()) + .build(); + } + + private static ProtocolMarshaller 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> fields; + + SimplePojo(SdkField... fields) { + this.fields = Arrays.asList(fields); + } + + @Override + public List> sdkFields() { + return fields; + } + + @Override + public boolean equalsBySdkFields(Object other) { + return other instanceof SimplePojo; + } + + @Override + public Map> sdkFieldNameToField() { + return Collections.emptyMap(); + } + } +} diff --git a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/PayloadMarshallingEquivalenceTest.java b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/PayloadMarshallingEquivalenceTest.java new file mode 100644 index 000000000000..a83192056584 --- /dev/null +++ b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/PayloadMarshallingEquivalenceTest.java @@ -0,0 +1,580 @@ +/* + * 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.math.BigDecimal; +import java.net.URI; +import java.time.Instant; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Function; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.core.SdkBytes; +import software.amazon.awssdk.core.SdkField; +import software.amazon.awssdk.core.SdkPojo; +import software.amazon.awssdk.core.document.Document; +import software.amazon.awssdk.core.protocol.MarshallLocation; +import software.amazon.awssdk.core.protocol.MarshallingType; +import software.amazon.awssdk.core.traits.ListTrait; +import software.amazon.awssdk.core.traits.LocationTrait; +import software.amazon.awssdk.core.traits.MapTrait; +import software.amazon.awssdk.core.traits.TimestampFormatTrait; +import software.amazon.awssdk.core.util.DefaultSdkAutoConstructList; +import software.amazon.awssdk.core.util.DefaultSdkAutoConstructMap; +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 switch-based payload dispatch in {@link JsonProtocolMarshaller#marshallPayloadField} + * produces correct JSON output for all 16 {@code MarshallingKnownType} values. + * + *

Validates: Property 1 — Payload marshalling behavioral equivalence

+ *

Validates: Requirements 2.1–2.12, 3.1–3.5, 4.1, 5.1–5.3, 6.1–6.4

+ */ +class PayloadMarshallingEquivalenceTest { + + 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(); + + // ---- STRING ---- + + @Test + void string_producesCorrectJson() { + SdkField field = payloadField("fieldName", MarshallingType.STRING, obj -> "hello world"); + String body = marshallAndGetBody(field); + assertThat(body).contains("\"fieldName\":\"hello world\""); + } + + // ---- INTEGER ---- + + @Test + void integer_producesCorrectJson() { + SdkField field = payloadField("fieldName", MarshallingType.INTEGER, obj -> 42); + String body = marshallAndGetBody(field); + assertThat(body).contains("\"fieldName\":42"); + } + + // ---- LONG ---- + + @Test + void long_producesCorrectJson() { + SdkField field = payloadField("fieldName", MarshallingType.LONG, obj -> 123456789L); + String body = marshallAndGetBody(field); + assertThat(body).contains("\"fieldName\":123456789"); + } + + // ---- SHORT ---- + + @Test + void short_producesCorrectJson() { + SdkField field = payloadField("fieldName", MarshallingType.SHORT, obj -> (short) 7); + String body = marshallAndGetBody(field); + assertThat(body).contains("\"fieldName\":7"); + } + + // ---- BYTE ---- + + @Test + void byte_producesCorrectJson() { + SdkField field = payloadField("fieldName", MarshallingType.BYTE, obj -> (byte) 3); + String body = marshallAndGetBody(field); + assertThat(body).contains("\"fieldName\":3"); + } + + // ---- FLOAT ---- + + @Test + void float_producesCorrectJson() { + SdkField field = payloadField("fieldName", MarshallingType.FLOAT, obj -> 1.5f); + String body = marshallAndGetBody(field); + assertThat(body).contains("\"fieldName\":1.5"); + } + + // ---- DOUBLE ---- + + @Test + void double_producesCorrectJson() { + SdkField field = payloadField("fieldName", MarshallingType.DOUBLE, obj -> 3.14); + String body = marshallAndGetBody(field); + assertThat(body).contains("\"fieldName\":3.14"); + } + + // ---- BIG_DECIMAL ---- + + @Test + void bigDecimal_producesCorrectJson() { + SdkField field = payloadField("fieldName", MarshallingType.BIG_DECIMAL, + obj -> new BigDecimal("99.99")); + String body = marshallAndGetBody(field); + // BigDecimal is serialized as a quoted string by the JSON generator + assertThat(body).contains("\"fieldName\":\"99.99\""); + } + + // ---- BOOLEAN ---- + + @Test + void boolean_producesCorrectJson() { + SdkField field = payloadField("fieldName", MarshallingType.BOOLEAN, obj -> true); + String body = marshallAndGetBody(field); + assertThat(body).contains("\"fieldName\":true"); + } + + // ---- INSTANT (default format — UNIX_TIMESTAMP for PAYLOAD) ---- + + @Test + void instant_defaultFormat_producesUnixTimestamp() { + SdkField field = payloadField("fieldName", MarshallingType.INSTANT, + obj -> Instant.ofEpochSecond(1000)); + String body = marshallAndGetBody(field); + // Default PAYLOAD format is UNIX_TIMESTAMP — written via jsonGenerator.writeValue(Instant) + // which for plain JSON writes epoch seconds (e.g. 1000.0 or 1000) + assertThat(body).contains("\"fieldName\":"); + assertThat(body).contains("1000"); + } + + // ---- INSTANT with UNIX_TIMESTAMP trait ---- + + @Test + void instant_unixTimestampTrait_producesUnixTimestamp() { + SdkField field = SdkField.builder(MarshallingType.INSTANT) + .memberName("fieldName") + .getter(obj -> Instant.ofEpochSecond(1000)) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("fieldName") + .build(), + TimestampFormatTrait.create(TimestampFormatTrait.Format.UNIX_TIMESTAMP)) + .build(); + String body = marshallAndGetBody(field); + assertThat(body).contains("\"fieldName\":"); + assertThat(body).contains("1000"); + } + + // ---- INSTANT with RFC_822 trait ---- + + @Test + void instant_rfc822Trait_producesRfc822String() { + SdkField field = SdkField.builder(MarshallingType.INSTANT) + .memberName("fieldName") + .getter(obj -> Instant.ofEpochSecond(1000)) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("fieldName") + .build(), + TimestampFormatTrait.create(TimestampFormatTrait.Format.RFC_822)) + .build(); + String body = marshallAndGetBody(field); + // RFC 822 format: e.g. "Thu, 01 Jan 1970 00:16:40 GMT" + assertThat(body).contains("\"fieldName\":\""); + assertThat(body).contains("1970"); + } + + // ---- INSTANT with ISO_8601 trait ---- + + @Test + void instant_iso8601Trait_producesIso8601String() { + SdkField field = SdkField.builder(MarshallingType.INSTANT) + .memberName("fieldName") + .getter(obj -> Instant.ofEpochSecond(1000)) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("fieldName") + .build(), + TimestampFormatTrait.create(TimestampFormatTrait.Format.ISO_8601)) + .build(); + String body = marshallAndGetBody(field); + // ISO 8601 format: e.g. "1970-01-01T00:16:40Z" + assertThat(body).contains("\"fieldName\":\""); + assertThat(body).contains("1970-01-01T"); + } + + // ---- SDK_BYTES ---- + + @Test + void sdkBytes_producesBase64EncodedJson() { + SdkField field = payloadField("fieldName", MarshallingType.SDK_BYTES, + obj -> SdkBytes.fromUtf8String("data")); + String body = marshallAndGetBody(field); + // "data" base64 encoded is "ZGF0YQ==" + assertThat(body).contains("\"fieldName\":\"ZGF0YQ==\""); + } + + // ---- SDK_POJO (nested) ---- + + @Test + void sdkPojo_producesNestedObjectJson() { + // Inner pojo with a single string field + SdkField innerField = payloadField("innerField", MarshallingType.STRING, obj -> "innerValue"); + SimplePojo innerPojo = new SimplePojo(innerField); + + SdkField outerField = SdkField.builder(MarshallingType.SDK_POJO) + .memberName("fieldName") + .getter(obj -> innerPojo) + .setter((obj, val) -> { }) + .constructor(() -> innerPojo) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("fieldName") + .build()) + .build(); + + String body = marshallAndGetBody(outerField); + assertThat(body).contains("\"fieldName\":{\"innerField\":\"innerValue\"}"); + } + + // ---- LIST (non-empty) ---- + + @Test + void list_nonEmpty_producesArrayJson() { + List listValue = Arrays.asList("a", "b", "c"); + + SdkField memberField = SdkField.builder(MarshallingType.STRING) + .memberName("member") + .getter(obj -> null) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("member") + .build()) + .build(); + + SdkField> field = SdkField.>builder(MarshallingType.LIST) + .memberName("fieldName") + .getter(obj -> listValue) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("fieldName") + .build(), + ListTrait.builder() + .memberFieldInfo(memberField) + .build()) + .build(); + + String body = marshallAndGetBody(field); + assertThat(body).contains("\"fieldName\":[\"a\",\"b\",\"c\"]"); + } + + // ---- LIST (empty SdkAutoConstructList — should be skipped) ---- + + @Test + void list_emptySdkAutoConstructList_isSkipped() { + List autoList = DefaultSdkAutoConstructList.getInstance(); + + SdkField memberField = SdkField.builder(MarshallingType.STRING) + .memberName("member") + .getter(obj -> null) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("member") + .build()) + .build(); + + SdkField> field = SdkField.>builder(MarshallingType.LIST) + .memberName("fieldName") + .getter(obj -> autoList) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("fieldName") + .build(), + ListTrait.builder() + .memberFieldInfo(memberField) + .build()) + .build(); + + String body = marshallAndGetBody(field); + assertThat(body).doesNotContain("fieldName"); + } + + // ---- LIST (empty regular list — should emit empty array) ---- + + @Test + void list_emptyRegularList_producesEmptyArray() { + List emptyList = new ArrayList<>(); + + SdkField memberField = SdkField.builder(MarshallingType.STRING) + .memberName("member") + .getter(obj -> null) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("member") + .build()) + .build(); + + SdkField> field = SdkField.>builder(MarshallingType.LIST) + .memberName("fieldName") + .getter(obj -> emptyList) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("fieldName") + .build(), + ListTrait.builder() + .memberFieldInfo(memberField) + .build()) + .build(); + + String body = marshallAndGetBody(field); + assertThat(body).contains("\"fieldName\":[]"); + } + + // ---- MAP (non-empty) ---- + + @Test + void map_nonEmpty_producesObjectJson() { + // Use LinkedHashMap for deterministic ordering + Map mapValue = new LinkedHashMap<>(); + mapValue.put("key1", "val1"); + mapValue.put("key2", "val2"); + + SdkField valueField = SdkField.builder(MarshallingType.STRING) + .memberName("value") + .getter(obj -> null) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("value") + .build()) + .build(); + + SdkField> field = SdkField.>builder(MarshallingType.MAP) + .memberName("fieldName") + .getter(obj -> mapValue) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("fieldName") + .build(), + MapTrait.builder() + .valueFieldInfo(valueField) + .build()) + .build(); + + String body = marshallAndGetBody(field); + assertThat(body).contains("\"fieldName\":{\"key1\":\"val1\",\"key2\":\"val2\"}"); + } + + // ---- MAP (empty SdkAutoConstructMap — should be skipped) ---- + + @Test + void map_emptySdkAutoConstructMap_isSkipped() { + Map autoMap = DefaultSdkAutoConstructMap.getInstance(); + + SdkField valueField = SdkField.builder(MarshallingType.STRING) + .memberName("value") + .getter(obj -> null) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("value") + .build()) + .build(); + + SdkField> field = SdkField.>builder(MarshallingType.MAP) + .memberName("fieldName") + .getter(obj -> autoMap) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("fieldName") + .build(), + MapTrait.builder() + .valueFieldInfo(valueField) + .build()) + .build(); + + String body = marshallAndGetBody(field); + assertThat(body).doesNotContain("fieldName"); + } + + // ---- MAP (empty regular map — should emit empty object) ---- + + @Test + void map_emptyRegularMap_producesEmptyObject() { + Map emptyMap = new HashMap<>(); + + SdkField valueField = SdkField.builder(MarshallingType.STRING) + .memberName("value") + .getter(obj -> null) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("value") + .build()) + .build(); + + SdkField> field = SdkField.>builder(MarshallingType.MAP) + .memberName("fieldName") + .getter(obj -> emptyMap) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("fieldName") + .build(), + MapTrait.builder() + .valueFieldInfo(valueField) + .build()) + .build(); + + String body = marshallAndGetBody(field); + assertThat(body).contains("\"fieldName\":{}"); + } + + // ---- MAP with null value entry — entry is skipped ---- + + @Test + void map_nullValueEntry_isSkipped() { + Map mapValue = new LinkedHashMap<>(); + mapValue.put("key1", "val1"); + mapValue.put("key2", null); + mapValue.put("key3", "val3"); + + SdkField valueField = SdkField.builder(MarshallingType.STRING) + .memberName("value") + .getter(obj -> null) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("value") + .build()) + .build(); + + SdkField> field = SdkField.>builder(MarshallingType.MAP) + .memberName("fieldName") + .getter(obj -> mapValue) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("fieldName") + .build(), + MapTrait.builder() + .valueFieldInfo(valueField) + .build()) + .build(); + + String body = marshallAndGetBody(field); + assertThat(body).contains("\"key1\":\"val1\""); + assertThat(body).doesNotContain("key2"); + assertThat(body).contains("\"key3\":\"val3\""); + } + + // ---- DOCUMENT ---- + + @Test + void document_producesCorrectJson() { + SdkField field = payloadField("fieldName", MarshallingType.DOCUMENT, + obj -> Document.fromString("test")); + String body = marshallAndGetBody(field); + assertThat(body).contains("\"fieldName\":\"test\""); + } + + // ---- Helper methods ---- + + @SuppressWarnings({"unchecked", "rawtypes"}) + private static SdkField payloadField(String name, + MarshallingType marshallingType, + Function getter) { + return (SdkField) SdkField.builder(marshallingType) + .memberName(name) + .getter((Function) getter) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName(name) + .build()) + .build(); + } + + private String marshallAndGetBody(SdkField... fields) { + SdkPojo pojo = new SimplePojo(fields); + SdkHttpFullRequest result = createMarshaller().marshall(pojo); + return bodyAsString(result); + } + + private static ProtocolMarshaller 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 String bodyAsString(SdkHttpFullRequest request) { + return request.contentStreamProvider() + .map(p -> { + try { + return software.amazon.awssdk.utils.IoUtils.toUtf8String(p.newStream()); + } catch (Exception e) { + throw new RuntimeException(e); + } + }) + .orElse(""); + } + + private static final class SimplePojo implements SdkPojo { + private final List> fields; + + SimplePojo(SdkField... fields) { + this.fields = Arrays.asList(fields); + } + + @Override + public List> sdkFields() { + return fields; + } + + @Override + public boolean equalsBySdkFields(Object other) { + return other instanceof SimplePojo; + } + + @Override + public Map> sdkFieldNameToField() { + return Collections.emptyMap(); + } + } +} diff --git a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/UnknownMarshallingKnownTypeFallbackTest.java b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/UnknownMarshallingKnownTypeFallbackTest.java new file mode 100644 index 000000000000..6886452c2dc1 --- /dev/null +++ b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/UnknownMarshallingKnownTypeFallbackTest.java @@ -0,0 +1,202 @@ +/* + * 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 static org.assertj.core.api.Assertions.assertThatThrownBy; + +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.MarshallingKnownType; +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 when {@code getKnownType()} returns null, the marshaller falls back to the + * registry-based path without throwing a {@link NullPointerException} from the switch statement. + * + *

Validates: Requirements 1.3, 1.4

+ */ +class UnknownMarshallingKnownTypeFallbackTest { + + 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(); + + /** + * A custom MarshallingType whose {@code getKnownType()} returns null. + * This simulates a future or third-party MarshallingType that is not in the known enum set. + */ + private static final MarshallingType CUSTOM_NULL_KNOWN_TYPE = new MarshallingType() { + @Override + public Class getTargetClass() { + return String.class; + } + + @Override + public MarshallingKnownType getKnownType() { + return null; + } + + @Override + public String toString() { + return "CUSTOM_NULL_KNOWN_TYPE"; + } + }; + + /** + * Validates Requirement 1.4: When {@code getKnownType()} returns null, the marshaller falls back + * to the registry-based path without throwing a NullPointerException from the switch statement. + * + *

Since the custom type is not registered in the static MARSHALLER_REGISTRY, the registry + * fallback will fail — but the failure must NOT be a NullPointerException from the switch. + * It should be a NullPointerException from invoking {@code .marshall()} on the null result + * returned by the registry lookup (since the custom type is unregistered).

+ */ + @Test + void nullKnownType_fallsBackToRegistryPath_doesNotThrowNpeFromSwitch() { + SdkField field = SdkField.builder(CUSTOM_NULL_KNOWN_TYPE) + .memberName("customField") + .getter(obj -> "someValue") + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("customField") + .build()) + .build(); + + SdkPojo pojo = new SimplePojo(field); + + // The null-knownType guard in marshallPayloadField should redirect to marshallFieldViaRegistry. + // Since CUSTOM_NULL_KNOWN_TYPE is not registered in the static MARSHALLER_REGISTRY, + // the registry returns null and a NullPointerException occurs when invoking .marshall() on it. + // The critical assertion: the NPE stack trace must NOT originate from the switch statement + // in marshallPayloadField — it must come from the registry fallback path. + assertThatThrownBy(() -> createMarshaller().marshall(pojo)) + .isInstanceOf(NullPointerException.class) + .satisfies(thrown -> { + // Verify the NPE comes from marshallFieldViaRegistry (the fallback), + // not from marshallPayloadField's switch statement + StackTraceElement[] stack = thrown.getStackTrace(); + boolean fromRegistryPath = false; + for (StackTraceElement element : stack) { + if ("marshallFieldViaRegistry".equals(element.getMethodName())) { + fromRegistryPath = true; + break; + } + } + assertThat(fromRegistryPath) + .as("NPE should originate from marshallFieldViaRegistry (registry fallback), " + + "not from the switch in marshallPayloadField") + .isTrue(); + }); + } + + /** + * Validates Requirement 1.3: A standard MarshallingType (STRING) with a known type is handled + * by the switch path, confirming the switch dispatch works for recognized types. + * This serves as a control test — if the switch were broken, this would fail too. + */ + @Test + void knownType_string_isHandledBySwitchPath() { + SdkField field = SdkField.builder(MarshallingType.STRING) + .memberName("normalField") + .getter(obj -> "hello") + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("normalField") + .build()) + .build(); + + SdkPojo pojo = new SimplePojo(field); + + SdkHttpFullRequest result = createMarshaller().marshall(pojo); + String body = bodyAsString(result); + assertThat(body).contains("\"normalField\":\"hello\""); + } + + // ---- Helper methods ---- + + private static ProtocolMarshaller 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 String bodyAsString(SdkHttpFullRequest request) { + return request.contentStreamProvider() + .map(p -> { + try { + return software.amazon.awssdk.utils.IoUtils.toUtf8String(p.newStream()); + } catch (Exception e) { + throw new RuntimeException(e); + } + }) + .orElse(""); + } + + private static final class SimplePojo implements SdkPojo { + private final List> fields; + + SimplePojo(SdkField... fields) { + this.fields = Arrays.asList(fields); + } + + @Override + public List> sdkFields() { + return fields; + } + + @Override + public boolean equalsBySdkFields(Object other) { + return other instanceof SimplePojo; + } + + @Override + public Map> sdkFieldNameToField() { + return Collections.emptyMap(); + } + } +} diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkField.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkField.java index 98561baca4ac..fc4f81038f26 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkField.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkField.java @@ -50,6 +50,9 @@ public final class SdkField { private final Map l1Traits; private final Map, Trait> l2Traits; + private volatile Object cachedMarshaller; + private volatile Object cachedMarshallerRegistryKey; + private SdkField(Builder builder) { this.memberName = builder.memberName; this.marshallingType = builder.marshallingType; @@ -253,6 +256,33 @@ public boolean containsTrait(Class clzz, TraitType type) { return getTrait(clzz, type) != null; } + /** + * Returns the cached marshaller for the given registry key, or null if not cached. + * Uses reference identity ({@code ==}) for the registry key comparison. + * + * @param registryKey The registry key to match against the cached key. + * @param The type of the cached marshaller. + * @return The cached marshaller if the registry key matches, or null. + */ + @SuppressWarnings("unchecked") + public T cachedMarshaller(Object registryKey) { + if (cachedMarshallerRegistryKey == registryKey) { + return (T) cachedMarshaller; + } + return null; + } + + /** + * Caches the resolved marshaller for the given registry key. + * + * @param registryKey The registry key to associate with the cached marshaller. + * @param marshaller The marshaller instance to cache. + */ + public void cacheMarshaller(Object registryKey, Object marshaller) { + this.cachedMarshaller = marshaller; + this.cachedMarshallerRegistryKey = registryKey; + } + /** * Retrieves the current value of 'this' field from the given POJO. Uses the getter passed into the {@link Builder}. * diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/SdkFieldCacheMarshallerTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/SdkFieldCacheMarshallerTest.java new file mode 100644 index 000000000000..e266b3363814 --- /dev/null +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/SdkFieldCacheMarshallerTest.java @@ -0,0 +1,117 @@ +/* + * 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.core; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.core.protocol.MarshallLocation; +import software.amazon.awssdk.core.protocol.MarshallingType; +import software.amazon.awssdk.core.traits.LocationTrait; + +/** + * Tests for the marshaller cache on {@link SdkField}. + * + *

Validates: Requirements 7.1, 7.2

+ *

Property 2: Marshaller cache round-trip

+ */ +public class SdkFieldCacheMarshallerTest { + + private static SdkField newStringField() { + return SdkField.builder(MarshallingType.STRING) + .memberName("testField") + .getter(obj -> null) + .setter((obj, val) -> { }) + .traits(LocationTrait.builder() + .location(MarshallLocation.PAYLOAD) + .locationName("testField") + .build()) + .build(); + } + + /** + * cachedMarshaller returns null when nothing has been cached yet. + */ + @Test + public void cachedMarshaller_beforeAnyCaching_returnsNull() { + SdkField field = newStringField(); + Object registryKey = new Object(); + + Object cached = field.cachedMarshaller(registryKey); + assertThat(cached).isNull(); + } + + /** + * Round-trip: cacheMarshaller(key, m) then cachedMarshaller(key) returns the same instance. + */ + @Test + public void cachedMarshaller_afterCaching_returnsSameInstance() { + SdkField field = newStringField(); + Object registryKey = new Object(); + Object marshaller = new Object(); + + field.cacheMarshaller(registryKey, marshaller); + + Object cached = field.cachedMarshaller(registryKey); + assertThat(cached).isSameAs(marshaller); + } + + /** + * A different registry key reference returns null, even if both keys are "equal" by value. + * The cache uses reference identity (==), not equals(). + */ + @Test + public void cachedMarshaller_differentKeyReference_returnsNull() { + SdkField field = newStringField(); + // Use strings constructed so they are .equals() but not == + String key1 = new String("registry"); + String key2 = new String("registry"); + Object marshaller = new Object(); + + field.cacheMarshaller(key1, marshaller); + + // key2.equals(key1) is true, but key2 != key1 + Object cached = field.cachedMarshaller(key2); + assertThat(cached).isNull(); + } + + /** + * Overwriting the cache with a new registry key replaces the old entry. + * The old key no longer returns the old marshaller (single-slot replacement). + */ + @Test + public void cacheMarshaller_overwrite_replacesOldEntry() { + SdkField field = newStringField(); + Object oldKey = new Object(); + Object oldMarshaller = new Object(); + Object newKey = new Object(); + Object newMarshaller = new Object(); + + field.cacheMarshaller(oldKey, oldMarshaller); + Object cachedOld = field.cachedMarshaller(oldKey); + assertThat(cachedOld).isSameAs(oldMarshaller); + + // Overwrite with a new key + field.cacheMarshaller(newKey, newMarshaller); + + // New key returns the new marshaller + Object cachedNew = field.cachedMarshaller(newKey); + assertThat(cachedNew).isSameAs(newMarshaller); + // Old key no longer returns anything + Object cachedOldAfter = field.cachedMarshaller(oldKey); + assertThat(cachedOldAfter).isNull(); + } +} From 3a4814415de70fa0198bae879d6088d4c71e68cd Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Mon, 13 Apr 2026 17:23:12 -0700 Subject: [PATCH 02/12] Refactor code to reduce duplication from switch statement --- .../feature-AWSSDKforJavav2-439f346.json | 6 +++ .../marshall/JsonProtocolMarshaller.java | 39 ++++--------------- .../software/amazon/awssdk/core/SdkField.java | 6 +++ 3 files changed, 20 insertions(+), 31 deletions(-) create mode 100644 .changes/next-release/feature-AWSSDKforJavav2-439f346.json diff --git a/.changes/next-release/feature-AWSSDKforJavav2-439f346.json b/.changes/next-release/feature-AWSSDKforJavav2-439f346.json new file mode 100644 index 000000000000..46ea293d42cc --- /dev/null +++ b/.changes/next-release/feature-AWSSDKforJavav2-439f346.json @@ -0,0 +1,6 @@ +{ + "type": "feature", + "category": "AWS SDK for Java v2", + "contributor": "", + "description": "Optimized JSON marshalling performance for JSON RPC and REST JSON protocols." +} diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java index d62a254d8cc0..39370010ffc9 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java @@ -42,8 +42,6 @@ import software.amazon.awssdk.core.traits.RequiredTrait; import software.amazon.awssdk.core.traits.TimestampFormatTrait; import software.amazon.awssdk.core.traits.TraitType; -import software.amazon.awssdk.core.util.SdkAutoConstructList; -import software.amazon.awssdk.core.util.SdkAutoConstructMap; import software.amazon.awssdk.http.SdkHttpFullRequest; import software.amazon.awssdk.protocols.core.InstantToString; import software.amazon.awssdk.protocols.core.OperationInfo; @@ -385,41 +383,20 @@ private void marshallPayloadField(SdkField field, Object val) { gen.writeValue(((SdkBytes) val).asByteBuffer()); break; case SDK_POJO: - gen.writeFieldName(fieldName); - gen.writeStartObject(); - doMarshall((SdkPojo) val); - gen.writeEndObject(); + SimpleTypeJsonMarshaller.SDK_POJO.marshall((SdkPojo) val, marshallerContext, + fieldName, (SdkField) field); break; case LIST: - List list = (List) val; - if (list.isEmpty() && list instanceof SdkAutoConstructList) { - break; - } - gen.writeFieldName(fieldName); - gen.writeStartArray(list.size()); - for (Object item : list) { - marshallerContext.marshall(MarshallLocation.PAYLOAD, item); - } - gen.writeEndArray(); + SimpleTypeJsonMarshaller.LIST.marshall((List) val, marshallerContext, + fieldName, (SdkField>) field); break; case MAP: - Map map = (Map) val; - if (map.isEmpty() && map instanceof SdkAutoConstructMap) { - break; - } - gen.writeFieldName(fieldName); - gen.writeStartObject(); - for (Map.Entry entry : map.entrySet()) { - if (entry.getValue() != null) { - gen.writeFieldName(entry.getKey()); - marshallerContext.marshall(MarshallLocation.PAYLOAD, entry.getValue()); - } - } - gen.writeEndObject(); + SimpleTypeJsonMarshaller.MAP.marshall((Map) val, marshallerContext, + fieldName, (SdkField>) field); break; case DOCUMENT: - gen.writeFieldName(fieldName); - ((Document) val).accept(new DocumentTypeJsonMarshaller(gen)); + SimpleTypeJsonMarshaller.DOCUMENT.marshall((Document) val, marshallerContext, + fieldName, (SdkField) field); break; default: // Unknown type — fall back to registry lookup diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkField.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkField.java index fc4f81038f26..730144e62363 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkField.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkField.java @@ -50,6 +50,12 @@ public final class SdkField { private final Map l1Traits; private final Map, Trait> l2Traits; + // Single-slot marshaller cache. Two volatile fields are used instead of an AtomicReference to an immutable + // holder to avoid per-SdkField object allocation. The read in cachedMarshaller() is not atomic across both + // fields: between reading the key and reading the marshaller, another thread could overwrite both. This is + // safe because (1) in practice there is only one registry per protocol, so all threads converge to the same + // marshaller, and (2) the worst case with multiple registries is a benign cache miss or a single call using + // a marshaller from a different registry, which self-corrects on the next call. private volatile Object cachedMarshaller; private volatile Object cachedMarshallerRegistryKey; From 9be8184379df80aba22a02b2dc30e71346bb8024 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Thu, 23 Apr 2026 08:06:52 -0700 Subject: [PATCH 03/12] Change approaches - move cache into the marshaller instead of SDKFields --- .../marshall/JsonProtocolMarshaller.java | 14 ++- .../CachedNonPayloadMarshallingTest.java | 19 +-- .../software/amazon/awssdk/core/SdkField.java | 36 ------ .../core/SdkFieldCacheMarshallerTest.java | 117 ------------------ 4 files changed, 11 insertions(+), 175 deletions(-) delete mode 100644 core/sdk-core/src/test/java/software/amazon/awssdk/core/SdkFieldCacheMarshallerTest.java diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java index 39370010ffc9..aad4114d084d 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java @@ -30,6 +30,7 @@ import java.util.EnumMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.core.SdkBytes; import software.amazon.awssdk.core.SdkField; @@ -65,6 +66,12 @@ public class JsonProtocolMarshaller implements ProtocolMarshaller, JsonMarshaller> MARSHALLER_CACHE = + new ConcurrentHashMap<>(); + private final URI endpoint; private final StructuredJsonGenerator jsonGenerator; private final SdkHttpFullRequest.Builder request; @@ -412,11 +419,8 @@ private void marshallFieldViaRegistry(SdkField field, Object val) { .marshall(val, marshallerContext, field.locationName(), (SdkField) field); return; } - JsonMarshaller marshaller = field.cachedMarshaller(MARSHALLER_REGISTRY); - if (marshaller == null) { - marshaller = MARSHALLER_REGISTRY.getMarshaller(field.location(), field.marshallingType(), val); - field.cacheMarshaller(MARSHALLER_REGISTRY, marshaller); - } + JsonMarshaller marshaller = MARSHALLER_CACHE.computeIfAbsent(field, + f -> MARSHALLER_REGISTRY.getMarshaller(f.location(), f.marshallingType(), val)); marshaller.marshall(val, marshallerContext, field.locationName(), (SdkField) field); } diff --git a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/CachedNonPayloadMarshallingTest.java b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/CachedNonPayloadMarshallingTest.java index 2b4e559a518e..929ae22c38ce 100644 --- a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/CachedNonPayloadMarshallingTest.java +++ b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/CachedNonPayloadMarshallingTest.java @@ -77,36 +77,21 @@ void header_string_secondCall_usesCachedMarshaller() { // Use the SAME SdkField instance for both calls so the cache is shared SdkField field = headerField("x-custom-header", obj -> "headerValue"); - // First call — populates the cache + // First call — populates the internal marshaller cache SdkPojo pojo1 = new SimplePojo(field); SdkHttpFullRequest result1 = createMarshaller().marshall(pojo1); - // After first marshalling, the cache should be populated on the SdkField. - // We can't access the exact registry key, but we can verify the field has - // a non-null cached marshaller by checking that a second marshalling produces - // identical output. - // Second call — should use cached marshaller SdkPojo pojo2 = new SimplePojo(field); SdkHttpFullRequest result2 = createMarshaller().marshall(pojo2); - // Both calls produce identical header output + // 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"); - - // Verify the cache was populated: the field should have a non-null cached - // marshaller for at least one registry key. Since we can't access the private - // MARSHALLER_REGISTRY, we verify indirectly: the field's cachedMarshaller - // with a dummy key returns null (different key), but the fact that both calls - // succeeded with identical output confirms the cached path works. - Object cachedWithDifferentKey = field.cachedMarshaller(new Object()); - assertThat(cachedWithDifferentKey) - .as("Different registry key should return null") - .isNull(); } // ---- QUERY_PARAM tests ---- diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkField.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkField.java index 730144e62363..98561baca4ac 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkField.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkField.java @@ -50,15 +50,6 @@ public final class SdkField { private final Map l1Traits; private final Map, Trait> l2Traits; - // Single-slot marshaller cache. Two volatile fields are used instead of an AtomicReference to an immutable - // holder to avoid per-SdkField object allocation. The read in cachedMarshaller() is not atomic across both - // fields: between reading the key and reading the marshaller, another thread could overwrite both. This is - // safe because (1) in practice there is only one registry per protocol, so all threads converge to the same - // marshaller, and (2) the worst case with multiple registries is a benign cache miss or a single call using - // a marshaller from a different registry, which self-corrects on the next call. - private volatile Object cachedMarshaller; - private volatile Object cachedMarshallerRegistryKey; - private SdkField(Builder builder) { this.memberName = builder.memberName; this.marshallingType = builder.marshallingType; @@ -262,33 +253,6 @@ public boolean containsTrait(Class clzz, TraitType type) { return getTrait(clzz, type) != null; } - /** - * Returns the cached marshaller for the given registry key, or null if not cached. - * Uses reference identity ({@code ==}) for the registry key comparison. - * - * @param registryKey The registry key to match against the cached key. - * @param The type of the cached marshaller. - * @return The cached marshaller if the registry key matches, or null. - */ - @SuppressWarnings("unchecked") - public T cachedMarshaller(Object registryKey) { - if (cachedMarshallerRegistryKey == registryKey) { - return (T) cachedMarshaller; - } - return null; - } - - /** - * Caches the resolved marshaller for the given registry key. - * - * @param registryKey The registry key to associate with the cached marshaller. - * @param marshaller The marshaller instance to cache. - */ - public void cacheMarshaller(Object registryKey, Object marshaller) { - this.cachedMarshaller = marshaller; - this.cachedMarshallerRegistryKey = registryKey; - } - /** * Retrieves the current value of 'this' field from the given POJO. Uses the getter passed into the {@link Builder}. * diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/SdkFieldCacheMarshallerTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/SdkFieldCacheMarshallerTest.java deleted file mode 100644 index e266b3363814..000000000000 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/SdkFieldCacheMarshallerTest.java +++ /dev/null @@ -1,117 +0,0 @@ -/* - * 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.core; - -import static org.assertj.core.api.Assertions.assertThat; - -import org.junit.jupiter.api.Test; -import software.amazon.awssdk.core.protocol.MarshallLocation; -import software.amazon.awssdk.core.protocol.MarshallingType; -import software.amazon.awssdk.core.traits.LocationTrait; - -/** - * Tests for the marshaller cache on {@link SdkField}. - * - *

Validates: Requirements 7.1, 7.2

- *

Property 2: Marshaller cache round-trip

- */ -public class SdkFieldCacheMarshallerTest { - - private static SdkField newStringField() { - return SdkField.builder(MarshallingType.STRING) - .memberName("testField") - .getter(obj -> null) - .setter((obj, val) -> { }) - .traits(LocationTrait.builder() - .location(MarshallLocation.PAYLOAD) - .locationName("testField") - .build()) - .build(); - } - - /** - * cachedMarshaller returns null when nothing has been cached yet. - */ - @Test - public void cachedMarshaller_beforeAnyCaching_returnsNull() { - SdkField field = newStringField(); - Object registryKey = new Object(); - - Object cached = field.cachedMarshaller(registryKey); - assertThat(cached).isNull(); - } - - /** - * Round-trip: cacheMarshaller(key, m) then cachedMarshaller(key) returns the same instance. - */ - @Test - public void cachedMarshaller_afterCaching_returnsSameInstance() { - SdkField field = newStringField(); - Object registryKey = new Object(); - Object marshaller = new Object(); - - field.cacheMarshaller(registryKey, marshaller); - - Object cached = field.cachedMarshaller(registryKey); - assertThat(cached).isSameAs(marshaller); - } - - /** - * A different registry key reference returns null, even if both keys are "equal" by value. - * The cache uses reference identity (==), not equals(). - */ - @Test - public void cachedMarshaller_differentKeyReference_returnsNull() { - SdkField field = newStringField(); - // Use strings constructed so they are .equals() but not == - String key1 = new String("registry"); - String key2 = new String("registry"); - Object marshaller = new Object(); - - field.cacheMarshaller(key1, marshaller); - - // key2.equals(key1) is true, but key2 != key1 - Object cached = field.cachedMarshaller(key2); - assertThat(cached).isNull(); - } - - /** - * Overwriting the cache with a new registry key replaces the old entry. - * The old key no longer returns the old marshaller (single-slot replacement). - */ - @Test - public void cacheMarshaller_overwrite_replacesOldEntry() { - SdkField field = newStringField(); - Object oldKey = new Object(); - Object oldMarshaller = new Object(); - Object newKey = new Object(); - Object newMarshaller = new Object(); - - field.cacheMarshaller(oldKey, oldMarshaller); - Object cachedOld = field.cachedMarshaller(oldKey); - assertThat(cachedOld).isSameAs(oldMarshaller); - - // Overwrite with a new key - field.cacheMarshaller(newKey, newMarshaller); - - // New key returns the new marshaller - Object cachedNew = field.cachedMarshaller(newKey); - assertThat(cachedNew).isSameAs(newMarshaller); - // Old key no longer returns anything - Object cachedOldAfter = field.cachedMarshaller(oldKey); - assertThat(cachedOldAfter).isNull(); - } -} From 6f5bf03ffd03a9fa1c872ab2476211ae1ed0f20b Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Sat, 25 Apr 2026 12:32:12 -0700 Subject: [PATCH 04/12] Optimize byte buffer/output stream --- .../json/ExposedByteArrayOutputStream.java | 51 +++++++ .../protocols/json/SdkJsonGenerator.java | 27 +++- .../marshall/JsonProtocolMarshaller.java | 24 +++- .../ExposedByteArrayOutputStreamTest.java | 79 +++++++++++ .../protocols/json/SdkJsonGeneratorTest.java | 129 ++++++++++++++++++ 5 files changed, 303 insertions(+), 7 deletions(-) create mode 100644 core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/ExposedByteArrayOutputStream.java create mode 100644 core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/ExposedByteArrayOutputStreamTest.java diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/ExposedByteArrayOutputStream.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/ExposedByteArrayOutputStream.java new file mode 100644 index 000000000000..ce321c373377 --- /dev/null +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/ExposedByteArrayOutputStream.java @@ -0,0 +1,51 @@ +/* + * 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; + +import java.io.ByteArrayOutputStream; +import software.amazon.awssdk.annotations.SdkInternalApi; + +/** + * A thin subclass of {@link ByteArrayOutputStream} that exposes the internal buffer and count + * without copying. This allows {@link SdkJsonGenerator} to create a {@code ContentStreamProvider} + * that wraps the buffer directly via {@code ByteArrayInputStream(buf, 0, count)}, avoiding the + * contiguous copy that {@link ByteArrayOutputStream#toByteArray()} performs. + * + *

The write path is identical to {@code ByteArrayOutputStream} — no overhead is added. + * Only the final "get the bytes" step is optimized. + * + *

This class is not thread-safe. + */ +@SdkInternalApi +final class ExposedByteArrayOutputStream extends ByteArrayOutputStream { + + ExposedByteArrayOutputStream(int size) { + super(size); + } + + /** + * Returns the internal buffer. The valid data is in {@code buf[0..count-1]}. + * The returned array may be larger than {@link #size()}; callers must use + * {@link #size()} to determine the valid range. + * + *

Warning: The returned array is the live internal buffer. Do not modify it, + * and do not write to this stream after capturing the reference — the buffer may be + * replaced by a larger one on the next write if growth is needed. + */ + byte[] buf() { + return buf; + } +} diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java index bfd819708b33..24fe7d928a80 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java @@ -15,7 +15,7 @@ package software.amazon.awssdk.protocols.json; -import java.io.ByteArrayOutputStream; +import java.io.ByteArrayInputStream; import java.io.IOException; import java.math.BigDecimal; import java.math.BigInteger; @@ -23,6 +23,7 @@ import java.time.Instant; import software.amazon.awssdk.annotations.SdkProtectedApi; import software.amazon.awssdk.core.exception.SdkClientException; +import software.amazon.awssdk.http.ContentStreamProvider; import software.amazon.awssdk.thirdparty.jackson.core.JsonFactory; import software.amazon.awssdk.thirdparty.jackson.core.JsonGenerator; import software.amazon.awssdk.utils.BinaryUtils; @@ -39,7 +40,7 @@ public class SdkJsonGenerator implements StructuredJsonGenerator { * prevent frequent resizings but small enough to avoid wasted allocations for small requests. */ private static final int DEFAULT_BUFFER_SIZE = 1024; - private final ByteArrayOutputStream baos = new ByteArrayOutputStream(DEFAULT_BUFFER_SIZE); + private final ExposedByteArrayOutputStream baos = new ExposedByteArrayOutputStream(DEFAULT_BUFFER_SIZE); private final JsonGenerator generator; private final String contentType; @@ -277,6 +278,28 @@ public byte[] getBytes() { return baos.toByteArray(); } + /** + * Returns the size of the generated content in bytes without copying. + */ + public int contentSize() { + close(); + return baos.size(); + } + + /** + * Returns a {@link ContentStreamProvider} that wraps the internal buffer directly, + * avoiding the contiguous copy that {@link #getBytes()} performs via + * {@code ByteArrayOutputStream.toByteArray()}. Each call to + * {@link ContentStreamProvider#newStream()} creates a fresh {@code ByteArrayInputStream} + * over the same buffer for retry safety. + */ + public ContentStreamProvider contentStreamProvider() { + close(); + byte[] buf = baos.buf(); + int count = baos.size(); + return () -> new ByteArrayInputStream(buf, 0, count); + } + @Override public String getContentType() { return contentType; diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java index aad4114d084d..cfcfdbbda310 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java @@ -43,6 +43,7 @@ import software.amazon.awssdk.core.traits.RequiredTrait; import software.amazon.awssdk.core.traits.TimestampFormatTrait; import software.amazon.awssdk.core.traits.TraitType; +import software.amazon.awssdk.http.ContentStreamProvider; import software.amazon.awssdk.http.SdkHttpFullRequest; import software.amazon.awssdk.protocols.core.InstantToString; import software.amazon.awssdk.protocols.core.OperationInfo; @@ -52,6 +53,7 @@ import software.amazon.awssdk.protocols.json.AwsJsonProtocol; import software.amazon.awssdk.protocols.json.AwsJsonProtocolMetadata; import software.amazon.awssdk.protocols.json.BaseAwsJsonProtocolFactory; +import software.amazon.awssdk.protocols.json.SdkJsonGenerator; import software.amazon.awssdk.protocols.json.StructuredJsonGenerator; import software.amazon.awssdk.protocols.json.internal.ProtocolFact; @@ -288,12 +290,24 @@ private SdkHttpFullRequest finishMarshalling() { jsonGenerator.writeEndObject(); } - byte[] content = jsonGenerator.getBytes(); + if (jsonGenerator instanceof SdkJsonGenerator) { + // Optimized path: stream directly from chunked buffers, avoiding a single + // contiguous byte[] allocation that can cause G1GC humongous allocations. + SdkJsonGenerator sdkGenerator = (SdkJsonGenerator) jsonGenerator; + ContentStreamProvider contentProvider = sdkGenerator.contentStreamProvider(); + request.contentStreamProvider(contentProvider); + int contentSize = sdkGenerator.contentSize(); + if (contentSize > 0) { + request.putHeader(CONTENT_LENGTH, Integer.toString(contentSize)); + } + } else { + byte[] content = jsonGenerator.getBytes(); - if (content != null) { - request.contentStreamProvider(() -> new ByteArrayInputStream(content)); - if (content.length > 0) { - request.putHeader(CONTENT_LENGTH, Integer.toString(content.length)); + if (content != null) { + request.contentStreamProvider(() -> new ByteArrayInputStream(content)); + if (content.length > 0) { + request.putHeader(CONTENT_LENGTH, Integer.toString(content.length)); + } } } } diff --git a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/ExposedByteArrayOutputStreamTest.java b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/ExposedByteArrayOutputStreamTest.java new file mode 100644 index 000000000000..81980dca9cc4 --- /dev/null +++ b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/ExposedByteArrayOutputStreamTest.java @@ -0,0 +1,79 @@ +/* + * 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; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Arrays; +import org.junit.jupiter.api.Test; + +class ExposedByteArrayOutputStreamTest { + + @Test + void emptyStream_hasZeroSize() { + ExposedByteArrayOutputStream stream = new ExposedByteArrayOutputStream(64); + assertThat(stream.size()).isEqualTo(0); + assertThat(stream.toByteArray()).isEmpty(); + } + + @Test + void buf_returnsInternalBuffer() { + ExposedByteArrayOutputStream stream = new ExposedByteArrayOutputStream(64); + byte[] data = {1, 2, 3, 4, 5}; + stream.write(data, 0, data.length); + + byte[] buf = stream.buf(); + // buf is the live internal buffer — it may be larger than size() + assertThat(buf.length).isGreaterThanOrEqualTo(stream.size()); + // The valid data in buf[0..size()-1] matches what was written + assertThat(Arrays.copyOf(buf, stream.size())).isEqualTo(data); + } + + @Test + void buf_reflectsWrittenData_afterGrowth() { + // Start with a tiny buffer to force growth + ExposedByteArrayOutputStream stream = new ExposedByteArrayOutputStream(4); + byte[] data = new byte[100]; + for (int i = 0; i < data.length; i++) { + data[i] = (byte) i; + } + stream.write(data, 0, data.length); + + byte[] buf = stream.buf(); + assertThat(stream.size()).isEqualTo(100); + assertThat(Arrays.copyOf(buf, stream.size())).isEqualTo(data); + } + + @Test + void toByteArray_returnsCopy_notSameReference() { + ExposedByteArrayOutputStream stream = new ExposedByteArrayOutputStream(64); + stream.write(new byte[]{1, 2, 3}, 0, 3); + + byte[] copy = stream.toByteArray(); + byte[] buf = stream.buf(); + // toByteArray returns a copy, buf returns the live buffer + assertThat(copy).isNotSameAs(buf); + assertThat(copy).isEqualTo(Arrays.copyOf(buf, stream.size())); + } + + @Test + void singleByteWrite_worksCorrectly() { + ExposedByteArrayOutputStream stream = new ExposedByteArrayOutputStream(64); + stream.write(0x42); + assertThat(stream.size()).isEqualTo(1); + assertThat(stream.buf()[0]).isEqualTo((byte) 0x42); + } +} diff --git a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/SdkJsonGeneratorTest.java b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/SdkJsonGeneratorTest.java index bba1caedfb0d..0ab737ca91f5 100644 --- a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/SdkJsonGeneratorTest.java +++ b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/SdkJsonGeneratorTest.java @@ -21,11 +21,13 @@ import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.InputStream; import java.nio.ByteBuffer; import java.nio.charset.Charset; import java.time.Instant; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import software.amazon.awssdk.http.ContentStreamProvider; import software.amazon.awssdk.protocols.jsoncore.JsonNode; import software.amazon.awssdk.thirdparty.jackson.core.JsonFactory; import software.amazon.awssdk.thirdparty.jackson.core.StreamReadFeature; @@ -178,4 +180,131 @@ private JsonNode toJsonNode() throws IOException { return JsonNode.parser().parse(new ByteArrayInputStream(jsonGenerator.getBytes())); } + @Test + public void contentSize_matchesGetBytesLength() { + SdkJsonGenerator gen = newSdkJsonGenerator(); + gen.writeStartObject(); + gen.writeFieldName("key").writeValue("value"); + gen.writeFieldName("num").writeValue(42); + gen.writeEndObject(); + + byte[] bytes = gen.getBytes(); + + SdkJsonGenerator gen2 = newSdkJsonGenerator(); + gen2.writeStartObject(); + gen2.writeFieldName("key").writeValue("value"); + gen2.writeFieldName("num").writeValue(42); + gen2.writeEndObject(); + + assertEquals(bytes.length, gen2.contentSize()); + } + + @Test + public void contentStreamProvider_producesSameBytesAsGetBytes() throws IOException { + SdkJsonGenerator gen = newSdkJsonGenerator(); + gen.writeStartObject(); + gen.writeFieldName("hello").writeValue("world"); + gen.writeFieldName("count").writeValue(123); + gen.writeEndObject(); + + byte[] expected = gen.getBytes(); + + SdkJsonGenerator gen2 = newSdkJsonGenerator(); + gen2.writeStartObject(); + gen2.writeFieldName("hello").writeValue("world"); + gen2.writeFieldName("count").writeValue(123); + gen2.writeEndObject(); + + ContentStreamProvider provider = gen2.contentStreamProvider(); + byte[] actual = readAllBytes(provider.newStream()); + + assertTrue(java.util.Arrays.equals(expected, actual), + "contentStreamProvider should produce identical bytes to getBytes"); + } + + @Test + public void contentStreamProvider_isResettable() throws IOException { + SdkJsonGenerator gen = newSdkJsonGenerator(); + gen.writeStartObject(); + gen.writeFieldName("data").writeValue("test"); + gen.writeEndObject(); + + ContentStreamProvider provider = gen.contentStreamProvider(); + byte[] first = readAllBytes(provider.newStream()); + byte[] second = readAllBytes(provider.newStream()); + + assertTrue(java.util.Arrays.equals(first, second), + "Multiple calls to newStream() should produce identical content"); + assertTrue(first.length > 0, "Content should not be empty"); + } + + @Test + public void emptyGenerator_contentSizeIsZero() throws IOException { + SdkJsonGenerator gen = newSdkJsonGenerator(); + assertEquals(0, gen.contentSize()); + + ContentStreamProvider provider = gen.contentStreamProvider(); + assertTrue(provider != null, "Provider should not be null even for empty content"); + byte[] content = readAllBytes(provider.newStream()); + assertEquals(0, content.length, "Empty generator should produce empty stream"); + } + + @Test + public void largePayload_contentStreamProviderStreamsCorrectData() throws IOException { + // Generate JSON exceeding 64 KB to verify contentStreamProvider works for large payloads + SdkJsonGenerator gen = newSdkJsonGenerator(); + gen.writeStartObject(); + gen.writeFieldName("items"); + gen.writeStartArray(); + for (int i = 0; i < 2000; i++) { + gen.writeStartObject(); + gen.writeFieldName("index").writeValue(i); + gen.writeFieldName("description").writeValue( + "This is a moderately long string value for item number " + i + + " that helps push the total payload size beyond the 64KB chunk boundary."); + gen.writeEndObject(); + } + gen.writeEndArray(); + gen.writeEndObject(); + + byte[] expected = gen.getBytes(); + assertTrue(expected.length > 64 * 1024, "Payload should exceed 64 KB"); + + SdkJsonGenerator gen2 = newSdkJsonGenerator(); + gen2.writeStartObject(); + gen2.writeFieldName("items"); + gen2.writeStartArray(); + for (int i = 0; i < 2000; i++) { + gen2.writeStartObject(); + gen2.writeFieldName("index").writeValue(i); + gen2.writeFieldName("description").writeValue( + "This is a moderately long string value for item number " + i + + " that helps push the total payload size beyond the 64KB chunk boundary."); + gen2.writeEndObject(); + } + gen2.writeEndArray(); + gen2.writeEndObject(); + + assertEquals(expected.length, gen2.contentSize()); + byte[] actual = readAllBytes(gen2.contentStreamProvider().newStream()); + assertTrue(java.util.Arrays.equals(expected, actual), + "Large payload should stream correctly via contentStreamProvider"); + } + + private SdkJsonGenerator newSdkJsonGenerator() { + return new SdkJsonGenerator(JsonFactory.builder() + .enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION) + .build(), "application/json"); + } + + private static byte[] readAllBytes(InputStream is) throws IOException { + java.io.ByteArrayOutputStream bos = new java.io.ByteArrayOutputStream(); + byte[] buf = new byte[1024]; + int n; + while ((n = is.read(buf)) != -1) { + bos.write(buf, 0, n); + } + return bos.toByteArray(); + } + } From 6776dc908561f23799c081a3174222b7b3f828eb Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Mon, 27 Apr 2026 08:35:08 -0700 Subject: [PATCH 05/12] Optimize binary marshalling --- .../amazon/awssdk/protocols/json/SdkJsonGenerator.java | 10 ++++++++++ .../awssdk/protocols/json/StructuredJsonGenerator.java | 9 +++++++++ .../json/internal/marshall/JsonProtocolMarshaller.java | 2 +- 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java index 24fe7d928a80..905f00f09efb 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java @@ -207,6 +207,16 @@ public StructuredJsonGenerator writeValue(ByteBuffer bytes) { return this; } + @Override + public StructuredJsonGenerator writeBinaryValue(byte[] bytes) { + try { + generator.writeBinary(bytes); + } catch (IOException e) { + throw new JsonGenerationException(e); + } + return this; + } + @Override //TODO: This date formatting is coupled to AWS's format. Should generalize it public StructuredJsonGenerator writeValue(Instant instant) { diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/StructuredJsonGenerator.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/StructuredJsonGenerator.java index 8d02b2ea78f8..db883f8c5325 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/StructuredJsonGenerator.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/StructuredJsonGenerator.java @@ -169,6 +169,15 @@ default StructuredJsonGenerator writeValue(byte val) { StructuredJsonGenerator writeValue(ByteBuffer bytes); + /** + * Writes binary data directly from a byte array, avoiding the overhead of wrapping in a + * {@link ByteBuffer}. The default implementation wraps the array and delegates to + * {@link #writeValue(ByteBuffer)}. + */ + default StructuredJsonGenerator writeBinaryValue(byte[] bytes) { + return writeValue(ByteBuffer.wrap(bytes)); + } + StructuredJsonGenerator writeValue(Instant instant); StructuredJsonGenerator writeNumber(String number); diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java index cfcfdbbda310..8db37cfc530c 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java @@ -401,7 +401,7 @@ private void marshallPayloadField(SdkField field, Object val) { break; case SDK_BYTES: gen.writeFieldName(fieldName); - gen.writeValue(((SdkBytes) val).asByteBuffer()); + gen.writeBinaryValue(((SdkBytes) val).asByteArrayUnsafe()); break; case SDK_POJO: SimpleTypeJsonMarshaller.SDK_POJO.marshall((SdkPojo) val, marshallerContext, From c3993ac08169c7f2eb732799dde00290eaf87087 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Mon, 27 Apr 2026 14:03:27 -0700 Subject: [PATCH 06/12] Try new more dynamic output stream approach --- .../json/SdkByteArrayOutputStream.java | 209 ++++++++++++++++++ .../protocols/json/SdkJsonGenerator.java | 17 +- 2 files changed, 215 insertions(+), 11 deletions(-) create mode 100644 core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStream.java diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStream.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStream.java new file mode 100644 index 000000000000..bcd41fc4d441 --- /dev/null +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStream.java @@ -0,0 +1,209 @@ +/* + * 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; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.InputStream; +import java.io.SequenceInputStream; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import software.amazon.awssdk.annotations.SdkInternalApi; +import software.amazon.awssdk.http.ContentStreamProvider; + +/** + * A {@link ByteArrayOutputStream} subclass that behaves identically to the JDK implementation for + * small payloads, but caps internal buffer growth to avoid G1GC humongous object allocations for + * large payloads. + * + *

How it works: Writes flow into the inherited {@code ByteArrayOutputStream} buffer + * normally. When a write would cause the buffer to grow beyond {@link #MAX_BUFFER_SIZE}, the + * current buffer contents are frozen into the first "chunk" and subsequent writes go into + * fixed-size overflow chunks ({@link #CHUNK_SIZE} bytes each). No single allocation ever exceeds + * {@code MAX_BUFFER_SIZE}. + * + *

Performance characteristics: + *

    + *
  • Payloads ≤ {@code MAX_BUFFER_SIZE}: Identical to {@code ByteArrayOutputStream} — the + * JIT can inline and optimize the write path exactly as it does for the stock class. Zero + * overhead.
    • + *
  • Payloads > {@code MAX_BUFFER_SIZE}: Overflow writes go through a simple chunked path. + * This is slightly slower per-byte than {@code ByteArrayOutputStream}'s doubling strategy, + * but avoids allocations that exceed half the G1 region size.
    • + *
+ * + *

This class is not thread-safe. + */ +@SdkInternalApi +final class SdkByteArrayOutputStream extends ByteArrayOutputStream { + + /** + * Maximum size of the primary ByteArrayOutputStream buffer before overflow kicks in. + * Chosen to be well below the G1 humongous threshold (region_size / 2). With a 4 GB heap + * the default region size is 2 MB, so humongous threshold is 1 MB. 128 KB is safely below + * that for any reasonable heap size (humongous threshold is 512 KB for a 256 MB heap). + */ + static final int MAX_BUFFER_SIZE = 128 * 1024; + + /** + * Size of each overflow chunk. 64 KB is well below any G1 humongous threshold. + */ + static final int CHUNK_SIZE = 64 * 1024; + + private List overflowChunks; + private int overflowChunkOffset; + private int overflowTotalBytes; + private boolean overflowing; + + SdkByteArrayOutputStream(int initialCapacity) { + super(initialCapacity); + } + + @Override + public void write(int b) { + if (overflowing) { + ensureOverflowCapacity(1); + currentOverflowChunk()[overflowChunkOffset++] = (byte) b; + overflowTotalBytes++; + } else if (count + 1 > MAX_BUFFER_SIZE) { + startOverflow(); + write(b); + } else { + super.write(b); + } + } + + @Override + public void write(byte[] b, int off, int len) { + if (overflowing) { + writeToOverflow(b, off, len); + } else if (count + len > MAX_BUFFER_SIZE) { + // Write what fits into the base buffer, then overflow the rest + int fits = MAX_BUFFER_SIZE - count; + if (fits > 0) { + super.write(b, off, fits); + } + startOverflow(); + writeToOverflow(b, off + fits, len - fits); + } else { + super.write(b, off, len); + } + } + + /** + * Returns the total number of bytes written (base buffer + overflow). + */ + @Override + public int size() { + return count + overflowTotalBytes; + } + + /** + * Returns all written data as a single contiguous byte array. Exists for backward + * compatibility via {@link #toByteArray()} but should not be used on the hot path. + */ + @Override + public byte[] toByteArray() { + if (!overflowing) { + return super.toByteArray(); + } + int total = size(); + byte[] result = new byte[total]; + // Copy base buffer + System.arraycopy(buf, 0, result, 0, count); + // Copy overflow chunks + int destOff = count; + for (int i = 0; i < overflowChunks.size(); i++) { + int len = (i < overflowChunks.size() - 1) ? overflowChunks.get(i).length : overflowChunkOffset; + System.arraycopy(overflowChunks.get(i), 0, result, destOff, len); + destOff += len; + } + return result; + } + + /** + * Returns a {@link ContentStreamProvider} that streams directly from the internal buffers + * without creating a contiguous copy. For small payloads this wraps the single base buffer; + * for large payloads it chains the base buffer and overflow chunks via + * {@link SequenceInputStream}. + */ + ContentStreamProvider contentStreamProvider() { + if (!overflowing) { + // Small payload: single buffer, wrap directly (same as ExposedByteArrayOutputStream) + byte[] b = buf; + int c = count; + return () -> new ByteArrayInputStream(b, 0, c); + } + + // Large payload: chain base buffer + overflow chunks + byte[] baseBuf = buf; + int baseCount = count; + List chunks = overflowChunks; + int lastChunkLen = overflowChunkOffset; + + return () -> { + List streams = new ArrayList<>(1 + chunks.size()); + streams.add(new ByteArrayInputStream(baseBuf, 0, baseCount)); + for (int i = 0; i < chunks.size(); i++) { + int len = (i < chunks.size() - 1) ? chunks.get(i).length : lastChunkLen; + streams.add(new ByteArrayInputStream(chunks.get(i), 0, len)); + } + return new SequenceInputStream(Collections.enumeration(streams)); + }; + } + + /** + * Returns the content size without copying. + */ + int contentSize() { + return size(); + } + + private void startOverflow() { + overflowing = true; + overflowChunks = new ArrayList<>(); + overflowChunks.add(new byte[CHUNK_SIZE]); + overflowChunkOffset = 0; + overflowTotalBytes = 0; + } + + private void writeToOverflow(byte[] b, int off, int len) { + int remaining = len; + int srcOff = off; + while (remaining > 0) { + ensureOverflowCapacity(1); + int space = currentOverflowChunk().length - overflowChunkOffset; + int toCopy = Math.min(remaining, space); + System.arraycopy(b, srcOff, currentOverflowChunk(), overflowChunkOffset, toCopy); + overflowChunkOffset += toCopy; + overflowTotalBytes += toCopy; + srcOff += toCopy; + remaining -= toCopy; + } + } + + private byte[] currentOverflowChunk() { + return overflowChunks.get(overflowChunks.size() - 1); + } + + private void ensureOverflowCapacity(int needed) { + if (overflowChunkOffset + needed > currentOverflowChunk().length) { + overflowChunks.add(new byte[CHUNK_SIZE]); + overflowChunkOffset = 0; + } + } +} diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java index 905f00f09efb..e178885774f2 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java @@ -15,7 +15,6 @@ package software.amazon.awssdk.protocols.json; -import java.io.ByteArrayInputStream; import java.io.IOException; import java.math.BigDecimal; import java.math.BigInteger; @@ -40,7 +39,7 @@ public class SdkJsonGenerator implements StructuredJsonGenerator { * prevent frequent resizings but small enough to avoid wasted allocations for small requests. */ private static final int DEFAULT_BUFFER_SIZE = 1024; - private final ExposedByteArrayOutputStream baos = new ExposedByteArrayOutputStream(DEFAULT_BUFFER_SIZE); + private final SdkByteArrayOutputStream baos = new SdkByteArrayOutputStream(DEFAULT_BUFFER_SIZE); private final JsonGenerator generator; private final String contentType; @@ -293,21 +292,17 @@ public byte[] getBytes() { */ public int contentSize() { close(); - return baos.size(); + return baos.contentSize(); } /** - * Returns a {@link ContentStreamProvider} that wraps the internal buffer directly, - * avoiding the contiguous copy that {@link #getBytes()} performs via - * {@code ByteArrayOutputStream.toByteArray()}. Each call to - * {@link ContentStreamProvider#newStream()} creates a fresh {@code ByteArrayInputStream} - * over the same buffer for retry safety. + * Returns a {@link ContentStreamProvider} that streams directly from the internal buffers + * without creating a contiguous copy. For small payloads this wraps the single base buffer; + * for large payloads it chains the base buffer and overflow chunks. */ public ContentStreamProvider contentStreamProvider() { close(); - byte[] buf = baos.buf(); - int count = baos.size(); - return () -> new ByteArrayInputStream(buf, 0, count); + return baos.contentStreamProvider(); } @Override From b4ae8a7b4c6f63b81676b5d9af5307ba56022241 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Wed, 29 Apr 2026 10:57:10 -0700 Subject: [PATCH 07/12] Cleanups --- .../feature-AWSSDKforJavav2-439f346.json | 4 +- .../json/ExposedByteArrayOutputStream.java | 51 ------------ .../json/SdkByteArrayOutputStream.java | 36 ++++++++- .../json/StructuredJsonGenerator.java | 31 ++++++++ .../marshall/JsonProtocolMarshaller.java | 21 ++--- .../ExposedByteArrayOutputStreamTest.java | 79 ------------------- .../PayloadMarshallingEquivalenceTest.java | 46 +++++------ 7 files changed, 96 insertions(+), 172 deletions(-) delete mode 100644 core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/ExposedByteArrayOutputStream.java delete mode 100644 core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/ExposedByteArrayOutputStreamTest.java diff --git a/.changes/next-release/feature-AWSSDKforJavav2-439f346.json b/.changes/next-release/feature-AWSSDKforJavav2-439f346.json index 46ea293d42cc..f47e873c3cdf 100644 --- a/.changes/next-release/feature-AWSSDKforJavav2-439f346.json +++ b/.changes/next-release/feature-AWSSDKforJavav2-439f346.json @@ -2,5 +2,5 @@ "type": "feature", "category": "AWS SDK for Java v2", "contributor": "", - "description": "Optimized JSON marshalling performance for JSON RPC and REST JSON protocols." -} + "description": "Optimized JSON marshalling performance for JSON RPC, REST JSON and RPCv2 Cbor protocols." +} \ No newline at end of file diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/ExposedByteArrayOutputStream.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/ExposedByteArrayOutputStream.java deleted file mode 100644 index ce321c373377..000000000000 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/ExposedByteArrayOutputStream.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * 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; - -import java.io.ByteArrayOutputStream; -import software.amazon.awssdk.annotations.SdkInternalApi; - -/** - * A thin subclass of {@link ByteArrayOutputStream} that exposes the internal buffer and count - * without copying. This allows {@link SdkJsonGenerator} to create a {@code ContentStreamProvider} - * that wraps the buffer directly via {@code ByteArrayInputStream(buf, 0, count)}, avoiding the - * contiguous copy that {@link ByteArrayOutputStream#toByteArray()} performs. - * - *

The write path is identical to {@code ByteArrayOutputStream} — no overhead is added. - * Only the final "get the bytes" step is optimized. - * - *

This class is not thread-safe. - */ -@SdkInternalApi -final class ExposedByteArrayOutputStream extends ByteArrayOutputStream { - - ExposedByteArrayOutputStream(int size) { - super(size); - } - - /** - * Returns the internal buffer. The valid data is in {@code buf[0..count-1]}. - * The returned array may be larger than {@link #size()}; callers must use - * {@link #size()} to determine the valid range. - * - *

Warning: The returned array is the live internal buffer. Do not modify it, - * and do not write to this stream after capturing the reference — the buffer may be - * replaced by a larger one on the next write if growth is needed. - */ - byte[] buf() { - return buf; - } -} diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStream.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStream.java index bcd41fc4d441..000981e00a12 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStream.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStream.java @@ -17,7 +17,9 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; import java.io.SequenceInputStream; import java.util.ArrayList; import java.util.Collections; @@ -135,6 +137,38 @@ public byte[] toByteArray() { return result; } + /** + * Resets this stream so that all currently accumulated output is discarded, including any + * overflow chunks. After calling this method, the stream can be reused as if freshly constructed. + */ + @Override + public void reset() { + super.reset(); + overflowing = false; + overflowChunks = null; + overflowChunkOffset = 0; + overflowTotalBytes = 0; + } + + /** + * Writes the complete contents of this stream to the specified output stream, including + * any overflow chunks. + */ + @Override + public void writeTo(OutputStream out) throws IOException { + if (!overflowing) { + super.writeTo(out); + return; + } + // Write base buffer + out.write(buf, 0, count); + // Write overflow chunks + for (int i = 0; i < overflowChunks.size(); i++) { + int len = (i < overflowChunks.size() - 1) ? overflowChunks.get(i).length : overflowChunkOffset; + out.write(overflowChunks.get(i), 0, len); + } + } + /** * Returns a {@link ContentStreamProvider} that streams directly from the internal buffers * without creating a contiguous copy. For small payloads this wraps the single base buffer; @@ -143,7 +177,7 @@ public byte[] toByteArray() { */ ContentStreamProvider contentStreamProvider() { if (!overflowing) { - // Small payload: single buffer, wrap directly (same as ExposedByteArrayOutputStream) + // Small payload: single buffer, wrap directly byte[] b = buf; int c = count; return () -> new ByteArrayInputStream(b, 0, c); diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/StructuredJsonGenerator.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/StructuredJsonGenerator.java index db883f8c5325..5345305ffcd5 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/StructuredJsonGenerator.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/StructuredJsonGenerator.java @@ -15,11 +15,13 @@ package software.amazon.awssdk.protocols.json; +import java.io.ByteArrayInputStream; import java.math.BigDecimal; import java.math.BigInteger; import java.nio.ByteBuffer; import java.time.Instant; import software.amazon.awssdk.annotations.SdkProtectedApi; +import software.amazon.awssdk.http.ContentStreamProvider; /** * Interface for generating a JSON @@ -102,6 +104,11 @@ public StructuredJsonGenerator writeValue(ByteBuffer bytes) { return this; } + @Override + public StructuredJsonGenerator writeBinaryValue(byte[] bytes) { + return this; + } + @Override public StructuredJsonGenerator writeValue(Instant instant) { return this; @@ -193,4 +200,28 @@ default StructuredJsonGenerator writeBinaryValue(byte[] bytes) { */ @Deprecated String getContentType(); + + /** + * Returns the size of the generated content in bytes without copying. The default + * implementation falls back to {@link #getBytes()}.length. + */ + default int contentSize() { + byte[] bytes = getBytes(); + return bytes == null ? 0 : bytes.length; + } + + /** + * Returns a {@link ContentStreamProvider} that streams the generated content. The default + * implementation wraps the result of {@link #getBytes()} in a {@code ByteArrayInputStream}. + * Implementations may override this to stream directly from internal buffers without copying. + * + * @return a content stream provider, or {@code null} if {@link #getBytes()} returns null + */ + default ContentStreamProvider contentStreamProvider() { + byte[] bytes = getBytes(); + if (bytes == null) { + return null; + } + return () -> new ByteArrayInputStream(bytes); + } } diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java index 8db37cfc530c..0d95d5d23377 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java @@ -53,7 +53,6 @@ import software.amazon.awssdk.protocols.json.AwsJsonProtocol; import software.amazon.awssdk.protocols.json.AwsJsonProtocolMetadata; import software.amazon.awssdk.protocols.json.BaseAwsJsonProtocolFactory; -import software.amazon.awssdk.protocols.json.SdkJsonGenerator; import software.amazon.awssdk.protocols.json.StructuredJsonGenerator; import software.amazon.awssdk.protocols.json.internal.ProtocolFact; @@ -70,6 +69,8 @@ public class JsonProtocolMarshaller implements ProtocolMarshaller, JsonMarshaller> MARSHALLER_CACHE = new ConcurrentHashMap<>(); @@ -290,25 +291,13 @@ private SdkHttpFullRequest finishMarshalling() { jsonGenerator.writeEndObject(); } - if (jsonGenerator instanceof SdkJsonGenerator) { - // Optimized path: stream directly from chunked buffers, avoiding a single - // contiguous byte[] allocation that can cause G1GC humongous allocations. - SdkJsonGenerator sdkGenerator = (SdkJsonGenerator) jsonGenerator; - ContentStreamProvider contentProvider = sdkGenerator.contentStreamProvider(); + ContentStreamProvider contentProvider = jsonGenerator.contentStreamProvider(); + if (contentProvider != null) { request.contentStreamProvider(contentProvider); - int contentSize = sdkGenerator.contentSize(); + int contentSize = jsonGenerator.contentSize(); if (contentSize > 0) { request.putHeader(CONTENT_LENGTH, Integer.toString(contentSize)); } - } else { - byte[] content = jsonGenerator.getBytes(); - - if (content != null) { - request.contentStreamProvider(() -> new ByteArrayInputStream(content)); - if (content.length > 0) { - request.putHeader(CONTENT_LENGTH, Integer.toString(content.length)); - } - } } } diff --git a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/ExposedByteArrayOutputStreamTest.java b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/ExposedByteArrayOutputStreamTest.java deleted file mode 100644 index 81980dca9cc4..000000000000 --- a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/ExposedByteArrayOutputStreamTest.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * 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; - -import static org.assertj.core.api.Assertions.assertThat; - -import java.util.Arrays; -import org.junit.jupiter.api.Test; - -class ExposedByteArrayOutputStreamTest { - - @Test - void emptyStream_hasZeroSize() { - ExposedByteArrayOutputStream stream = new ExposedByteArrayOutputStream(64); - assertThat(stream.size()).isEqualTo(0); - assertThat(stream.toByteArray()).isEmpty(); - } - - @Test - void buf_returnsInternalBuffer() { - ExposedByteArrayOutputStream stream = new ExposedByteArrayOutputStream(64); - byte[] data = {1, 2, 3, 4, 5}; - stream.write(data, 0, data.length); - - byte[] buf = stream.buf(); - // buf is the live internal buffer — it may be larger than size() - assertThat(buf.length).isGreaterThanOrEqualTo(stream.size()); - // The valid data in buf[0..size()-1] matches what was written - assertThat(Arrays.copyOf(buf, stream.size())).isEqualTo(data); - } - - @Test - void buf_reflectsWrittenData_afterGrowth() { - // Start with a tiny buffer to force growth - ExposedByteArrayOutputStream stream = new ExposedByteArrayOutputStream(4); - byte[] data = new byte[100]; - for (int i = 0; i < data.length; i++) { - data[i] = (byte) i; - } - stream.write(data, 0, data.length); - - byte[] buf = stream.buf(); - assertThat(stream.size()).isEqualTo(100); - assertThat(Arrays.copyOf(buf, stream.size())).isEqualTo(data); - } - - @Test - void toByteArray_returnsCopy_notSameReference() { - ExposedByteArrayOutputStream stream = new ExposedByteArrayOutputStream(64); - stream.write(new byte[]{1, 2, 3}, 0, 3); - - byte[] copy = stream.toByteArray(); - byte[] buf = stream.buf(); - // toByteArray returns a copy, buf returns the live buffer - assertThat(copy).isNotSameAs(buf); - assertThat(copy).isEqualTo(Arrays.copyOf(buf, stream.size())); - } - - @Test - void singleByteWrite_worksCorrectly() { - ExposedByteArrayOutputStream stream = new ExposedByteArrayOutputStream(64); - stream.write(0x42); - assertThat(stream.size()).isEqualTo(1); - assertThat(stream.buf()[0]).isEqualTo((byte) 0x42); - } -} diff --git a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/PayloadMarshallingEquivalenceTest.java b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/PayloadMarshallingEquivalenceTest.java index a83192056584..5fe211f857c2 100644 --- a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/PayloadMarshallingEquivalenceTest.java +++ b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/PayloadMarshallingEquivalenceTest.java @@ -73,7 +73,7 @@ class PayloadMarshallingEquivalenceTest { // ---- STRING ---- @Test - void string_producesCorrectJson() { + void marshallPayloadField_withStringValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.STRING, obj -> "hello world"); String body = marshallAndGetBody(field); assertThat(body).contains("\"fieldName\":\"hello world\""); @@ -82,7 +82,7 @@ void string_producesCorrectJson() { // ---- INTEGER ---- @Test - void integer_producesCorrectJson() { + void marshallPayloadField_withIntegerValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.INTEGER, obj -> 42); String body = marshallAndGetBody(field); assertThat(body).contains("\"fieldName\":42"); @@ -91,7 +91,7 @@ void integer_producesCorrectJson() { // ---- LONG ---- @Test - void long_producesCorrectJson() { + void marshallPayloadField_withLongValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.LONG, obj -> 123456789L); String body = marshallAndGetBody(field); assertThat(body).contains("\"fieldName\":123456789"); @@ -100,7 +100,7 @@ void long_producesCorrectJson() { // ---- SHORT ---- @Test - void short_producesCorrectJson() { + void marshallPayloadField_withShortValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.SHORT, obj -> (short) 7); String body = marshallAndGetBody(field); assertThat(body).contains("\"fieldName\":7"); @@ -109,7 +109,7 @@ void short_producesCorrectJson() { // ---- BYTE ---- @Test - void byte_producesCorrectJson() { + void marshallPayloadField_withByteValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.BYTE, obj -> (byte) 3); String body = marshallAndGetBody(field); assertThat(body).contains("\"fieldName\":3"); @@ -118,7 +118,7 @@ void byte_producesCorrectJson() { // ---- FLOAT ---- @Test - void float_producesCorrectJson() { + void marshallPayloadField_withFloatValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.FLOAT, obj -> 1.5f); String body = marshallAndGetBody(field); assertThat(body).contains("\"fieldName\":1.5"); @@ -127,7 +127,7 @@ void float_producesCorrectJson() { // ---- DOUBLE ---- @Test - void double_producesCorrectJson() { + void marshallPayloadField_withDoubleValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.DOUBLE, obj -> 3.14); String body = marshallAndGetBody(field); assertThat(body).contains("\"fieldName\":3.14"); @@ -136,7 +136,7 @@ void double_producesCorrectJson() { // ---- BIG_DECIMAL ---- @Test - void bigDecimal_producesCorrectJson() { + void marshallPayloadField_withBigDecimalValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.BIG_DECIMAL, obj -> new BigDecimal("99.99")); String body = marshallAndGetBody(field); @@ -147,7 +147,7 @@ void bigDecimal_producesCorrectJson() { // ---- BOOLEAN ---- @Test - void boolean_producesCorrectJson() { + void marshallPayloadField_withBooleanValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.BOOLEAN, obj -> true); String body = marshallAndGetBody(field); assertThat(body).contains("\"fieldName\":true"); @@ -156,7 +156,7 @@ void boolean_producesCorrectJson() { // ---- INSTANT (default format — UNIX_TIMESTAMP for PAYLOAD) ---- @Test - void instant_defaultFormat_producesUnixTimestamp() { + void marshallPayloadField_withInstantDefaultFormat_producesUnixTimestamp() { SdkField field = payloadField("fieldName", MarshallingType.INSTANT, obj -> Instant.ofEpochSecond(1000)); String body = marshallAndGetBody(field); @@ -169,7 +169,7 @@ void instant_defaultFormat_producesUnixTimestamp() { // ---- INSTANT with UNIX_TIMESTAMP trait ---- @Test - void instant_unixTimestampTrait_producesUnixTimestamp() { + void marshallPayloadField_withInstantUnixTimestampTrait_producesUnixTimestamp() { SdkField field = SdkField.builder(MarshallingType.INSTANT) .memberName("fieldName") .getter(obj -> Instant.ofEpochSecond(1000)) @@ -188,7 +188,7 @@ void instant_unixTimestampTrait_producesUnixTimestamp() { // ---- INSTANT with RFC_822 trait ---- @Test - void instant_rfc822Trait_producesRfc822String() { + void marshallPayloadField_withInstantRfc822Trait_producesRfc822String() { SdkField field = SdkField.builder(MarshallingType.INSTANT) .memberName("fieldName") .getter(obj -> Instant.ofEpochSecond(1000)) @@ -208,7 +208,7 @@ void instant_rfc822Trait_producesRfc822String() { // ---- INSTANT with ISO_8601 trait ---- @Test - void instant_iso8601Trait_producesIso8601String() { + void marshallPayloadField_withInstantIso8601Trait_producesIso8601String() { SdkField field = SdkField.builder(MarshallingType.INSTANT) .memberName("fieldName") .getter(obj -> Instant.ofEpochSecond(1000)) @@ -228,7 +228,7 @@ void instant_iso8601Trait_producesIso8601String() { // ---- SDK_BYTES ---- @Test - void sdkBytes_producesBase64EncodedJson() { + void marshallPayloadField_withSdkBytesValue_producesBase64EncodedJson() { SdkField field = payloadField("fieldName", MarshallingType.SDK_BYTES, obj -> SdkBytes.fromUtf8String("data")); String body = marshallAndGetBody(field); @@ -239,7 +239,7 @@ void sdkBytes_producesBase64EncodedJson() { // ---- SDK_POJO (nested) ---- @Test - void sdkPojo_producesNestedObjectJson() { + void marshallPayloadField_withSdkPojoValue_producesNestedObjectJson() { // Inner pojo with a single string field SdkField innerField = payloadField("innerField", MarshallingType.STRING, obj -> "innerValue"); SimplePojo innerPojo = new SimplePojo(innerField); @@ -262,7 +262,7 @@ void sdkPojo_producesNestedObjectJson() { // ---- LIST (non-empty) ---- @Test - void list_nonEmpty_producesArrayJson() { + void marshallPayloadField_withNonEmptyList_producesArrayJson() { List listValue = Arrays.asList("a", "b", "c"); SdkField memberField = SdkField.builder(MarshallingType.STRING) @@ -295,7 +295,7 @@ void list_nonEmpty_producesArrayJson() { // ---- LIST (empty SdkAutoConstructList — should be skipped) ---- @Test - void list_emptySdkAutoConstructList_isSkipped() { + void marshallPayloadField_withEmptySdkAutoConstructList_isSkipped() { List autoList = DefaultSdkAutoConstructList.getInstance(); SdkField memberField = SdkField.builder(MarshallingType.STRING) @@ -328,7 +328,7 @@ void list_emptySdkAutoConstructList_isSkipped() { // ---- LIST (empty regular list — should emit empty array) ---- @Test - void list_emptyRegularList_producesEmptyArray() { + void marshallPayloadField_withEmptyRegularList_producesEmptyArray() { List emptyList = new ArrayList<>(); SdkField memberField = SdkField.builder(MarshallingType.STRING) @@ -361,7 +361,7 @@ void list_emptyRegularList_producesEmptyArray() { // ---- MAP (non-empty) ---- @Test - void map_nonEmpty_producesObjectJson() { + void marshallPayloadField_withNonEmptyMap_producesObjectJson() { // Use LinkedHashMap for deterministic ordering Map mapValue = new LinkedHashMap<>(); mapValue.put("key1", "val1"); @@ -397,7 +397,7 @@ void map_nonEmpty_producesObjectJson() { // ---- MAP (empty SdkAutoConstructMap — should be skipped) ---- @Test - void map_emptySdkAutoConstructMap_isSkipped() { + void marshallPayloadField_withEmptySdkAutoConstructMap_isSkipped() { Map autoMap = DefaultSdkAutoConstructMap.getInstance(); SdkField valueField = SdkField.builder(MarshallingType.STRING) @@ -430,7 +430,7 @@ void map_emptySdkAutoConstructMap_isSkipped() { // ---- MAP (empty regular map — should emit empty object) ---- @Test - void map_emptyRegularMap_producesEmptyObject() { + void marshallPayloadField_withEmptyRegularMap_producesEmptyObject() { Map emptyMap = new HashMap<>(); SdkField valueField = SdkField.builder(MarshallingType.STRING) @@ -463,7 +463,7 @@ void map_emptyRegularMap_producesEmptyObject() { // ---- MAP with null value entry — entry is skipped ---- @Test - void map_nullValueEntry_isSkipped() { + void marshallPayloadField_withMapNullValueEntry_isSkipped() { Map mapValue = new LinkedHashMap<>(); mapValue.put("key1", "val1"); mapValue.put("key2", null); @@ -501,7 +501,7 @@ void map_nullValueEntry_isSkipped() { // ---- DOCUMENT ---- @Test - void document_producesCorrectJson() { + void marshallPayloadField_withDocumentValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.DOCUMENT, obj -> Document.fromString("test")); String body = marshallAndGetBody(field); From e93c62f25aeebf4adba5130e98d70eab8dc3a807 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Wed, 29 Apr 2026 12:06:42 -0700 Subject: [PATCH 08/12] Improve testing --- .../json/SdkByteArrayOutputStream.java | 32 +- .../json/SdkByteArrayOutputStreamTest.java | 354 ++++++++++++++++++ .../CachedNonPayloadMarshallingTest.java | 5 - .../PayloadMarshallingEquivalenceTest.java | 66 +--- ...knownMarshallingKnownTypeFallbackTest.java | 18 - 5 files changed, 378 insertions(+), 97 deletions(-) create mode 100644 core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStreamTest.java diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStream.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStream.java index 000981e00a12..a2c915b55e90 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStream.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStream.java @@ -24,46 +24,28 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import software.amazon.awssdk.annotations.NotThreadSafe; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.http.ContentStreamProvider; /** * A {@link ByteArrayOutputStream} subclass that behaves identically to the JDK implementation for - * small payloads, but caps internal buffer growth to avoid G1GC humongous object allocations for + * small payloads, but caps internal buffer growth to avoid large object allocations for * large payloads. * - *

How it works: Writes flow into the inherited {@code ByteArrayOutputStream} buffer + *

+ * Writes flow into the inherited {@code ByteArrayOutputStream} buffer * normally. When a write would cause the buffer to grow beyond {@link #MAX_BUFFER_SIZE}, the * current buffer contents are frozen into the first "chunk" and subsequent writes go into * fixed-size overflow chunks ({@link #CHUNK_SIZE} bytes each). No single allocation ever exceeds * {@code MAX_BUFFER_SIZE}. * - *

Performance characteristics: - *

    - *
  • Payloads ≤ {@code MAX_BUFFER_SIZE}: Identical to {@code ByteArrayOutputStream} — the - * JIT can inline and optimize the write path exactly as it does for the stock class. Zero - * overhead.
    • - *
  • Payloads > {@code MAX_BUFFER_SIZE}: Overflow writes go through a simple chunked path. - * This is slightly slower per-byte than {@code ByteArrayOutputStream}'s doubling strategy, - * but avoids allocations that exceed half the G1 region size.
    • - *
- * - *

This class is not thread-safe. */ +@NotThreadSafe @SdkInternalApi final class SdkByteArrayOutputStream extends ByteArrayOutputStream { - - /** - * Maximum size of the primary ByteArrayOutputStream buffer before overflow kicks in. - * Chosen to be well below the G1 humongous threshold (region_size / 2). With a 4 GB heap - * the default region size is 2 MB, so humongous threshold is 1 MB. 128 KB is safely below - * that for any reasonable heap size (humongous threshold is 512 KB for a 256 MB heap). - */ - static final int MAX_BUFFER_SIZE = 128 * 1024; - - /** - * Size of each overflow chunk. 64 KB is well below any G1 humongous threshold. - */ + // 128 KB, choosen to be well below 1 MB "humongous threshold" for most heap sizes + static final int MAX_BUFFER_SIZE = 128 * 1024; static final int CHUNK_SIZE = 64 * 1024; private List overflowChunks; diff --git a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStreamTest.java b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStreamTest.java new file mode 100644 index 000000000000..9ac3120df1c4 --- /dev/null +++ b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStreamTest.java @@ -0,0 +1,354 @@ +/* + * 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; + +import static org.assertj.core.api.Assertions.assertThat; +import static software.amazon.awssdk.protocols.json.SdkByteArrayOutputStream.CHUNK_SIZE; +import static software.amazon.awssdk.protocols.json.SdkByteArrayOutputStream.MAX_BUFFER_SIZE; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.Arrays; +import java.util.Random; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.http.ContentStreamProvider; + +/** + * Unit tests for {@link SdkByteArrayOutputStream}, covering both the small-payload (base buffer) + * and large-payload (overflow) paths. + */ +class SdkByteArrayOutputStreamTest { + + private static final Random RANDOM = new Random(42); + + @Test + void write_smallPayload_behavesLikeByteArrayOutputStream() { + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(64); + byte[] data = randomBytes(1000); + stream.write(data, 0, data.length); + + assertThat(stream.size()).isEqualTo(1000); + assertThat(stream.toByteArray()).isEqualTo(data); + } + + @Test + void write_singleBytes_smallPayload_behavesCorrectly() { + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(16); + for (int i = 0; i < 256; i++) { + stream.write(i); + } + + assertThat(stream.size()).isEqualTo(256); + byte[] result = stream.toByteArray(); + for (int i = 0; i < 256; i++) { + assertThat(result[i]).isEqualTo((byte) i); + } + } + + @Test + void size_emptyStream_returnsZero() { + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(64); + assertThat(stream.size()).isEqualTo(0); + assertThat(stream.toByteArray()).isEmpty(); + } + + @Test + void write_exactlyMaxBufferSize_doesNotOverflow() { + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); + byte[] data = randomBytes(MAX_BUFFER_SIZE); + stream.write(data, 0, data.length); + + assertThat(stream.size()).isEqualTo(MAX_BUFFER_SIZE); + assertThat(stream.toByteArray()).isEqualTo(data); + } + + @Test + void write_oneBytePastMaxBufferSize_triggersOverflow() { + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); + byte[] data = randomBytes(MAX_BUFFER_SIZE + 1); + stream.write(data, 0, data.length); + + assertThat(stream.size()).isEqualTo(MAX_BUFFER_SIZE + 1); + assertThat(stream.toByteArray()).isEqualTo(data); + } + + @Test + void write_singleByte_triggersOverflow() { + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); + byte[] base = randomBytes(MAX_BUFFER_SIZE); + stream.write(base, 0, base.length); + + // This single byte should trigger overflow + stream.write(0xFF); + + assertThat(stream.size()).isEqualTo(MAX_BUFFER_SIZE + 1); + byte[] result = stream.toByteArray(); + assertThat(Arrays.copyOf(result, MAX_BUFFER_SIZE)).isEqualTo(base); + assertThat(result[MAX_BUFFER_SIZE]).isEqualTo((byte) 0xFF); + } + + @Test + void write_largePayload_multipleChunks_producesCorrectOutput() { + // Write enough to span the base buffer + multiple overflow chunks + int totalSize = MAX_BUFFER_SIZE + (CHUNK_SIZE * 3) + 100; + byte[] data = randomBytes(totalSize); + + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); + stream.write(data, 0, data.length); + + assertThat(stream.size()).isEqualTo(totalSize); + assertThat(stream.toByteArray()).isEqualTo(data); + } + + @Test + void write_largePayload_incrementalWrites_producesCorrectOutput() { + // Write in small increments that cross chunk boundaries + int totalSize = MAX_BUFFER_SIZE + (CHUNK_SIZE * 2) + 500; + byte[] data = randomBytes(totalSize); + + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); + int offset = 0; + int chunkSize = 1337; // Deliberately not aligned to CHUNK_SIZE + while (offset < data.length) { + int len = Math.min(chunkSize, data.length - offset); + stream.write(data, offset, len); + offset += len; + } + + assertThat(stream.size()).isEqualTo(totalSize); + assertThat(stream.toByteArray()).isEqualTo(data); + } + + @Test + void write_singleBytes_intoOverflow_producesCorrectOutput() { + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); + byte[] base = randomBytes(MAX_BUFFER_SIZE); + stream.write(base, 0, base.length); + + // Write 200 single bytes into overflow + byte[] overflow = new byte[200]; + for (int i = 0; i < 200; i++) { + overflow[i] = (byte) (i & 0xFF); + stream.write(overflow[i]); + } + + assertThat(stream.size()).isEqualTo(MAX_BUFFER_SIZE + 200); + byte[] result = stream.toByteArray(); + assertThat(Arrays.copyOf(result, MAX_BUFFER_SIZE)).isEqualTo(base); + assertThat(Arrays.copyOfRange(result, MAX_BUFFER_SIZE, result.length)).isEqualTo(overflow); + } + + @Test + void contentSize_smallPayload_matchesSize() { + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(64); + byte[] data = randomBytes(500); + stream.write(data, 0, data.length); + + assertThat(stream.contentSize()).isEqualTo(500); + assertThat(stream.contentSize()).isEqualTo(stream.size()); + } + + @Test + void contentSize_largePayload_matchesSize() { + int totalSize = MAX_BUFFER_SIZE + CHUNK_SIZE + 100; + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); + stream.write(randomBytes(totalSize), 0, totalSize); + + assertThat(stream.contentSize()).isEqualTo(totalSize); + assertThat(stream.contentSize()).isEqualTo(stream.size()); + } + + @Test + void contentStreamProvider_smallPayload_producesSameBytesAsToByteArray() throws IOException { + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(64); + byte[] data = randomBytes(5000); + stream.write(data, 0, data.length); + + ContentStreamProvider provider = stream.contentStreamProvider(); + byte[] streamed = readAllBytes(provider.newStream()); + + assertThat(streamed).isEqualTo(data); + } + + @Test + void contentStreamProvider_largePayload_producesSameBytesAsToByteArray() throws IOException { + int totalSize = MAX_BUFFER_SIZE + (CHUNK_SIZE * 2) + 500; + byte[] data = randomBytes(totalSize); + + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); + stream.write(data, 0, data.length); + + byte[] expected = stream.toByteArray(); + ContentStreamProvider provider = stream.contentStreamProvider(); + byte[] streamed = readAllBytes(provider.newStream()); + + assertThat(streamed).isEqualTo(expected); + } + + @Test + void contentStreamProvider_isResettable_smallPayload() throws IOException { + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(64); + byte[] data = randomBytes(100); + stream.write(data, 0, data.length); + + ContentStreamProvider provider = stream.contentStreamProvider(); + byte[] first = readAllBytes(provider.newStream()); + byte[] second = readAllBytes(provider.newStream()); + + assertThat(first).isEqualTo(data); + assertThat(second).isEqualTo(data); + } + + @Test + void contentStreamProvider_isResettable_largePayload() throws IOException { + int totalSize = MAX_BUFFER_SIZE + CHUNK_SIZE + 100; + byte[] data = randomBytes(totalSize); + + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); + stream.write(data, 0, data.length); + + ContentStreamProvider provider = stream.contentStreamProvider(); + byte[] first = readAllBytes(provider.newStream()); + byte[] second = readAllBytes(provider.newStream()); + + assertThat(first).isEqualTo(data); + assertThat(second).isEqualTo(data); + } + + @Test + void contentStreamProvider_emptyStream_producesEmptyContent() throws IOException { + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(64); + ContentStreamProvider provider = stream.contentStreamProvider(); + byte[] content = readAllBytes(provider.newStream()); + + assertThat(content).isEmpty(); + } + + @Test + void reset_smallPayload_clearsAllData() { + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(64); + stream.write(new byte[]{1, 2, 3}, 0, 3); + + stream.reset(); + + assertThat(stream.size()).isEqualTo(0); + assertThat(stream.toByteArray()).isEmpty(); + } + + @Test + void reset_afterOverflow_clearsAllState() { + int totalSize = MAX_BUFFER_SIZE + CHUNK_SIZE + 100; + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); + stream.write(randomBytes(totalSize), 0, totalSize); + + assertThat(stream.size()).isEqualTo(totalSize); + + stream.reset(); + + assertThat(stream.size()).isEqualTo(0); + assertThat(stream.toByteArray()).isEmpty(); + assertThat(stream.contentSize()).isEqualTo(0); + } + + @Test + void reset_afterOverflow_allowsReuse() { + int totalSize = MAX_BUFFER_SIZE + CHUNK_SIZE + 100; + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); + stream.write(randomBytes(totalSize), 0, totalSize); + + stream.reset(); + + // Write new data after reset + byte[] newData = randomBytes(500); + stream.write(newData, 0, newData.length); + + assertThat(stream.size()).isEqualTo(500); + assertThat(stream.toByteArray()).isEqualTo(newData); + } + + @Test + void reset_afterOverflow_thenWriteLargeAgain_producesCorrectOutput() { + int totalSize = MAX_BUFFER_SIZE + 500; + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); + stream.write(randomBytes(totalSize), 0, totalSize); + + stream.reset(); + + // Write a different large payload + byte[] newData = randomBytes(MAX_BUFFER_SIZE + 1000); + stream.write(newData, 0, newData.length); + + assertThat(stream.size()).isEqualTo(newData.length); + assertThat(stream.toByteArray()).isEqualTo(newData); + } + + @Test + void writeTo_smallPayload_writesAllData() throws IOException { + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(64); + byte[] data = randomBytes(500); + stream.write(data, 0, data.length); + + ByteArrayOutputStream target = new ByteArrayOutputStream(); + stream.writeTo(target); + + assertThat(target.toByteArray()).isEqualTo(data); + } + + @Test + void writeTo_largePayload_writesAllData() throws IOException { + int totalSize = MAX_BUFFER_SIZE + (CHUNK_SIZE * 2) + 500; + byte[] data = randomBytes(totalSize); + + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); + stream.write(data, 0, data.length); + + ByteArrayOutputStream target = new ByteArrayOutputStream(); + stream.writeTo(target); + + assertThat(target.toByteArray()).isEqualTo(data); + } + + @Test + void writeTo_afterOverflow_matchesToByteArray() throws IOException { + int totalSize = MAX_BUFFER_SIZE + CHUNK_SIZE + 100; + byte[] data = randomBytes(totalSize); + + SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); + stream.write(data, 0, data.length); + + ByteArrayOutputStream target = new ByteArrayOutputStream(); + stream.writeTo(target); + + assertThat(target.toByteArray()).isEqualTo(stream.toByteArray()); + } + + private static byte[] randomBytes(int length) { + byte[] data = new byte[length]; + RANDOM.nextBytes(data); + return data; + } + + private static byte[] readAllBytes(InputStream is) throws IOException { + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + byte[] buf = new byte[4096]; + int n; + while ((n = is.read(buf)) != -1) { + bos.write(buf, 0, n); + } + return bos.toByteArray(); + } +} diff --git a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/CachedNonPayloadMarshallingTest.java b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/CachedNonPayloadMarshallingTest.java index 929ae22c38ce..906330fc35c9 100644 --- a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/CachedNonPayloadMarshallingTest.java +++ b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/CachedNonPayloadMarshallingTest.java @@ -40,9 +40,6 @@ * 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. - * - *

Validates: Property 3 — Cached non-payload marshalling equivalence

- *

Validates: Requirements 7.3, 7.4

*/ class CachedNonPayloadMarshallingTest { @@ -108,8 +105,6 @@ void queryParam_string_producesCorrectQueryParam() { .containsExactly("paramValue"); } - // ---- Helper methods ---- - private static SdkField headerField(String headerName, java.util.function.Function getter) { return SdkField.builder(MarshallingType.STRING) diff --git a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/PayloadMarshallingEquivalenceTest.java b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/PayloadMarshallingEquivalenceTest.java index 5fe211f857c2..339fa9f1e8a3 100644 --- a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/PayloadMarshallingEquivalenceTest.java +++ b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/PayloadMarshallingEquivalenceTest.java @@ -22,6 +22,7 @@ import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; +import java.util.Base64; import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; @@ -51,10 +52,7 @@ /** * Tests that the switch-based payload dispatch in {@link JsonProtocolMarshaller#marshallPayloadField} - * produces correct JSON output for all 16 {@code MarshallingKnownType} values. - * - *

Validates: Property 1 — Payload marshalling behavioral equivalence

- *

Validates: Requirements 2.1–2.12, 3.1–3.5, 4.1, 5.1–5.3, 6.1–6.4

+ * produces correct output for all 16 {@code MarshallingKnownType} values. */ class PayloadMarshallingEquivalenceTest { @@ -70,8 +68,6 @@ class PayloadMarshallingEquivalenceTest { .contentType(CONTENT_TYPE) .build(); - // ---- STRING ---- - @Test void marshallPayloadField_withStringValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.STRING, obj -> "hello world"); @@ -79,8 +75,6 @@ void marshallPayloadField_withStringValue_producesCorrectJson() { assertThat(body).contains("\"fieldName\":\"hello world\""); } - // ---- INTEGER ---- - @Test void marshallPayloadField_withIntegerValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.INTEGER, obj -> 42); @@ -88,8 +82,6 @@ void marshallPayloadField_withIntegerValue_producesCorrectJson() { assertThat(body).contains("\"fieldName\":42"); } - // ---- LONG ---- - @Test void marshallPayloadField_withLongValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.LONG, obj -> 123456789L); @@ -97,8 +89,6 @@ void marshallPayloadField_withLongValue_producesCorrectJson() { assertThat(body).contains("\"fieldName\":123456789"); } - // ---- SHORT ---- - @Test void marshallPayloadField_withShortValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.SHORT, obj -> (short) 7); @@ -106,8 +96,6 @@ void marshallPayloadField_withShortValue_producesCorrectJson() { assertThat(body).contains("\"fieldName\":7"); } - // ---- BYTE ---- - @Test void marshallPayloadField_withByteValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.BYTE, obj -> (byte) 3); @@ -115,8 +103,6 @@ void marshallPayloadField_withByteValue_producesCorrectJson() { assertThat(body).contains("\"fieldName\":3"); } - // ---- FLOAT ---- - @Test void marshallPayloadField_withFloatValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.FLOAT, obj -> 1.5f); @@ -124,8 +110,6 @@ void marshallPayloadField_withFloatValue_producesCorrectJson() { assertThat(body).contains("\"fieldName\":1.5"); } - // ---- DOUBLE ---- - @Test void marshallPayloadField_withDoubleValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.DOUBLE, obj -> 3.14); @@ -133,8 +117,6 @@ void marshallPayloadField_withDoubleValue_producesCorrectJson() { assertThat(body).contains("\"fieldName\":3.14"); } - // ---- BIG_DECIMAL ---- - @Test void marshallPayloadField_withBigDecimalValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.BIG_DECIMAL, @@ -144,8 +126,6 @@ void marshallPayloadField_withBigDecimalValue_producesCorrectJson() { assertThat(body).contains("\"fieldName\":\"99.99\""); } - // ---- BOOLEAN ---- - @Test void marshallPayloadField_withBooleanValue_producesCorrectJson() { SdkField field = payloadField("fieldName", MarshallingType.BOOLEAN, obj -> true); @@ -153,8 +133,6 @@ void marshallPayloadField_withBooleanValue_producesCorrectJson() { assertThat(body).contains("\"fieldName\":true"); } - // ---- INSTANT (default format — UNIX_TIMESTAMP for PAYLOAD) ---- - @Test void marshallPayloadField_withInstantDefaultFormat_producesUnixTimestamp() { SdkField field = payloadField("fieldName", MarshallingType.INSTANT, @@ -166,8 +144,6 @@ void marshallPayloadField_withInstantDefaultFormat_producesUnixTimestamp() { assertThat(body).contains("1000"); } - // ---- INSTANT with UNIX_TIMESTAMP trait ---- - @Test void marshallPayloadField_withInstantUnixTimestampTrait_producesUnixTimestamp() { SdkField field = SdkField.builder(MarshallingType.INSTANT) @@ -185,8 +161,6 @@ void marshallPayloadField_withInstantUnixTimestampTrait_producesUnixTimestamp() assertThat(body).contains("1000"); } - // ---- INSTANT with RFC_822 trait ---- - @Test void marshallPayloadField_withInstantRfc822Trait_producesRfc822String() { SdkField field = SdkField.builder(MarshallingType.INSTANT) @@ -205,8 +179,6 @@ void marshallPayloadField_withInstantRfc822Trait_producesRfc822String() { assertThat(body).contains("1970"); } - // ---- INSTANT with ISO_8601 trait ---- - @Test void marshallPayloadField_withInstantIso8601Trait_producesIso8601String() { SdkField field = SdkField.builder(MarshallingType.INSTANT) @@ -225,8 +197,6 @@ void marshallPayloadField_withInstantIso8601Trait_producesIso8601String() { assertThat(body).contains("1970-01-01T"); } - // ---- SDK_BYTES ---- - @Test void marshallPayloadField_withSdkBytesValue_producesBase64EncodedJson() { SdkField field = payloadField("fieldName", MarshallingType.SDK_BYTES, @@ -236,7 +206,21 @@ void marshallPayloadField_withSdkBytesValue_producesBase64EncodedJson() { assertThat(body).contains("\"fieldName\":\"ZGF0YQ==\""); } - // ---- SDK_POJO (nested) ---- + // ---- SDK_BYTES (large — exceeds SdkByteArrayOutputStream.MAX_BUFFER_SIZE) ---- + + @Test + void marshallPayloadField_withLargeSdkBytesValue_producesCorrectBase64() { + // 200 KB of random data — large enough to trigger SdkByteArrayOutputStream overflow + byte[] rawData = new byte[200 * 1024]; + new java.util.Random(12345).nextBytes(rawData); + SdkBytes sdkBytes = SdkBytes.fromByteArray(rawData); + String expectedBase64 = Base64.getEncoder().encodeToString(rawData); + + SdkField field = payloadField("binaryField", MarshallingType.SDK_BYTES, + obj -> sdkBytes); + String body = marshallAndGetBody(field); + assertThat(body).contains("\"binaryField\":\"" + expectedBase64 + "\""); + } @Test void marshallPayloadField_withSdkPojoValue_producesNestedObjectJson() { @@ -259,8 +243,6 @@ void marshallPayloadField_withSdkPojoValue_producesNestedObjectJson() { assertThat(body).contains("\"fieldName\":{\"innerField\":\"innerValue\"}"); } - // ---- LIST (non-empty) ---- - @Test void marshallPayloadField_withNonEmptyList_producesArrayJson() { List listValue = Arrays.asList("a", "b", "c"); @@ -292,8 +274,6 @@ void marshallPayloadField_withNonEmptyList_producesArrayJson() { assertThat(body).contains("\"fieldName\":[\"a\",\"b\",\"c\"]"); } - // ---- LIST (empty SdkAutoConstructList — should be skipped) ---- - @Test void marshallPayloadField_withEmptySdkAutoConstructList_isSkipped() { List autoList = DefaultSdkAutoConstructList.getInstance(); @@ -325,8 +305,6 @@ void marshallPayloadField_withEmptySdkAutoConstructList_isSkipped() { assertThat(body).doesNotContain("fieldName"); } - // ---- LIST (empty regular list — should emit empty array) ---- - @Test void marshallPayloadField_withEmptyRegularList_producesEmptyArray() { List emptyList = new ArrayList<>(); @@ -358,8 +336,6 @@ void marshallPayloadField_withEmptyRegularList_producesEmptyArray() { assertThat(body).contains("\"fieldName\":[]"); } - // ---- MAP (non-empty) ---- - @Test void marshallPayloadField_withNonEmptyMap_producesObjectJson() { // Use LinkedHashMap for deterministic ordering @@ -394,8 +370,6 @@ void marshallPayloadField_withNonEmptyMap_producesObjectJson() { assertThat(body).contains("\"fieldName\":{\"key1\":\"val1\",\"key2\":\"val2\"}"); } - // ---- MAP (empty SdkAutoConstructMap — should be skipped) ---- - @Test void marshallPayloadField_withEmptySdkAutoConstructMap_isSkipped() { Map autoMap = DefaultSdkAutoConstructMap.getInstance(); @@ -427,8 +401,6 @@ void marshallPayloadField_withEmptySdkAutoConstructMap_isSkipped() { assertThat(body).doesNotContain("fieldName"); } - // ---- MAP (empty regular map — should emit empty object) ---- - @Test void marshallPayloadField_withEmptyRegularMap_producesEmptyObject() { Map emptyMap = new HashMap<>(); @@ -460,8 +432,6 @@ void marshallPayloadField_withEmptyRegularMap_producesEmptyObject() { assertThat(body).contains("\"fieldName\":{}"); } - // ---- MAP with null value entry — entry is skipped ---- - @Test void marshallPayloadField_withMapNullValueEntry_isSkipped() { Map mapValue = new LinkedHashMap<>(); @@ -508,8 +478,6 @@ void marshallPayloadField_withDocumentValue_producesCorrectJson() { assertThat(body).contains("\"fieldName\":\"test\""); } - // ---- Helper methods ---- - @SuppressWarnings({"unchecked", "rawtypes"}) private static SdkField payloadField(String name, MarshallingType marshallingType, diff --git a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/UnknownMarshallingKnownTypeFallbackTest.java b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/UnknownMarshallingKnownTypeFallbackTest.java index 6886452c2dc1..e0b4d8e00074 100644 --- a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/UnknownMarshallingKnownTypeFallbackTest.java +++ b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/UnknownMarshallingKnownTypeFallbackTest.java @@ -41,8 +41,6 @@ /** * Tests that when {@code getKnownType()} returns null, the marshaller falls back to the * registry-based path without throwing a {@link NullPointerException} from the switch statement. - * - *

Validates: Requirements 1.3, 1.4

*/ class UnknownMarshallingKnownTypeFallbackTest { @@ -79,15 +77,6 @@ public String toString() { } }; - /** - * Validates Requirement 1.4: When {@code getKnownType()} returns null, the marshaller falls back - * to the registry-based path without throwing a NullPointerException from the switch statement. - * - *

Since the custom type is not registered in the static MARSHALLER_REGISTRY, the registry - * fallback will fail — but the failure must NOT be a NullPointerException from the switch. - * It should be a NullPointerException from invoking {@code .marshall()} on the null result - * returned by the registry lookup (since the custom type is unregistered).

- */ @Test void nullKnownType_fallsBackToRegistryPath_doesNotThrowNpeFromSwitch() { SdkField field = SdkField.builder(CUSTOM_NULL_KNOWN_TYPE) @@ -127,11 +116,6 @@ void nullKnownType_fallsBackToRegistryPath_doesNotThrowNpeFromSwitch() { }); } - /** - * Validates Requirement 1.3: A standard MarshallingType (STRING) with a known type is handled - * by the switch path, confirming the switch dispatch works for recognized types. - * This serves as a control test — if the switch were broken, this would fail too. - */ @Test void knownType_string_isHandledBySwitchPath() { SdkField field = SdkField.builder(MarshallingType.STRING) @@ -151,8 +135,6 @@ void knownType_string_isHandledBySwitchPath() { assertThat(body).contains("\"normalField\":\"hello\""); } - // ---- Helper methods ---- - private static ProtocolMarshaller createMarshaller() { return JsonProtocolMarshallerBuilder.create() .endpoint(ENDPOINT) From 5198d8e6d353b3f991da922d46a76b76d82c6790 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Thu, 30 Apr 2026 08:44:52 -0700 Subject: [PATCH 09/12] Remove computeIfAbsent from the warm path --- .../software/amazon/awssdk/spotbugs-suppressions.xml | 11 +++++++++++ .../internal/marshall/JsonProtocolMarshaller.java | 12 ++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml b/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml index 998bbf8b4139..05606dc6d574 100644 --- a/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml +++ b/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml @@ -536,4 +536,15 @@ + + + + + + + diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java index 0d95d5d23377..8fc991e7ecca 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java @@ -422,8 +422,16 @@ private void marshallFieldViaRegistry(SdkField field, Object val) { .marshall(val, marshallerContext, field.locationName(), (SdkField) field); return; } - JsonMarshaller marshaller = MARSHALLER_CACHE.computeIfAbsent(field, - f -> MARSHALLER_REGISTRY.getMarshaller(f.location(), f.marshallingType(), val)); + // Use get-before-put instead of computeIfAbsent. ConcurrentHashMap.get() is a single lock-free + // volatile read, whereas computeIfAbsent() has additional overhead even on cache hits (bucket-level + // synchronization bookkeeping). The benign-race on first access is safe: SdkField instances are + // static final, and the registry always returns the same marshaller for a given (location, type) pair, + // so concurrent puts are idempotent. + JsonMarshaller marshaller = MARSHALLER_CACHE.get(field); + if (marshaller == null) { + marshaller = MARSHALLER_REGISTRY.getMarshaller(field.location(), field.marshallingType(), val); + MARSHALLER_CACHE.put(field, marshaller); + } marshaller.marshall(val, marshallerContext, field.locationName(), (SdkField) field); } From b71bc567f8d37280457997b1f5635e428718f468 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Thu, 30 Apr 2026 11:50:46 -0700 Subject: [PATCH 10/12] 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) --- .../json/SdkByteArrayOutputStream.java | 225 ----------- .../protocols/json/SdkJsonGenerator.java | 22 +- .../json/StructuredJsonGenerator.java | 26 -- .../marshall/JsonProtocolMarshaller.java | 13 +- .../json/SdkByteArrayOutputStreamTest.java | 354 ------------------ .../protocols/json/SdkJsonGeneratorTest.java | 129 ------- 6 files changed, 8 insertions(+), 761 deletions(-) delete mode 100644 core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStream.java delete mode 100644 core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStreamTest.java diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStream.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStream.java deleted file mode 100644 index a2c915b55e90..000000000000 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStream.java +++ /dev/null @@ -1,225 +0,0 @@ -/* - * 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; - -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.io.SequenceInputStream; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import software.amazon.awssdk.annotations.NotThreadSafe; -import software.amazon.awssdk.annotations.SdkInternalApi; -import software.amazon.awssdk.http.ContentStreamProvider; - -/** - * A {@link ByteArrayOutputStream} subclass that behaves identically to the JDK implementation for - * small payloads, but caps internal buffer growth to avoid large object allocations for - * large payloads. - * - *

- * Writes flow into the inherited {@code ByteArrayOutputStream} buffer - * normally. When a write would cause the buffer to grow beyond {@link #MAX_BUFFER_SIZE}, the - * current buffer contents are frozen into the first "chunk" and subsequent writes go into - * fixed-size overflow chunks ({@link #CHUNK_SIZE} bytes each). No single allocation ever exceeds - * {@code MAX_BUFFER_SIZE}. - * - */ -@NotThreadSafe -@SdkInternalApi -final class SdkByteArrayOutputStream extends ByteArrayOutputStream { - // 128 KB, choosen to be well below 1 MB "humongous threshold" for most heap sizes - static final int MAX_BUFFER_SIZE = 128 * 1024; - static final int CHUNK_SIZE = 64 * 1024; - - private List overflowChunks; - private int overflowChunkOffset; - private int overflowTotalBytes; - private boolean overflowing; - - SdkByteArrayOutputStream(int initialCapacity) { - super(initialCapacity); - } - - @Override - public void write(int b) { - if (overflowing) { - ensureOverflowCapacity(1); - currentOverflowChunk()[overflowChunkOffset++] = (byte) b; - overflowTotalBytes++; - } else if (count + 1 > MAX_BUFFER_SIZE) { - startOverflow(); - write(b); - } else { - super.write(b); - } - } - - @Override - public void write(byte[] b, int off, int len) { - if (overflowing) { - writeToOverflow(b, off, len); - } else if (count + len > MAX_BUFFER_SIZE) { - // Write what fits into the base buffer, then overflow the rest - int fits = MAX_BUFFER_SIZE - count; - if (fits > 0) { - super.write(b, off, fits); - } - startOverflow(); - writeToOverflow(b, off + fits, len - fits); - } else { - super.write(b, off, len); - } - } - - /** - * Returns the total number of bytes written (base buffer + overflow). - */ - @Override - public int size() { - return count + overflowTotalBytes; - } - - /** - * Returns all written data as a single contiguous byte array. Exists for backward - * compatibility via {@link #toByteArray()} but should not be used on the hot path. - */ - @Override - public byte[] toByteArray() { - if (!overflowing) { - return super.toByteArray(); - } - int total = size(); - byte[] result = new byte[total]; - // Copy base buffer - System.arraycopy(buf, 0, result, 0, count); - // Copy overflow chunks - int destOff = count; - for (int i = 0; i < overflowChunks.size(); i++) { - int len = (i < overflowChunks.size() - 1) ? overflowChunks.get(i).length : overflowChunkOffset; - System.arraycopy(overflowChunks.get(i), 0, result, destOff, len); - destOff += len; - } - return result; - } - - /** - * Resets this stream so that all currently accumulated output is discarded, including any - * overflow chunks. After calling this method, the stream can be reused as if freshly constructed. - */ - @Override - public void reset() { - super.reset(); - overflowing = false; - overflowChunks = null; - overflowChunkOffset = 0; - overflowTotalBytes = 0; - } - - /** - * Writes the complete contents of this stream to the specified output stream, including - * any overflow chunks. - */ - @Override - public void writeTo(OutputStream out) throws IOException { - if (!overflowing) { - super.writeTo(out); - return; - } - // Write base buffer - out.write(buf, 0, count); - // Write overflow chunks - for (int i = 0; i < overflowChunks.size(); i++) { - int len = (i < overflowChunks.size() - 1) ? overflowChunks.get(i).length : overflowChunkOffset; - out.write(overflowChunks.get(i), 0, len); - } - } - - /** - * Returns a {@link ContentStreamProvider} that streams directly from the internal buffers - * without creating a contiguous copy. For small payloads this wraps the single base buffer; - * for large payloads it chains the base buffer and overflow chunks via - * {@link SequenceInputStream}. - */ - ContentStreamProvider contentStreamProvider() { - if (!overflowing) { - // Small payload: single buffer, wrap directly - byte[] b = buf; - int c = count; - return () -> new ByteArrayInputStream(b, 0, c); - } - - // Large payload: chain base buffer + overflow chunks - byte[] baseBuf = buf; - int baseCount = count; - List chunks = overflowChunks; - int lastChunkLen = overflowChunkOffset; - - return () -> { - List streams = new ArrayList<>(1 + chunks.size()); - streams.add(new ByteArrayInputStream(baseBuf, 0, baseCount)); - for (int i = 0; i < chunks.size(); i++) { - int len = (i < chunks.size() - 1) ? chunks.get(i).length : lastChunkLen; - streams.add(new ByteArrayInputStream(chunks.get(i), 0, len)); - } - return new SequenceInputStream(Collections.enumeration(streams)); - }; - } - - /** - * Returns the content size without copying. - */ - int contentSize() { - return size(); - } - - private void startOverflow() { - overflowing = true; - overflowChunks = new ArrayList<>(); - overflowChunks.add(new byte[CHUNK_SIZE]); - overflowChunkOffset = 0; - overflowTotalBytes = 0; - } - - private void writeToOverflow(byte[] b, int off, int len) { - int remaining = len; - int srcOff = off; - while (remaining > 0) { - ensureOverflowCapacity(1); - int space = currentOverflowChunk().length - overflowChunkOffset; - int toCopy = Math.min(remaining, space); - System.arraycopy(b, srcOff, currentOverflowChunk(), overflowChunkOffset, toCopy); - overflowChunkOffset += toCopy; - overflowTotalBytes += toCopy; - srcOff += toCopy; - remaining -= toCopy; - } - } - - private byte[] currentOverflowChunk() { - return overflowChunks.get(overflowChunks.size() - 1); - } - - private void ensureOverflowCapacity(int needed) { - if (overflowChunkOffset + needed > currentOverflowChunk().length) { - overflowChunks.add(new byte[CHUNK_SIZE]); - overflowChunkOffset = 0; - } - } -} diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java index e178885774f2..3f139e6ae3ca 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java @@ -15,6 +15,7 @@ package software.amazon.awssdk.protocols.json; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.math.BigDecimal; import java.math.BigInteger; @@ -22,7 +23,6 @@ import java.time.Instant; import software.amazon.awssdk.annotations.SdkProtectedApi; import software.amazon.awssdk.core.exception.SdkClientException; -import software.amazon.awssdk.http.ContentStreamProvider; import software.amazon.awssdk.thirdparty.jackson.core.JsonFactory; import software.amazon.awssdk.thirdparty.jackson.core.JsonGenerator; import software.amazon.awssdk.utils.BinaryUtils; @@ -39,7 +39,7 @@ public class SdkJsonGenerator implements StructuredJsonGenerator { * prevent frequent resizings but small enough to avoid wasted allocations for small requests. */ private static final int DEFAULT_BUFFER_SIZE = 1024; - private final SdkByteArrayOutputStream baos = new SdkByteArrayOutputStream(DEFAULT_BUFFER_SIZE); + private final ByteArrayOutputStream baos = new ByteArrayOutputStream(DEFAULT_BUFFER_SIZE); private final JsonGenerator generator; private final String contentType; @@ -287,24 +287,6 @@ public byte[] getBytes() { return baos.toByteArray(); } - /** - * Returns the size of the generated content in bytes without copying. - */ - public int contentSize() { - close(); - return baos.contentSize(); - } - - /** - * Returns a {@link ContentStreamProvider} that streams directly from the internal buffers - * without creating a contiguous copy. For small payloads this wraps the single base buffer; - * for large payloads it chains the base buffer and overflow chunks. - */ - public ContentStreamProvider contentStreamProvider() { - close(); - return baos.contentStreamProvider(); - } - @Override public String getContentType() { return contentType; diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/StructuredJsonGenerator.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/StructuredJsonGenerator.java index 5345305ffcd5..8beb83797d8f 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/StructuredJsonGenerator.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/StructuredJsonGenerator.java @@ -15,13 +15,11 @@ package software.amazon.awssdk.protocols.json; -import java.io.ByteArrayInputStream; import java.math.BigDecimal; import java.math.BigInteger; import java.nio.ByteBuffer; import java.time.Instant; import software.amazon.awssdk.annotations.SdkProtectedApi; -import software.amazon.awssdk.http.ContentStreamProvider; /** * Interface for generating a JSON @@ -200,28 +198,4 @@ default StructuredJsonGenerator writeBinaryValue(byte[] bytes) { */ @Deprecated String getContentType(); - - /** - * Returns the size of the generated content in bytes without copying. The default - * implementation falls back to {@link #getBytes()}.length. - */ - default int contentSize() { - byte[] bytes = getBytes(); - return bytes == null ? 0 : bytes.length; - } - - /** - * Returns a {@link ContentStreamProvider} that streams the generated content. The default - * implementation wraps the result of {@link #getBytes()} in a {@code ByteArrayInputStream}. - * Implementations may override this to stream directly from internal buffers without copying. - * - * @return a content stream provider, or {@code null} if {@link #getBytes()} returns null - */ - default ContentStreamProvider contentStreamProvider() { - byte[] bytes = getBytes(); - if (bytes == null) { - return null; - } - return () -> new ByteArrayInputStream(bytes); - } } diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java index 8fc991e7ecca..59f11bb5bfeb 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java @@ -43,7 +43,6 @@ import software.amazon.awssdk.core.traits.RequiredTrait; import software.amazon.awssdk.core.traits.TimestampFormatTrait; import software.amazon.awssdk.core.traits.TraitType; -import software.amazon.awssdk.http.ContentStreamProvider; import software.amazon.awssdk.http.SdkHttpFullRequest; import software.amazon.awssdk.protocols.core.InstantToString; import software.amazon.awssdk.protocols.core.OperationInfo; @@ -291,12 +290,12 @@ private SdkHttpFullRequest finishMarshalling() { jsonGenerator.writeEndObject(); } - ContentStreamProvider contentProvider = jsonGenerator.contentStreamProvider(); - if (contentProvider != null) { - request.contentStreamProvider(contentProvider); - int contentSize = jsonGenerator.contentSize(); - if (contentSize > 0) { - request.putHeader(CONTENT_LENGTH, Integer.toString(contentSize)); + byte[] content = jsonGenerator.getBytes(); + + if (content != null) { + request.contentStreamProvider(() -> new ByteArrayInputStream(content)); + if (content.length > 0) { + request.putHeader(CONTENT_LENGTH, Integer.toString(content.length)); } } } diff --git a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStreamTest.java b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStreamTest.java deleted file mode 100644 index 9ac3120df1c4..000000000000 --- a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/SdkByteArrayOutputStreamTest.java +++ /dev/null @@ -1,354 +0,0 @@ -/* - * 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; - -import static org.assertj.core.api.Assertions.assertThat; -import static software.amazon.awssdk.protocols.json.SdkByteArrayOutputStream.CHUNK_SIZE; -import static software.amazon.awssdk.protocols.json.SdkByteArrayOutputStream.MAX_BUFFER_SIZE; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.util.Arrays; -import java.util.Random; -import org.junit.jupiter.api.Test; -import software.amazon.awssdk.http.ContentStreamProvider; - -/** - * Unit tests for {@link SdkByteArrayOutputStream}, covering both the small-payload (base buffer) - * and large-payload (overflow) paths. - */ -class SdkByteArrayOutputStreamTest { - - private static final Random RANDOM = new Random(42); - - @Test - void write_smallPayload_behavesLikeByteArrayOutputStream() { - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(64); - byte[] data = randomBytes(1000); - stream.write(data, 0, data.length); - - assertThat(stream.size()).isEqualTo(1000); - assertThat(stream.toByteArray()).isEqualTo(data); - } - - @Test - void write_singleBytes_smallPayload_behavesCorrectly() { - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(16); - for (int i = 0; i < 256; i++) { - stream.write(i); - } - - assertThat(stream.size()).isEqualTo(256); - byte[] result = stream.toByteArray(); - for (int i = 0; i < 256; i++) { - assertThat(result[i]).isEqualTo((byte) i); - } - } - - @Test - void size_emptyStream_returnsZero() { - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(64); - assertThat(stream.size()).isEqualTo(0); - assertThat(stream.toByteArray()).isEmpty(); - } - - @Test - void write_exactlyMaxBufferSize_doesNotOverflow() { - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); - byte[] data = randomBytes(MAX_BUFFER_SIZE); - stream.write(data, 0, data.length); - - assertThat(stream.size()).isEqualTo(MAX_BUFFER_SIZE); - assertThat(stream.toByteArray()).isEqualTo(data); - } - - @Test - void write_oneBytePastMaxBufferSize_triggersOverflow() { - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); - byte[] data = randomBytes(MAX_BUFFER_SIZE + 1); - stream.write(data, 0, data.length); - - assertThat(stream.size()).isEqualTo(MAX_BUFFER_SIZE + 1); - assertThat(stream.toByteArray()).isEqualTo(data); - } - - @Test - void write_singleByte_triggersOverflow() { - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); - byte[] base = randomBytes(MAX_BUFFER_SIZE); - stream.write(base, 0, base.length); - - // This single byte should trigger overflow - stream.write(0xFF); - - assertThat(stream.size()).isEqualTo(MAX_BUFFER_SIZE + 1); - byte[] result = stream.toByteArray(); - assertThat(Arrays.copyOf(result, MAX_BUFFER_SIZE)).isEqualTo(base); - assertThat(result[MAX_BUFFER_SIZE]).isEqualTo((byte) 0xFF); - } - - @Test - void write_largePayload_multipleChunks_producesCorrectOutput() { - // Write enough to span the base buffer + multiple overflow chunks - int totalSize = MAX_BUFFER_SIZE + (CHUNK_SIZE * 3) + 100; - byte[] data = randomBytes(totalSize); - - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); - stream.write(data, 0, data.length); - - assertThat(stream.size()).isEqualTo(totalSize); - assertThat(stream.toByteArray()).isEqualTo(data); - } - - @Test - void write_largePayload_incrementalWrites_producesCorrectOutput() { - // Write in small increments that cross chunk boundaries - int totalSize = MAX_BUFFER_SIZE + (CHUNK_SIZE * 2) + 500; - byte[] data = randomBytes(totalSize); - - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); - int offset = 0; - int chunkSize = 1337; // Deliberately not aligned to CHUNK_SIZE - while (offset < data.length) { - int len = Math.min(chunkSize, data.length - offset); - stream.write(data, offset, len); - offset += len; - } - - assertThat(stream.size()).isEqualTo(totalSize); - assertThat(stream.toByteArray()).isEqualTo(data); - } - - @Test - void write_singleBytes_intoOverflow_producesCorrectOutput() { - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); - byte[] base = randomBytes(MAX_BUFFER_SIZE); - stream.write(base, 0, base.length); - - // Write 200 single bytes into overflow - byte[] overflow = new byte[200]; - for (int i = 0; i < 200; i++) { - overflow[i] = (byte) (i & 0xFF); - stream.write(overflow[i]); - } - - assertThat(stream.size()).isEqualTo(MAX_BUFFER_SIZE + 200); - byte[] result = stream.toByteArray(); - assertThat(Arrays.copyOf(result, MAX_BUFFER_SIZE)).isEqualTo(base); - assertThat(Arrays.copyOfRange(result, MAX_BUFFER_SIZE, result.length)).isEqualTo(overflow); - } - - @Test - void contentSize_smallPayload_matchesSize() { - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(64); - byte[] data = randomBytes(500); - stream.write(data, 0, data.length); - - assertThat(stream.contentSize()).isEqualTo(500); - assertThat(stream.contentSize()).isEqualTo(stream.size()); - } - - @Test - void contentSize_largePayload_matchesSize() { - int totalSize = MAX_BUFFER_SIZE + CHUNK_SIZE + 100; - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); - stream.write(randomBytes(totalSize), 0, totalSize); - - assertThat(stream.contentSize()).isEqualTo(totalSize); - assertThat(stream.contentSize()).isEqualTo(stream.size()); - } - - @Test - void contentStreamProvider_smallPayload_producesSameBytesAsToByteArray() throws IOException { - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(64); - byte[] data = randomBytes(5000); - stream.write(data, 0, data.length); - - ContentStreamProvider provider = stream.contentStreamProvider(); - byte[] streamed = readAllBytes(provider.newStream()); - - assertThat(streamed).isEqualTo(data); - } - - @Test - void contentStreamProvider_largePayload_producesSameBytesAsToByteArray() throws IOException { - int totalSize = MAX_BUFFER_SIZE + (CHUNK_SIZE * 2) + 500; - byte[] data = randomBytes(totalSize); - - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); - stream.write(data, 0, data.length); - - byte[] expected = stream.toByteArray(); - ContentStreamProvider provider = stream.contentStreamProvider(); - byte[] streamed = readAllBytes(provider.newStream()); - - assertThat(streamed).isEqualTo(expected); - } - - @Test - void contentStreamProvider_isResettable_smallPayload() throws IOException { - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(64); - byte[] data = randomBytes(100); - stream.write(data, 0, data.length); - - ContentStreamProvider provider = stream.contentStreamProvider(); - byte[] first = readAllBytes(provider.newStream()); - byte[] second = readAllBytes(provider.newStream()); - - assertThat(first).isEqualTo(data); - assertThat(second).isEqualTo(data); - } - - @Test - void contentStreamProvider_isResettable_largePayload() throws IOException { - int totalSize = MAX_BUFFER_SIZE + CHUNK_SIZE + 100; - byte[] data = randomBytes(totalSize); - - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); - stream.write(data, 0, data.length); - - ContentStreamProvider provider = stream.contentStreamProvider(); - byte[] first = readAllBytes(provider.newStream()); - byte[] second = readAllBytes(provider.newStream()); - - assertThat(first).isEqualTo(data); - assertThat(second).isEqualTo(data); - } - - @Test - void contentStreamProvider_emptyStream_producesEmptyContent() throws IOException { - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(64); - ContentStreamProvider provider = stream.contentStreamProvider(); - byte[] content = readAllBytes(provider.newStream()); - - assertThat(content).isEmpty(); - } - - @Test - void reset_smallPayload_clearsAllData() { - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(64); - stream.write(new byte[]{1, 2, 3}, 0, 3); - - stream.reset(); - - assertThat(stream.size()).isEqualTo(0); - assertThat(stream.toByteArray()).isEmpty(); - } - - @Test - void reset_afterOverflow_clearsAllState() { - int totalSize = MAX_BUFFER_SIZE + CHUNK_SIZE + 100; - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); - stream.write(randomBytes(totalSize), 0, totalSize); - - assertThat(stream.size()).isEqualTo(totalSize); - - stream.reset(); - - assertThat(stream.size()).isEqualTo(0); - assertThat(stream.toByteArray()).isEmpty(); - assertThat(stream.contentSize()).isEqualTo(0); - } - - @Test - void reset_afterOverflow_allowsReuse() { - int totalSize = MAX_BUFFER_SIZE + CHUNK_SIZE + 100; - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); - stream.write(randomBytes(totalSize), 0, totalSize); - - stream.reset(); - - // Write new data after reset - byte[] newData = randomBytes(500); - stream.write(newData, 0, newData.length); - - assertThat(stream.size()).isEqualTo(500); - assertThat(stream.toByteArray()).isEqualTo(newData); - } - - @Test - void reset_afterOverflow_thenWriteLargeAgain_producesCorrectOutput() { - int totalSize = MAX_BUFFER_SIZE + 500; - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); - stream.write(randomBytes(totalSize), 0, totalSize); - - stream.reset(); - - // Write a different large payload - byte[] newData = randomBytes(MAX_BUFFER_SIZE + 1000); - stream.write(newData, 0, newData.length); - - assertThat(stream.size()).isEqualTo(newData.length); - assertThat(stream.toByteArray()).isEqualTo(newData); - } - - @Test - void writeTo_smallPayload_writesAllData() throws IOException { - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(64); - byte[] data = randomBytes(500); - stream.write(data, 0, data.length); - - ByteArrayOutputStream target = new ByteArrayOutputStream(); - stream.writeTo(target); - - assertThat(target.toByteArray()).isEqualTo(data); - } - - @Test - void writeTo_largePayload_writesAllData() throws IOException { - int totalSize = MAX_BUFFER_SIZE + (CHUNK_SIZE * 2) + 500; - byte[] data = randomBytes(totalSize); - - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); - stream.write(data, 0, data.length); - - ByteArrayOutputStream target = new ByteArrayOutputStream(); - stream.writeTo(target); - - assertThat(target.toByteArray()).isEqualTo(data); - } - - @Test - void writeTo_afterOverflow_matchesToByteArray() throws IOException { - int totalSize = MAX_BUFFER_SIZE + CHUNK_SIZE + 100; - byte[] data = randomBytes(totalSize); - - SdkByteArrayOutputStream stream = new SdkByteArrayOutputStream(1024); - stream.write(data, 0, data.length); - - ByteArrayOutputStream target = new ByteArrayOutputStream(); - stream.writeTo(target); - - assertThat(target.toByteArray()).isEqualTo(stream.toByteArray()); - } - - private static byte[] randomBytes(int length) { - byte[] data = new byte[length]; - RANDOM.nextBytes(data); - return data; - } - - private static byte[] readAllBytes(InputStream is) throws IOException { - ByteArrayOutputStream bos = new ByteArrayOutputStream(); - byte[] buf = new byte[4096]; - int n; - while ((n = is.read(buf)) != -1) { - bos.write(buf, 0, n); - } - return bos.toByteArray(); - } -} diff --git a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/SdkJsonGeneratorTest.java b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/SdkJsonGeneratorTest.java index 0ab737ca91f5..bba1caedfb0d 100644 --- a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/SdkJsonGeneratorTest.java +++ b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/SdkJsonGeneratorTest.java @@ -21,13 +21,11 @@ import java.io.ByteArrayInputStream; import java.io.IOException; -import java.io.InputStream; import java.nio.ByteBuffer; import java.nio.charset.Charset; import java.time.Instant; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import software.amazon.awssdk.http.ContentStreamProvider; import software.amazon.awssdk.protocols.jsoncore.JsonNode; import software.amazon.awssdk.thirdparty.jackson.core.JsonFactory; import software.amazon.awssdk.thirdparty.jackson.core.StreamReadFeature; @@ -180,131 +178,4 @@ private JsonNode toJsonNode() throws IOException { return JsonNode.parser().parse(new ByteArrayInputStream(jsonGenerator.getBytes())); } - @Test - public void contentSize_matchesGetBytesLength() { - SdkJsonGenerator gen = newSdkJsonGenerator(); - gen.writeStartObject(); - gen.writeFieldName("key").writeValue("value"); - gen.writeFieldName("num").writeValue(42); - gen.writeEndObject(); - - byte[] bytes = gen.getBytes(); - - SdkJsonGenerator gen2 = newSdkJsonGenerator(); - gen2.writeStartObject(); - gen2.writeFieldName("key").writeValue("value"); - gen2.writeFieldName("num").writeValue(42); - gen2.writeEndObject(); - - assertEquals(bytes.length, gen2.contentSize()); - } - - @Test - public void contentStreamProvider_producesSameBytesAsGetBytes() throws IOException { - SdkJsonGenerator gen = newSdkJsonGenerator(); - gen.writeStartObject(); - gen.writeFieldName("hello").writeValue("world"); - gen.writeFieldName("count").writeValue(123); - gen.writeEndObject(); - - byte[] expected = gen.getBytes(); - - SdkJsonGenerator gen2 = newSdkJsonGenerator(); - gen2.writeStartObject(); - gen2.writeFieldName("hello").writeValue("world"); - gen2.writeFieldName("count").writeValue(123); - gen2.writeEndObject(); - - ContentStreamProvider provider = gen2.contentStreamProvider(); - byte[] actual = readAllBytes(provider.newStream()); - - assertTrue(java.util.Arrays.equals(expected, actual), - "contentStreamProvider should produce identical bytes to getBytes"); - } - - @Test - public void contentStreamProvider_isResettable() throws IOException { - SdkJsonGenerator gen = newSdkJsonGenerator(); - gen.writeStartObject(); - gen.writeFieldName("data").writeValue("test"); - gen.writeEndObject(); - - ContentStreamProvider provider = gen.contentStreamProvider(); - byte[] first = readAllBytes(provider.newStream()); - byte[] second = readAllBytes(provider.newStream()); - - assertTrue(java.util.Arrays.equals(first, second), - "Multiple calls to newStream() should produce identical content"); - assertTrue(first.length > 0, "Content should not be empty"); - } - - @Test - public void emptyGenerator_contentSizeIsZero() throws IOException { - SdkJsonGenerator gen = newSdkJsonGenerator(); - assertEquals(0, gen.contentSize()); - - ContentStreamProvider provider = gen.contentStreamProvider(); - assertTrue(provider != null, "Provider should not be null even for empty content"); - byte[] content = readAllBytes(provider.newStream()); - assertEquals(0, content.length, "Empty generator should produce empty stream"); - } - - @Test - public void largePayload_contentStreamProviderStreamsCorrectData() throws IOException { - // Generate JSON exceeding 64 KB to verify contentStreamProvider works for large payloads - SdkJsonGenerator gen = newSdkJsonGenerator(); - gen.writeStartObject(); - gen.writeFieldName("items"); - gen.writeStartArray(); - for (int i = 0; i < 2000; i++) { - gen.writeStartObject(); - gen.writeFieldName("index").writeValue(i); - gen.writeFieldName("description").writeValue( - "This is a moderately long string value for item number " + i + - " that helps push the total payload size beyond the 64KB chunk boundary."); - gen.writeEndObject(); - } - gen.writeEndArray(); - gen.writeEndObject(); - - byte[] expected = gen.getBytes(); - assertTrue(expected.length > 64 * 1024, "Payload should exceed 64 KB"); - - SdkJsonGenerator gen2 = newSdkJsonGenerator(); - gen2.writeStartObject(); - gen2.writeFieldName("items"); - gen2.writeStartArray(); - for (int i = 0; i < 2000; i++) { - gen2.writeStartObject(); - gen2.writeFieldName("index").writeValue(i); - gen2.writeFieldName("description").writeValue( - "This is a moderately long string value for item number " + i + - " that helps push the total payload size beyond the 64KB chunk boundary."); - gen2.writeEndObject(); - } - gen2.writeEndArray(); - gen2.writeEndObject(); - - assertEquals(expected.length, gen2.contentSize()); - byte[] actual = readAllBytes(gen2.contentStreamProvider().newStream()); - assertTrue(java.util.Arrays.equals(expected, actual), - "Large payload should stream correctly via contentStreamProvider"); - } - - private SdkJsonGenerator newSdkJsonGenerator() { - return new SdkJsonGenerator(JsonFactory.builder() - .enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION) - .build(), "application/json"); - } - - private static byte[] readAllBytes(InputStream is) throws IOException { - java.io.ByteArrayOutputStream bos = new java.io.ByteArrayOutputStream(); - byte[] buf = new byte[1024]; - int n; - while ((n = is.read(buf)) != -1) { - bos.write(buf, 0, n); - } - return bos.toByteArray(); - } - } From 58a9d99aadb0ee5e7b1bfdfe9e4c8a7805d31568 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Thu, 30 Apr 2026 13:53:41 -0700 Subject: [PATCH 11/12] 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. --- .../PayloadMarshallingEquivalenceTest.java | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/PayloadMarshallingEquivalenceTest.java b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/PayloadMarshallingEquivalenceTest.java index 339fa9f1e8a3..51b8b49f0ed7 100644 --- a/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/PayloadMarshallingEquivalenceTest.java +++ b/core/protocols/aws-json-protocol/src/test/java/software/amazon/awssdk/protocols/json/internal/marshall/PayloadMarshallingEquivalenceTest.java @@ -22,7 +22,6 @@ import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; -import java.util.Base64; import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; @@ -206,22 +205,6 @@ void marshallPayloadField_withSdkBytesValue_producesBase64EncodedJson() { assertThat(body).contains("\"fieldName\":\"ZGF0YQ==\""); } - // ---- SDK_BYTES (large — exceeds SdkByteArrayOutputStream.MAX_BUFFER_SIZE) ---- - - @Test - void marshallPayloadField_withLargeSdkBytesValue_producesCorrectBase64() { - // 200 KB of random data — large enough to trigger SdkByteArrayOutputStream overflow - byte[] rawData = new byte[200 * 1024]; - new java.util.Random(12345).nextBytes(rawData); - SdkBytes sdkBytes = SdkBytes.fromByteArray(rawData); - String expectedBase64 = Base64.getEncoder().encodeToString(rawData); - - SdkField field = payloadField("binaryField", MarshallingType.SDK_BYTES, - obj -> sdkBytes); - String body = marshallAndGetBody(field); - assertThat(body).contains("\"binaryField\":\"" + expectedBase64 + "\""); - } - @Test void marshallPayloadField_withSdkPojoValue_producesNestedObjectJson() { // Inner pojo with a single string field From a3feb6ce63beb81f96be6a626c225d5096a29044 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Thu, 30 Apr 2026 14:07:04 -0700 Subject: [PATCH 12/12] 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. --- .../awssdk/protocols/json/SdkJsonGenerator.java | 10 ---------- .../protocols/json/StructuredJsonGenerator.java | 14 -------------- .../internal/marshall/JsonProtocolMarshaller.java | 2 +- 3 files changed, 1 insertion(+), 25 deletions(-) diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java index 3f139e6ae3ca..bfd819708b33 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/SdkJsonGenerator.java @@ -206,16 +206,6 @@ public StructuredJsonGenerator writeValue(ByteBuffer bytes) { return this; } - @Override - public StructuredJsonGenerator writeBinaryValue(byte[] bytes) { - try { - generator.writeBinary(bytes); - } catch (IOException e) { - throw new JsonGenerationException(e); - } - return this; - } - @Override //TODO: This date formatting is coupled to AWS's format. Should generalize it public StructuredJsonGenerator writeValue(Instant instant) { diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/StructuredJsonGenerator.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/StructuredJsonGenerator.java index 8beb83797d8f..8d02b2ea78f8 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/StructuredJsonGenerator.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/StructuredJsonGenerator.java @@ -102,11 +102,6 @@ public StructuredJsonGenerator writeValue(ByteBuffer bytes) { return this; } - @Override - public StructuredJsonGenerator writeBinaryValue(byte[] bytes) { - return this; - } - @Override public StructuredJsonGenerator writeValue(Instant instant) { return this; @@ -174,15 +169,6 @@ default StructuredJsonGenerator writeValue(byte val) { StructuredJsonGenerator writeValue(ByteBuffer bytes); - /** - * Writes binary data directly from a byte array, avoiding the overhead of wrapping in a - * {@link ByteBuffer}. The default implementation wraps the array and delegates to - * {@link #writeValue(ByteBuffer)}. - */ - default StructuredJsonGenerator writeBinaryValue(byte[] bytes) { - return writeValue(ByteBuffer.wrap(bytes)); - } - StructuredJsonGenerator writeValue(Instant instant); StructuredJsonGenerator writeNumber(String number); diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java index 59f11bb5bfeb..3efd7d72dc5b 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java @@ -389,7 +389,7 @@ private void marshallPayloadField(SdkField field, Object val) { break; case SDK_BYTES: gen.writeFieldName(fieldName); - gen.writeBinaryValue(((SdkBytes) val).asByteArrayUnsafe()); + gen.writeValue(((SdkBytes) val).asByteBuffer()); break; case SDK_POJO: SimpleTypeJsonMarshaller.SDK_POJO.marshall((SdkPojo) val, marshallerContext,