Skip to content

Consolidate unmarshalling and parsing in a single pass #5643

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sugmanue merged 17 commits into from
Nov 4, 2024
Merged

Consolidate unmarshalling and parsing in a single pass #5643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
99923f8
Consolidate unmarshalling and parsing in a single pass
sugmanue Oct 1, 2024
24a07e7
Merge branch 'master' into sugmanue/unmarshalling-single-pass
sugmanue Oct 7, 2024
b33fa34
Merge branch 'master' into sugmanue/unmarshalling-single-pass
sugmanue Oct 9, 2024
1b93fce
Merge branch 'master' into sugmanue/unmarshalling-single-pass
sugmanue Oct 10, 2024
0bbb748
Address PR comments
sugmanue Oct 10, 2024
a412be7
Address PR comments
sugmanue Oct 10, 2024
6cfadea
Add a test to assert that a non-object literal throws
sugmanue Oct 10, 2024
8d49614
Add more testing
sugmanue Oct 11, 2024
ab45d70
Merge branch 'master' into sugmanue/unmarshalling-single-pass
sugmanue Oct 11, 2024
67f531a
Use Jackson's built-in fast float parsing
sugmanue Oct 14, 2024
99f0009
Add customizaion option to enable the fast unmarshalling path
sugmanue Oct 16, 2024
384ff36
Fix checkstyle issues
sugmanue Oct 19, 2024
138b13a
Merge branch 'master' into sugmanue/unmarshalling-single-pass
sugmanue Oct 19, 2024
0db0e56
Account for tests with unset clientConfiguration
sugmanue Oct 19, 2024
e3280fb
Fix codegen checkstyle
sugmanue Oct 19, 2024
ff1d00a
Merge branch 'master' into sugmanue/unmarshalling-single-pass
sugmanue Oct 28, 2024
3dd9420
Merge branch 'master' into sugmanue/unmarshalling-single-pass
sugmanue Nov 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/next-release/feature-AWSSDKforJavav2-70d110c.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"category": "AWS SDK for Java v2",
"contributor": "",
"type": "feature",
"description": "Improve unmarshalling performance of all JSON protocols by unifying parsing with unmarshalling instead of doing one after the other."
}
12 changes: 12 additions & 0 deletions ...n/java/software/amazon/awssdk/codegen/model/config/customization/CustomizationConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,11 @@ public class CustomizationConfig {
*/
private boolean batchManagerSupported;

/**
* A boolean flag to indicate if the fast unmarshaller code path is enabled.
*/
private boolean enableFastUnmarshaller;

private CustomizationConfig() {
}

Expand Down Expand Up @@ -914,4 +919,11 @@ public void setBatchManagerSupported(boolean batchManagerSupported) {
this.batchManagerSupported = batchManagerSupported;
}

public boolean getEnableFastUnmarshaller() {
return enableFastUnmarshaller;
}

public void setEnableFastUnmarshaller(boolean enableFastUnmarshaller) {
this.enableFastUnmarshaller = enableFastUnmarshaller;
}
}
8 changes: 8 additions & 0 deletions ...gen/src/main/java/software/amazon/awssdk/codegen/poet/builder/BaseClientBuilderClass.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
import software.amazon.awssdk.identity.spi.IdentityProvider;
import software.amazon.awssdk.identity.spi.IdentityProviders;
import software.amazon.awssdk.identity.spi.TokenIdentity;
import software.amazon.awssdk.protocols.json.internal.unmarshall.SdkClientJsonProtocolAdvancedOption;
import software.amazon.awssdk.regions.ServiceMetadataAdvancedOption;
import software.amazon.awssdk.utils.AttributeMap;
import software.amazon.awssdk.utils.CollectionUtils;
Expand Down Expand Up @@ -479,6 +480,13 @@ private MethodSpec finalizeServiceConfigurationMethod() {
.addCode(" .fipsEnabled(c.get($T.FIPS_ENDPOINT_ENABLED))", AwsClientOption.class)
.addCode(" .build());");

if (model.getMetadata().isJsonProtocol()) {
if (model.getCustomizationConfig().getEnableFastUnmarshaller()) {
builder.addStatement("builder.option($1T.ENABLE_FAST_UNMARSHALLER, true)",
Comment thread
dagnir marked this conversation as resolved.
Resolved
SdkClientJsonProtocolAdvancedOption.class);
}
}

builder.addStatement("return builder.build()");
return builder.build();
}
Expand Down
15 changes: 13 additions & 2 deletions codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/AwsServiceModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
Expand Down Expand Up @@ -98,7 +99,6 @@ public TypeSpec poetSpec() {
if (shapeModel.isEventStream()) {
return eventStreamInterfaceSpec();
}

List<FieldSpec> fields = shapeModelSpec.fields();

TypeSpec.Builder specBuilder = TypeSpec.classBuilder(className())
Expand All @@ -110,9 +110,9 @@ public TypeSpec poetSpec() {
.addFields(fields)
.addFields(shapeModelSpec.staticFields())
.addMethod(addModifier(sdkFieldsMethod(), FINAL))
.addMethod(addModifier(sdkFieldNameToFieldMethod(), FINAL))
.addTypes(nestedModelClassTypes());


if (shapeModel.isUnion()) {
specBuilder.addField(unionTypeField());
}
Expand Down Expand Up @@ -316,6 +316,17 @@ private MethodSpec sdkFieldsMethod() {
.build();
}

private MethodSpec sdkFieldNameToFieldMethod() {
ParameterizedTypeName sdkFieldType = ParameterizedTypeName.get(ClassName.get(SdkField.class),
WildcardTypeName.subtypeOf(ClassName.get(Object.class)));
return MethodSpec.methodBuilder("sdkFieldNameToField")
.addModifiers(PUBLIC)
.addAnnotation(Override.class)
.returns(ParameterizedTypeName.get(ClassName.get(Map.class), ClassName.get(String.class), sdkFieldType))
.addCode("return SDK_NAME_TO_FIELD;")
.build();
}

private MethodSpec getterCreator() {
TypeVariableName t = TypeVariableName.get("T");
return MethodSpec.methodBuilder("getter")
Expand Down
13 changes: 13 additions & 0 deletions codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ModelBuilderSpecs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.ArrayList;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Consumer;
import javax.lang.model.element.Modifier;
Expand Down Expand Up @@ -141,6 +142,7 @@ public TypeSpec beanStyleBuilder() {
builderClassBuilder.addMethods(accessors());
builderClassBuilder.addMethod(buildMethod());
builderClassBuilder.addMethod(sdkFieldsMethod());
builderClassBuilder.addMethod(sdkFieldNameToFieldMethod());

if (shapeModel.isUnion()) {
builderClassBuilder.addMethod(handleUnionValueChangeMethod());
Expand Down Expand Up @@ -169,6 +171,17 @@ private MethodSpec sdkFieldsMethod() {
.build();
}

private MethodSpec sdkFieldNameToFieldMethod() {
ParameterizedTypeName sdkFieldType = ParameterizedTypeName.get(ClassName.get(SdkField.class),
WildcardTypeName.subtypeOf(ClassName.get(Object.class)));
return MethodSpec.methodBuilder("sdkFieldNameToField")
.addModifiers(PUBLIC)
.addAnnotation(Override.class)
.returns(ParameterizedTypeName.get(ClassName.get(Map.class), ClassName.get(String.class), sdkFieldType))
.addCode("return SDK_NAME_TO_FIELD;")
.build();
}

private TypeName builderImplSuperClass() {
if (isRequest()) {
return new AwsServiceBaseRequestSpec(intermediateModel).className().nestedClass("BuilderImpl");
Expand Down
49 changes: 43 additions & 6 deletions codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ShapeModelSpec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@
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.Optional;
import java.util.stream.Collectors;
import javax.lang.model.element.Modifier;
Expand Down Expand Up @@ -95,6 +98,7 @@ public List<FieldSpec> fields(Modifier... modifiers) {

public Iterable<FieldSpec> staticFields(Modifier... modifiers) {
List<FieldSpec> fields = new ArrayList<>();
Map<String, String> nameToField = new LinkedHashMap<>();
shapeModel.getNonStreamingMembers().stream()
// Exceptions can be members of event stream shapes, need to filter those out of the models
.filter(m -> m.getShape() == null || m.getShape().getShapeType() != ShapeType.Exception)
Expand All @@ -107,6 +111,8 @@ public Iterable<FieldSpec> staticFields(Modifier... modifiers) {
Modifier.PRIVATE, Modifier.STATIC, Modifier.FINAL)
.initializer(sdkFieldInitializer(m))
.build());
String name = m.getHttp().getMarshallLocationName();
nameToField.put(name, namingStrategy.getSdkFieldFieldName(m));
});

ParameterizedTypeName sdkFieldType = ParameterizedTypeName.get(ClassName.get(SdkField.class),
Expand All @@ -115,13 +121,15 @@ public Iterable<FieldSpec> staticFields(Modifier... modifiers) {
fields.add(FieldSpec.builder(ParameterizedTypeName.get(ClassName.get(List.class),
sdkFieldType), "SDK_FIELDS",
Modifier.PRIVATE, Modifier.STATIC, Modifier.FINAL)
.initializer("$T.unmodifiableList($T.asList($L))",
ClassName.get(Collections.class),
ClassName.get(Arrays.class),
fields.stream()
.map(f -> f.name)
.collect(Collectors.joining(",")))
.initializer(sdkFieldsInitializer(fields))
.build());
fields.add(FieldSpec.builder(ParameterizedTypeName.get(ClassName.get(Map.class),
ClassName.get(String.class),
sdkFieldType),
"SDK_NAME_TO_FIELD",
Modifier.PRIVATE, Modifier.STATIC, Modifier.FINAL)
.initializer(memberNameToFieldInitializer(nameToField))
.build());
return fields;
}

Expand Down Expand Up @@ -369,4 +377,33 @@ private CodeBlock constructor(MemberModel m) {
}
}

private CodeBlock sdkFieldsInitializer(List<FieldSpec> fields) {
CodeBlock.Builder builder = CodeBlock.builder();
if (fields.isEmpty()) {
builder.add("$T.emptyList()", Collections.class);
return builder.build();
}
builder.add("$T.unmodifiableList($T.asList($L))",
ClassName.get(Collections.class),
ClassName.get(Arrays.class),
fields.stream()
.map(f -> f.name)
.collect(Collectors.joining(",")));
return builder.build();
}

private CodeBlock memberNameToFieldInitializer(Map<String, String> nameToField) {
CodeBlock.Builder builder = CodeBlock.builder();
if (nameToField.isEmpty()) {
builder.add("$T.emptyMap()", Collections.class);
return builder.build();
}
builder.add("$T.unmodifiableMap(", Collections.class);
builder.add("new $T<$T, $T<?>>() {{\n", HashMap.class, String.class, SdkField.class);
nameToField.forEach((name, field) -> builder.add("put($S, $L);\n", name, field));
builder.add("}}");
builder.add(")");
return builder.build();
}

}
53 changes: 53 additions & 0 deletions codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/alltypesrequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -450,6 +451,48 @@ SdkField.<SdkBytes> builder(MarshallingType.SDK_BYTES)
POLYMORPHIC_TYPE_WITHOUT_SUB_TYPES_FIELD, ENUM_TYPE_FIELD, UNDERSCORE_NAME_TYPE_FIELD, MY_DOCUMENT_FIELD,
ALL_TYPES_UNION_STRUCTURE_FIELD));

private static final Map<String, SdkField<?>> SDK_NAME_TO_FIELD = Collections
.unmodifiableMap(new HashMap<String, SdkField<?>>() {
{
put("StringMember", STRING_MEMBER_FIELD);
put("IntegerMember", INTEGER_MEMBER_FIELD);
put("BooleanMember", BOOLEAN_MEMBER_FIELD);
put("FloatMember", FLOAT_MEMBER_FIELD);
put("DoubleMember", DOUBLE_MEMBER_FIELD);
put("LongMember", LONG_MEMBER_FIELD);
put("ShortMember", SHORT_MEMBER_FIELD);
put("ByteMember", BYTE_MEMBER_FIELD);
put("SimpleList", SIMPLE_LIST_FIELD);
put("ListOfEnums", LIST_OF_ENUMS_FIELD);
put("ListOfMaps", LIST_OF_MAPS_FIELD);
put("ListOfStructs", LIST_OF_STRUCTS_FIELD);
put("ListOfMapOfEnumToString", LIST_OF_MAP_OF_ENUM_TO_STRING_FIELD);
put("ListOfMapOfStringToStruct", LIST_OF_MAP_OF_STRING_TO_STRUCT_FIELD);
put("MapOfStringToIntegerList", MAP_OF_STRING_TO_INTEGER_LIST_FIELD);
put("MapOfStringToString", MAP_OF_STRING_TO_STRING_FIELD);
put("MapOfStringToSimpleStruct", MAP_OF_STRING_TO_SIMPLE_STRUCT_FIELD);
put("MapOfEnumToEnum", MAP_OF_ENUM_TO_ENUM_FIELD);
put("MapOfEnumToString", MAP_OF_ENUM_TO_STRING_FIELD);
put("MapOfStringToEnum", MAP_OF_STRING_TO_ENUM_FIELD);
put("MapOfEnumToSimpleStruct", MAP_OF_ENUM_TO_SIMPLE_STRUCT_FIELD);
put("MapOfEnumToListOfEnums", MAP_OF_ENUM_TO_LIST_OF_ENUMS_FIELD);
put("MapOfEnumToMapOfStringToEnum", MAP_OF_ENUM_TO_MAP_OF_STRING_TO_ENUM_FIELD);
put("TimestampMember", TIMESTAMP_MEMBER_FIELD);
put("StructWithNestedTimestampMember", STRUCT_WITH_NESTED_TIMESTAMP_MEMBER_FIELD);
put("BlobArg", BLOB_ARG_FIELD);
put("StructWithNestedBlob", STRUCT_WITH_NESTED_BLOB_FIELD);
put("BlobMap", BLOB_MAP_FIELD);
put("ListOfBlobs", LIST_OF_BLOBS_FIELD);
put("RecursiveStruct", RECURSIVE_STRUCT_FIELD);
put("PolymorphicTypeWithSubTypes", POLYMORPHIC_TYPE_WITH_SUB_TYPES_FIELD);
put("PolymorphicTypeWithoutSubTypes", POLYMORPHIC_TYPE_WITHOUT_SUB_TYPES_FIELD);
put("EnumType", ENUM_TYPE_FIELD);
put("Underscore_Name_Type", UNDERSCORE_NAME_TYPE_FIELD);
put("MyDocument", MY_DOCUMENT_FIELD);
put("AllTypesUnionStructure", ALL_TYPES_UNION_STRUCTURE_FIELD);
}
});

private final String stringMember;

private final Integer integerMember;
Expand Down Expand Up @@ -1604,6 +1647,11 @@ public final List<SdkField<?>> sdkFields() {
return SDK_FIELDS;
}

@Override
public final Map<String, SdkField<?>> sdkFieldNameToField() {
return SDK_NAME_TO_FIELD;
}

private static <T> Function<Object, T> getter(Function<AllTypesRequest, T> g) {
return obj -> g.apply((AllTypesRequest) obj);
}
Expand Down Expand Up @@ -3065,5 +3113,10 @@ public AllTypesRequest build() {
public List<SdkField<?>> sdkFields() {
return SDK_FIELDS;
}

@Override
public Map<String, SdkField<?>> sdkFieldNameToField() {

@dagnir dagnir Oct 9, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it need to return a collection? Doesn't seem like we use any of the collection functionality anywhere. Can we just have a getByName type method, that returns the SdkField (or null)?

@sugmanue sugmanue Oct 9, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this method has to return a map as defined in the SdkPojo interface. We use it while parsing when we find a name to figure out to which field that name maps. Both the data class and the builder implement SdkPojo.

return SDK_NAME_TO_FIELD;
}
}
}
53 changes: 53 additions & 0 deletions codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/alltypesresponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -449,6 +450,48 @@ SdkField.<SdkBytes> builder(MarshallingType.SDK_BYTES)
POLYMORPHIC_TYPE_WITHOUT_SUB_TYPES_FIELD, ENUM_TYPE_FIELD, UNDERSCORE_NAME_TYPE_FIELD, MY_DOCUMENT_FIELD,
ALL_TYPES_UNION_STRUCTURE_FIELD));

private static final Map<String, SdkField<?>> SDK_NAME_TO_FIELD = Collections
.unmodifiableMap(new HashMap<String, SdkField<?>>() {
{
put("StringMember", STRING_MEMBER_FIELD);
put("IntegerMember", INTEGER_MEMBER_FIELD);
put("BooleanMember", BOOLEAN_MEMBER_FIELD);
put("FloatMember", FLOAT_MEMBER_FIELD);
put("DoubleMember", DOUBLE_MEMBER_FIELD);
put("LongMember", LONG_MEMBER_FIELD);
put("ShortMember", SHORT_MEMBER_FIELD);
put("ByteMember", BYTE_MEMBER_FIELD);
put("SimpleList", SIMPLE_LIST_FIELD);
put("ListOfEnums", LIST_OF_ENUMS_FIELD);
put("ListOfMaps", LIST_OF_MAPS_FIELD);
put("ListOfStructs", LIST_OF_STRUCTS_FIELD);
put("ListOfMapOfEnumToString", LIST_OF_MAP_OF_ENUM_TO_STRING_FIELD);
put("ListOfMapOfStringToStruct", LIST_OF_MAP_OF_STRING_TO_STRUCT_FIELD);
put("MapOfStringToIntegerList", MAP_OF_STRING_TO_INTEGER_LIST_FIELD);
put("MapOfStringToString", MAP_OF_STRING_TO_STRING_FIELD);
put("MapOfStringToSimpleStruct", MAP_OF_STRING_TO_SIMPLE_STRUCT_FIELD);
put("MapOfEnumToEnum", MAP_OF_ENUM_TO_ENUM_FIELD);
put("MapOfEnumToString", MAP_OF_ENUM_TO_STRING_FIELD);
put("MapOfStringToEnum", MAP_OF_STRING_TO_ENUM_FIELD);
put("MapOfEnumToSimpleStruct", MAP_OF_ENUM_TO_SIMPLE_STRUCT_FIELD);
put("MapOfEnumToListOfEnums", MAP_OF_ENUM_TO_LIST_OF_ENUMS_FIELD);
put("MapOfEnumToMapOfStringToEnum", MAP_OF_ENUM_TO_MAP_OF_STRING_TO_ENUM_FIELD);
put("TimestampMember", TIMESTAMP_MEMBER_FIELD);
put("StructWithNestedTimestampMember", STRUCT_WITH_NESTED_TIMESTAMP_MEMBER_FIELD);
put("BlobArg", BLOB_ARG_FIELD);
put("StructWithNestedBlob", STRUCT_WITH_NESTED_BLOB_FIELD);
put("BlobMap", BLOB_MAP_FIELD);
put("ListOfBlobs", LIST_OF_BLOBS_FIELD);
put("RecursiveStruct", RECURSIVE_STRUCT_FIELD);
put("PolymorphicTypeWithSubTypes", POLYMORPHIC_TYPE_WITH_SUB_TYPES_FIELD);
put("PolymorphicTypeWithoutSubTypes", POLYMORPHIC_TYPE_WITHOUT_SUB_TYPES_FIELD);
put("EnumType", ENUM_TYPE_FIELD);
put("Underscore_Name_Type", UNDERSCORE_NAME_TYPE_FIELD);
put("MyDocument", MY_DOCUMENT_FIELD);
put("AllTypesUnionStructure", ALL_TYPES_UNION_STRUCTURE_FIELD);
}
});

private final String stringMember;

private final Integer integerMember;
Expand Down Expand Up @@ -1603,6 +1646,11 @@ public final List<SdkField<?>> sdkFields() {
return SDK_FIELDS;
}

@Override
public final Map<String, SdkField<?>> sdkFieldNameToField() {
return SDK_NAME_TO_FIELD;
}

private static <T> Function<Object, T> getter(Function<AllTypesResponse, T> g) {
return obj -> g.apply((AllTypesResponse) obj);
}
Expand Down Expand Up @@ -3046,5 +3094,10 @@ public AllTypesResponse build() {
public List<SdkField<?>> sdkFields() {
return SDK_FIELDS;
}

@Override
public Map<String, SdkField<?>> sdkFieldNameToField() {
return SDK_NAME_TO_FIELD;
}
}
}
Loading