Skip to content

Improve performance of generated enum methods #2693

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
merged 3 commits into from
Sep 8, 2021
Merged

Improve performance of generated enum methods #2693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4bd79fb
Improve performance of generated enum methods
Sep 2, 2021
1fc12c3
Improve performance of generated enum methods
Sep 3, 2021
ce50536
Merge branch 'master' into enum-value-map
Bennett-Lynch Sep 8, 2021
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-0b3656e.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 performance of generated enum methods"
}
36 changes: 23 additions & 13 deletions codegen/src/main/java/software/amazon/awssdk/codegen/poet/common/AbstractEnumClass.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,30 @@

package software.amazon.awssdk.codegen.poet.common;

import static javax.lang.model.element.Modifier.FINAL;
import static javax.lang.model.element.Modifier.PRIVATE;
import static javax.lang.model.element.Modifier.STATIC;
import static software.amazon.awssdk.codegen.poet.PoetUtils.createEnumBuilder;
import static software.amazon.awssdk.codegen.poet.PoetUtils.toStringBuilder;

import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.FieldSpec;
import com.squareup.javapoet.MethodSpec;
import com.squareup.javapoet.ParameterizedTypeName;
import com.squareup.javapoet.TypeSpec;
import com.squareup.javapoet.TypeSpec.Builder;
import java.util.EnumSet;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.lang.model.element.Modifier;
import software.amazon.awssdk.codegen.model.intermediate.ShapeModel;
import software.amazon.awssdk.codegen.poet.ClassSpec;
import software.amazon.awssdk.utils.internal.EnumUtils;

public abstract class AbstractEnumClass implements ClassSpec {

private static final String VALUE = "value";
private static final String VALUE_MAP = "VALUE_MAP";
private static final String UNKNOWN_TO_SDK_VERSION = "UNKNOWN_TO_SDK_VERSION";
private final ShapeModel shape;

Expand All @@ -43,6 +49,7 @@ public AbstractEnumClass(ShapeModel shape) {
@Override
public final TypeSpec poetSpec() {
Builder enumBuilder = createEnumBuilder(className())
.addField(valueMapField())
.addField(String.class, VALUE, Modifier.PRIVATE, Modifier.FINAL)
.addMethod(toStringBuilder().addStatement("return $T.valueOf($N)", String.class, VALUE).build())
.addMethod(fromValueSpec())
Expand All @@ -67,6 +74,16 @@ protected final ShapeModel getShape() {
protected abstract void addJavadoc(Builder enumBuilder);

protected abstract void addEnumConstants(Builder enumBuilder);

private FieldSpec valueMapField() {
ParameterizedTypeName mapType = ParameterizedTypeName.get(ClassName.get(Map.class),
ClassName.get(String.class),
className());
return FieldSpec.builder(mapType, VALUE_MAP)
.addModifiers(PRIVATE, STATIC, FINAL)
.initializer("$1T.uniqueIndex($2T.class, $2T::toString)", EnumUtils.class, className())
.build();
}

private MethodSpec createConstructor() {
return MethodSpec.constructorBuilder()
Expand All @@ -88,13 +105,7 @@ private MethodSpec fromValueSpec() {
.beginControlFlow("if ($N == null)", VALUE)
.addStatement("return null")
.endControlFlow()
.addStatement("return $1T.of($2T.values())\n" +
".filter(e -> e.toString().equals($3N))\n" +
".findFirst()\n" +
".orElse(UNKNOWN_TO_SDK_VERSION)",
Stream.class,
className(),
VALUE)
.addStatement("return $N.getOrDefault($N, $N)", VALUE_MAP, VALUE, UNKNOWN_TO_SDK_VERSION)
.build();
}

Expand All @@ -106,10 +117,9 @@ private MethodSpec knownValuesSpec() {
+ "SDK.\n"
+ "This will return all known enum values except {@link #$N}.\n\n"
+ "@return a {@link $T} of known {@link $T}s", UNKNOWN_TO_SDK_VERSION, Set.class, className())
.addStatement("return $T.of(values()).filter(v -> v != $N).collect($T.toSet())",
Stream.class,
UNKNOWN_TO_SDK_VERSION,
Collectors.class)
.addStatement("$1T<$2T> knownValues = $3T.allOf($2T.class)", Set.class, className(), EnumSet.class)
.addStatement("knownValues.remove($N)", UNKNOWN_TO_SDK_VERSION)
.addStatement("return knownValues")
.build();
}
}
14 changes: 9 additions & 5 deletions codegen/src/test/resources/software/amazon/awssdk/codegen/poet/common/test-enum-class.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package software.amazon.awssdk.codegen.poet.common.model;

import java.util.EnumSet;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import software.amazon.awssdk.annotations.Generated;
import software.amazon.awssdk.utils.internal.EnumUtils;

/**
* Some comment on the class itself
Expand All @@ -16,6 +17,8 @@ public enum TestEnumClass {

UNKNOWN_TO_SDK_VERSION(null);

private static final Map<String, TestEnumClass> VALUE_MAP = EnumUtils.uniqueIndex(TestEnumClass.class, TestEnumClass::toString);

private final String value;

private TestEnumClass(String value) {
Expand All @@ -38,8 +41,7 @@ public static TestEnumClass fromValue(String value) {
if (value == null) {
return null;
}
return Stream.of(TestEnumClass.values()).filter(e -> e.toString().equals(value)).findFirst()
.orElse(UNKNOWN_TO_SDK_VERSION);
return VALUE_MAP.getOrDefault(value, UNKNOWN_TO_SDK_VERSION);
}

/**
Expand All @@ -49,7 +51,9 @@ public static TestEnumClass fromValue(String value) {
* @return a {@link Set} of known {@link TestEnumClass}s
*/
public static Set<TestEnumClass> knownValues() {
return Stream.of(values()).filter(v -> v != UNKNOWN_TO_SDK_VERSION).collect(Collectors.toSet());
Set<TestEnumClass> knownValues = EnumSet.allOf(TestEnumClass.class);
knownValues.remove(UNKNOWN_TO_SDK_VERSION);
return knownValues;
}
}

14 changes: 9 additions & 5 deletions codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/eventstream.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package software.amazon.awssdk.services.jsonprotocoltests.model;

import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import software.amazon.awssdk.annotations.Generated;
import software.amazon.awssdk.annotations.SdkPublicApi;
import software.amazon.awssdk.core.SdkField;
Expand All @@ -14,6 +14,7 @@
import software.amazon.awssdk.services.jsonprotocoltests.model.eventstream.DefaultEventthree;
import software.amazon.awssdk.services.jsonprotocoltests.model.eventstream.DefaultSecondEventOne;
import software.amazon.awssdk.services.jsonprotocoltests.model.eventstream.DefaultSecondEventTwo;
import software.amazon.awssdk.utils.internal.EnumUtils;

/**
* Base interface for all event types in EventStream.
Expand Down Expand Up @@ -103,6 +104,8 @@ enum EventType {

UNKNOWN_TO_SDK_VERSION(null);

private static final Map<String, EventType> VALUE_MAP = EnumUtils.uniqueIndex(EventType.class, EventType::toString);

private final String value;

private EventType(String value) {
Expand All @@ -125,8 +128,7 @@ public static EventType fromValue(String value) {
if (value == null) {
return null;
}
return Stream.of(EventType.values()).filter(e -> e.toString().equals(value)).findFirst()
.orElse(UNKNOWN_TO_SDK_VERSION);
return VALUE_MAP.getOrDefault(value, UNKNOWN_TO_SDK_VERSION);
}

/**
Expand All @@ -136,7 +138,9 @@ public static EventType fromValue(String value) {
* @return a {@link Set} of known {@link EventType}s
*/
public static Set<EventType> knownValues() {
return Stream.of(values()).filter(v -> v != UNKNOWN_TO_SDK_VERSION).collect(Collectors.toSet());
Set<EventType> knownValues = EnumSet.allOf(EventType.class);
knownValues.remove(UNKNOWN_TO_SDK_VERSION);
return knownValues;
}
}
}
Expand Down
14 changes: 9 additions & 5 deletions codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/inputeventstream.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package software.amazon.awssdk.services.jsonprotocoltests.model;

import java.util.EnumSet;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import software.amazon.awssdk.annotations.Generated;
import software.amazon.awssdk.annotations.SdkPublicApi;
import software.amazon.awssdk.services.jsonprotocoltests.model.inputeventstream.DefaultInputEvent;
import software.amazon.awssdk.utils.internal.EnumUtils;

/**
* Base interface for all event types in InputEventStream.
Expand Down Expand Up @@ -36,6 +37,8 @@ enum EventType {

UNKNOWN_TO_SDK_VERSION(null);

private static final Map<String, EventType> VALUE_MAP = EnumUtils.uniqueIndex(EventType.class, EventType::toString);

private final String value;

private EventType(String value) {
Expand All @@ -58,8 +61,7 @@ public static EventType fromValue(String value) {
if (value == null) {
return null;
}
return Stream.of(EventType.values()).filter(e -> e.toString().equals(value)).findFirst()
.orElse(UNKNOWN_TO_SDK_VERSION);
return VALUE_MAP.getOrDefault(value, UNKNOWN_TO_SDK_VERSION);
}

/**
Expand All @@ -69,7 +71,9 @@ public static EventType fromValue(String value) {
* @return a {@link Set} of known {@link EventType}s
*/
public static Set<EventType> knownValues() {
return Stream.of(values()).filter(v -> v != UNKNOWN_TO_SDK_VERSION).collect(Collectors.toSet());
Set<EventType> knownValues = EnumSet.allOf(EventType.class);
knownValues.remove(UNKNOWN_TO_SDK_VERSION);
return knownValues;
}
}
}
Expand Down
14 changes: 9 additions & 5 deletions ...gen/src/test/resources/software/amazon/awssdk/codegen/poet/model/inputeventstreamtwo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package software.amazon.awssdk.services.jsonprotocoltests.model;

import java.util.EnumSet;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import software.amazon.awssdk.annotations.Generated;
import software.amazon.awssdk.annotations.SdkPublicApi;
import software.amazon.awssdk.services.jsonprotocoltests.model.inputeventstreamtwo.DefaultInputEventTwo;
import software.amazon.awssdk.utils.internal.EnumUtils;

/**
* Base interface for all event types in InputEventStreamTwo.
Expand Down Expand Up @@ -36,6 +37,8 @@ enum EventType {

UNKNOWN_TO_SDK_VERSION(null);

private static final Map<String, EventType> VALUE_MAP = EnumUtils.uniqueIndex(EventType.class, EventType::toString);

private final String value;

private EventType(String value) {
Expand All @@ -58,8 +61,7 @@ public static EventType fromValue(String value) {
if (value == null) {
return null;
}
return Stream.of(EventType.values()).filter(e -> e.toString().equals(value)).findFirst()
.orElse(UNKNOWN_TO_SDK_VERSION);
return VALUE_MAP.getOrDefault(value, UNKNOWN_TO_SDK_VERSION);
}

/**
Expand All @@ -69,7 +71,9 @@ public static EventType fromValue(String value) {
* @return a {@link Set} of known {@link EventType}s
*/
public static Set<EventType> knownValues() {
return Stream.of(values()).filter(v -> v != UNKNOWN_TO_SDK_VERSION).collect(Collectors.toSet());
Set<EventType> knownValues = EnumSet.allOf(EventType.class);
knownValues.remove(UNKNOWN_TO_SDK_VERSION);
return knownValues;
}
}
}
Expand Down
14 changes: 9 additions & 5 deletions ...rc/test/resources/software/amazon/awssdk/codegen/poet/model/sharedstream/eventstream.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
package software.amazon.awssdk.services.sharedeventstream.model;

import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import software.amazon.awssdk.annotations.Generated;
import software.amazon.awssdk.annotations.SdkPublicApi;
import software.amazon.awssdk.core.SdkField;
import software.amazon.awssdk.core.SdkPojo;
import software.amazon.awssdk.services.sharedeventstream.model.eventstream.DefaultPerson;
import software.amazon.awssdk.utils.internal.EnumUtils;

/**
* Base interface for all event types in EventStream.
Expand Down Expand Up @@ -76,6 +77,8 @@ enum EventType {

UNKNOWN_TO_SDK_VERSION(null);

private static final Map<String, EventType> VALUE_MAP = EnumUtils.uniqueIndex(EventType.class, EventType::toString);

private final String value;

private EventType(String value) {
Expand All @@ -98,8 +101,7 @@ public static EventType fromValue(String value) {
if (value == null) {
return null;
}
return Stream.of(EventType.values()).filter(e -> e.toString().equals(value)).findFirst()
.orElse(UNKNOWN_TO_SDK_VERSION);
return VALUE_MAP.getOrDefault(value, UNKNOWN_TO_SDK_VERSION);
}

/**
Expand All @@ -109,7 +111,9 @@ public static EventType fromValue(String value) {
* @return a {@link Set} of known {@link EventType}s
*/
public static Set<EventType> knownValues() {
return Stream.of(values()).filter(v -> v != UNKNOWN_TO_SDK_VERSION).collect(Collectors.toSet());
Set<EventType> knownValues = EnumSet.allOf(EventType.class);
knownValues.remove(UNKNOWN_TO_SDK_VERSION);
return knownValues;
}
}
}
Expand Down
23 changes: 23 additions & 0 deletions utils/src/main/java/software/amazon/awssdk/utils/CollectionUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
Expand Down Expand Up @@ -139,4 +140,26 @@ public static <K, VInT, VOutT> Map<K, VOutT> mapValues(Map<K, VInT> inputMap, Fu
public static <K, V> Map<K, V> inverseMap(Map<V, K> inputMap) {
return inputMap.entrySet().stream().collect(Collectors.toMap(Map.Entry::getValue, Map.Entry::getKey));
}

/**
* For a collection of values of type {@code V} that can all be converted to type {@code K}, create a map that
* indexes all of the values by {@code K}. This requires that no two values map to the same index.
*
* @param values the collection of values to index
* @param indexFunction the function used to convert a value to its index
* @param <K> the index (or key) type
* @param <V> the value type
* @return a (modifiable) map that indexes K to its unique value V
* @throws IllegalArgumentException if any of the values map to the same index
*/
public static <K, V> Map<K, V> uniqueIndex(Iterable<V> values, Function<? super V, K> indexFunction) {
Map<K, V> map = new HashMap<>();
for (V value : values) {
K index = indexFunction.apply(value);
V prev = map.put(index, value);
Validate.isNull(prev, "No duplicate indices allowed but both %s and %s have the same index: %s",
Copy link
Contributor

Choose a reason for hiding this comment

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

Query : Since we are indexing to unique Converted type should that be reflected in the function name ?

In some case in future I might even want to index duplicate value in where I would expect Map<K, List> might have difficulty to use the same function name in this class

Copy link
Contributor Author

@Bennett-Lynch Bennett-Lynch Sep 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Good point/question.

My thinking is that indexing like this would be the common/standard use case, where users expect unique indices, and if we wanted to extend this to something like multi-map support, then that new function name would be specialized to differentiate. E.g., index vs indexMulti, or similar.

However, since the current function can throw, then perhaps there is an argument for giving this one more explicit naming. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed index to uniqueIndex.

prev, value, index);
}
return map;
}
}
Loading