diff --git a/codegen/codegen-core/src/main/java/software/amazon/smithy/java/codegen/generators/StructureGenerator.java b/codegen/codegen-core/src/main/java/software/amazon/smithy/java/codegen/generators/StructureGenerator.java index 2aedf5243..484498b20 100644 --- a/codegen/codegen-core/src/main/java/software/amazon/smithy/java/codegen/generators/StructureGenerator.java +++ b/codegen/codegen-core/src/main/java/software/amazon/smithy/java/codegen/generators/StructureGenerator.java @@ -14,6 +14,7 @@ import java.util.Arrays; import java.util.Base64; import java.util.Collections; +import java.util.Comparator; import java.util.LinkedHashSet; import java.util.List; import java.util.Objects; @@ -401,7 +402,8 @@ private void writeMemberEquals(JavaWriter writer) { } private void writePropertyEqualityChecks(JavaWriter writer) { - var iter = shape.members().iterator(); + var iter = + shape.members().stream().sorted(new CheapEqualsFirstComparator(symbolProvider, model)).iterator(); while (iter.hasNext()) { var member = iter.next(); var memberSymbol = symbolProvider.toSymbol(member); @@ -428,6 +430,53 @@ private void writePropertyEqualityChecks(JavaWriter writer) { writer.popState(); } } + + private record CheapEqualsFirstComparator(SymbolProvider symbolProvider, Model model) + implements Comparator { + + // Equality cost ranking by shape type (lower = cheaper). + // Primitives using == are cheapest, followed by types using static compare methods, + // then Object.equals/Arrays.equals comparisons. + private static final List COST_RANKING = List.of( + ShapeType.BOOLEAN, + ShapeType.BYTE, + ShapeType.SHORT, + ShapeType.INTEGER, + ShapeType.INT_ENUM, + ShapeType.LONG, + ShapeType.FLOAT, + ShapeType.DOUBLE, + ShapeType.STRING, + ShapeType.ENUM, + ShapeType.TIMESTAMP, + ShapeType.BIG_INTEGER, + ShapeType.BIG_DECIMAL, + ShapeType.BLOB, + ShapeType.UNION, + ShapeType.STRUCTURE, + ShapeType.LIST, + ShapeType.MAP, + ShapeType.DOCUMENT); + + @Override + public int compare(MemberShape o1, MemberShape o2) { + return Integer.compare(costOf(o1), costOf(o2)); + } + + private int costOf(MemberShape member) { + var symbol = symbolProvider.toSymbol(member); + boolean isPrimitive = symbol.getProperty(SymbolProperties.IS_PRIMITIVE).orElse(false) + && !CodegenUtils.isNullableMember(model, member); + var targetType = model.expectShape(member.getTarget()).getType(); + int typeRank = COST_RANKING.indexOf(targetType); + if (typeRank < 0) { + typeRank = COST_RANKING.size(); + } + // Boxed members use Objects.equals, which is more expensive than any primitive op. + // Shift them after all primitive types in the ranking. + return isPrimitive ? typeRank : COST_RANKING.size() + typeRank; + } + } } private record HashCodeGenerator(JavaWriter writer, Shape shape, SymbolProvider symbolProvider, Model model) diff --git a/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/defaults/expected/DefaultStructure.java b/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/defaults/expected/DefaultStructure.java index da9237b5d..6219138b5 100644 --- a/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/defaults/expected/DefaultStructure.java +++ b/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/defaults/expected/DefaultStructure.java @@ -237,31 +237,31 @@ public boolean equals(Object other) { } DefaultStructure that = (DefaultStructure) other; return this.booleanMember == that.booleanMember - && Objects.equals(this.bigDecimal, that.bigDecimal) - && Objects.equals(this.bigDecimalWithDoubleDefault, that.bigDecimalWithDoubleDefault) - && Objects.equals(this.bigDecimalWithLongDefault, that.bigDecimalWithLongDefault) - && Objects.equals(this.bigInteger, that.bigInteger) - && Objects.equals(this.bigIntegerWithLongDefault, that.bigIntegerWithLongDefault) && this.byteMember == that.byteMember - && Double.compare(this.doubleMember, that.doubleMember) == 0 - && Float.compare(this.floatMember, that.floatMember) == 0 + && this.shortMember == that.shortMember && this.integer == that.integer && this.longMember == that.longMember - && this.shortMember == that.shortMember + && Float.compare(this.floatMember, that.floatMember) == 0 + && Double.compare(this.doubleMember, that.doubleMember) == 0 + && Objects.equals(this.intEnum, that.intEnum) && Objects.equals(this.string, that.string) + && Objects.equals(this.enumMember, that.enumMember) + && Objects.equals(this.timestamp, that.timestamp) + && Objects.equals(this.bigInteger, that.bigInteger) + && Objects.equals(this.bigIntegerWithLongDefault, that.bigIntegerWithLongDefault) + && Objects.equals(this.bigDecimal, that.bigDecimal) + && Objects.equals(this.bigDecimalWithDoubleDefault, that.bigDecimalWithDoubleDefault) + && Objects.equals(this.bigDecimalWithLongDefault, that.bigDecimalWithLongDefault) && Objects.equals(this.blob, that.blob) && Objects.equals(this.streamingBlob, that.streamingBlob) + && Objects.equals(this.list, that.list) + && Objects.equals(this.map, that.map) && Objects.equals(this.boolDoc, that.boolDoc) && Objects.equals(this.stringDoc, that.stringDoc) && Objects.equals(this.numberDoc, that.numberDoc) && Objects.equals(this.floatingPointnumberDoc, that.floatingPointnumberDoc) && Objects.equals(this.listDoc, that.listDoc) - && Objects.equals(this.mapDoc, that.mapDoc) - && Objects.equals(this.list, that.list) - && Objects.equals(this.map, that.map) - && Objects.equals(this.timestamp, that.timestamp) - && Objects.equals(this.enumMember, that.enumMember) - && Objects.equals(this.intEnum, that.intEnum); + && Objects.equals(this.mapDoc, that.mapDoc); } @Override diff --git a/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/jspecify/expected/CollectionStruct.java b/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/jspecify/expected/CollectionStruct.java index 34cc89ff9..a468c99f9 100644 --- a/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/jspecify/expected/CollectionStruct.java +++ b/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/jspecify/expected/CollectionStruct.java @@ -172,12 +172,12 @@ public boolean equals(Object other) { CollectionStruct that = (CollectionStruct) other; return Objects.equals(this.nonSparseList, that.nonSparseList) && Objects.equals(this.sparseList, that.sparseList) - && Objects.equals(this.nonSparseMap, that.nonSparseMap) - && Objects.equals(this.sparseMap, that.sparseMap) && Objects.equals(this.nonSparseListOfSparseMap, that.nonSparseListOfSparseMap) && Objects.equals(this.sparseListOfSparseMap, that.sparseListOfSparseMap) - && Objects.equals(this.sparseMapOfNonSparseList, that.sparseMapOfNonSparseList) - && Objects.equals(this.nonSparseListOfNonSparseMap, that.nonSparseListOfNonSparseMap); + && Objects.equals(this.nonSparseListOfNonSparseMap, that.nonSparseListOfNonSparseMap) + && Objects.equals(this.nonSparseMap, that.nonSparseMap) + && Objects.equals(this.sparseMap, that.sparseMap) + && Objects.equals(this.sparseMapOfNonSparseList, that.sparseMapOfNonSparseList); } @Override diff --git a/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/jspecify/expected/JSpecifyStruct.java b/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/jspecify/expected/JSpecifyStruct.java index ee72a0ad3..1e09071b6 100644 --- a/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/jspecify/expected/JSpecifyStruct.java +++ b/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/jspecify/expected/JSpecifyStruct.java @@ -80,9 +80,9 @@ public boolean equals(Object other) { return false; } JSpecifyStruct that = (JSpecifyStruct) other; - return Objects.equals(this.requiredString, that.requiredString) + return this.requiredPrimitive == that.requiredPrimitive + && Objects.equals(this.requiredString, that.requiredString) && Objects.equals(this.optionalString, that.optionalString) - && this.requiredPrimitive == that.requiredPrimitive && Objects.equals(this.sparseList, that.sparseList); } diff --git a/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/naming-conflict/expected/NamingStruct.java b/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/naming-conflict/expected/NamingStruct.java index d0a19d036..a91da180a 100644 --- a/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/naming-conflict/expected/NamingStruct.java +++ b/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/naming-conflict/expected/NamingStruct.java @@ -115,10 +115,10 @@ public boolean equals(Object other) { } NamingStruct that = (NamingStruct) other; return Objects.equals(this.other, that.other) + && Objects.equals(this.union, that.union) && Objects.equals(this.builderMember, that.builderMember) && Objects.equals(this.type, that.type) && Objects.equals(this.objectMember, that.objectMember) - && Objects.equals(this.union, that.union) && Objects.equals(this.map, that.map) && Objects.equals(this.list, that.list) && Objects.equals(this.listOfList, that.listOfList) diff --git a/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/primitive-types/expected/PrimitivesNotNullable.java b/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/primitive-types/expected/PrimitivesNotNullable.java index 11ce2261b..9716341c2 100644 --- a/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/primitive-types/expected/PrimitivesNotNullable.java +++ b/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/primitive-types/expected/PrimitivesNotNullable.java @@ -88,13 +88,13 @@ public boolean equals(Object other) { return false; } PrimitivesNotNullable that = (PrimitivesNotNullable) other; - return this.byteMember == that.byteMember + return this.booleanMember == that.booleanMember + && this.byteMember == that.byteMember && this.shortMember == that.shortMember && this.intMember == that.intMember && this.longMember == that.longMember && Float.compare(this.floatMember, that.floatMember) == 0 - && Double.compare(this.doubleMember, that.doubleMember) == 0 - && this.booleanMember == that.booleanMember; + && Double.compare(this.doubleMember, that.doubleMember) == 0; } @Override diff --git a/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/primitive-types/expected/PrimitivesNullable.java b/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/primitive-types/expected/PrimitivesNullable.java index 091c8d353..1cb2b9df3 100644 --- a/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/primitive-types/expected/PrimitivesNullable.java +++ b/codegen/codegen-plugin/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/primitive-types/expected/PrimitivesNullable.java @@ -88,13 +88,13 @@ public boolean equals(Object other) { return false; } PrimitivesNullable that = (PrimitivesNullable) other; - return Objects.equals(this.byteMember, that.byteMember) + return Objects.equals(this.booleanMember, that.booleanMember) + && Objects.equals(this.byteMember, that.byteMember) && Objects.equals(this.shortMember, that.shortMember) && Objects.equals(this.intMember, that.intMember) && Objects.equals(this.longMember, that.longMember) && Objects.equals(this.floatMember, that.floatMember) - && Objects.equals(this.doubleMember, that.doubleMember) - && Objects.equals(this.booleanMember, that.booleanMember); + && Objects.equals(this.doubleMember, that.doubleMember); } @Override