Skip to content

Commit

Permalink
Reduce builder copies using a managed reference
Browse files Browse the repository at this point in the history
This commit reduces the number of copies builders need to make when
building up immutable objects.
  • Loading branch information
mtdowling committed Nov 29, 2021
1 parent 03909ca commit 41774d6
Show file tree
Hide file tree
Showing 27 changed files with 657 additions and 192 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

package software.amazon.smithy.aws.traits;

import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
Expand All @@ -26,6 +25,7 @@
import software.amazon.smithy.model.traits.AbstractTrait;
import software.amazon.smithy.model.traits.AbstractTraitBuilder;
import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.utils.BuilderRef;
import software.amazon.smithy.utils.ListUtils;
import software.amazon.smithy.utils.SmithyBuilder;
import software.amazon.smithy.utils.SmithyUnstableApi;
Expand Down Expand Up @@ -56,7 +56,7 @@ private HttpChecksumTrait(HttpChecksumTrait.Builder builder) {
this.requestChecksumRequired = builder.requestChecksumRequired;
this.requestAlgorithmMember = builder.requestAlgorithmMember;
this.requestValidationModeMember = builder.requestValidationModeMember;
this.responseAlgorithms = ListUtils.copyOf(builder.responseAlgorithms);
this.responseAlgorithms = builder.responseAlgorithms.copy();
}

public static Builder builder() {
Expand Down Expand Up @@ -172,7 +172,7 @@ public static final class Builder extends AbstractTraitBuilder<HttpChecksumTrait
private boolean requestChecksumRequired;

private String requestValidationModeMember;
private final List<String> responseAlgorithms = new ArrayList<>();
private final BuilderRef<List<String>> responseAlgorithms = BuilderRef.forList();

private Builder() {}

Expand All @@ -198,12 +198,12 @@ public Builder requestValidationModeMember(String input) {

public Builder responseAlgorithms(List<String> algorithms) {
this.responseAlgorithms.clear();
this.responseAlgorithms.addAll(algorithms);
this.responseAlgorithms.get().addAll(algorithms);
return this;
}

public Builder addResponseAlgorithm(String algorithm) {
this.responseAlgorithms.add(algorithm);
this.responseAlgorithms.get().add(algorithm);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

package software.amazon.smithy.aws.traits.auth;

import java.util.ArrayList;
import java.util.List;
import software.amazon.smithy.model.node.ArrayNode;
import software.amazon.smithy.model.node.Node;
Expand All @@ -25,8 +24,7 @@
import software.amazon.smithy.model.traits.AbstractTrait;
import software.amazon.smithy.model.traits.AbstractTraitBuilder;
import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.utils.ListUtils;
import software.amazon.smithy.utils.SmithyBuilder;
import software.amazon.smithy.utils.BuilderRef;
import software.amazon.smithy.utils.ToSmithyBuilder;

/**
Expand All @@ -41,7 +39,7 @@ public final class CognitoUserPoolsTrait extends AbstractTrait implements ToSmit

private CognitoUserPoolsTrait(Builder builder) {
super(ID, builder.getSourceLocation());
this.providerArns = ListUtils.copyOf(SmithyBuilder.requiredState("providerArns", builder.providerArns));
this.providerArns = builder.providerArns.copy();
}

public static final class Provider extends AbstractTrait.Provider {
Expand Down Expand Up @@ -90,7 +88,7 @@ protected Node createNode() {

/** Builder for {@link CognitoUserPoolsTrait}. */
public static final class Builder extends AbstractTraitBuilder<CognitoUserPoolsTrait, Builder> {
private final List<String> providerArns = new ArrayList<>();
private final BuilderRef<List<String>> providerArns = BuilderRef.forList();

private Builder() {}

Expand All @@ -107,7 +105,7 @@ public CognitoUserPoolsTrait build() {
*/
public Builder providerArns(List<String> providerArns) {
clearProviderArns();
this.providerArns.addAll(providerArns);
this.providerArns.get().addAll(providerArns);
return this;
}

Expand All @@ -118,7 +116,7 @@ public Builder providerArns(List<String> providerArns) {
* @return Returns the builder.
*/
public Builder addProviderArn(String arn) {
providerArns.add(arn);
providerArns.get().add(arn);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,14 @@

package software.amazon.smithy.aws.traits.protocols;

import java.util.ArrayList;
import java.util.List;
import software.amazon.smithy.model.node.Node;
import software.amazon.smithy.model.node.ObjectNode;
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.model.traits.AbstractTrait;
import software.amazon.smithy.model.traits.AbstractTraitBuilder;
import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.utils.ListUtils;
import software.amazon.smithy.utils.BuilderRef;

/**
* Represents a configurable AWS protocol trait.
Expand All @@ -42,8 +41,8 @@ public abstract class AwsProtocolTrait extends AbstractTrait {
// package-private constructor (at least for now)
AwsProtocolTrait(ShapeId id, Builder<?, ?> builder) {
super(id, builder.getSourceLocation());
http = ListUtils.copyOf(builder.http);
eventStreamHttp = ListUtils.copyOf(builder.eventStreamHttp);
http = builder.http.copy();
eventStreamHttp = builder.eventStreamHttp.copy();
}

/**
Expand Down Expand Up @@ -89,8 +88,8 @@ protected Node createNode() {
*/
public abstract static class Builder<T extends Trait, B extends Builder> extends AbstractTraitBuilder<T, B> {

private final List<String> http = new ArrayList<>();
private final List<String> eventStreamHttp = new ArrayList<>();
private final BuilderRef<List<String>> http = BuilderRef.forList();
private final BuilderRef<List<String>> eventStreamHttp = BuilderRef.forList();

/**
* Sets the list of supported HTTP protocols.
Expand All @@ -101,7 +100,7 @@ public abstract static class Builder<T extends Trait, B extends Builder> extends
@SuppressWarnings("unchecked")
public B http(List<String> http) {
this.http.clear();
this.http.addAll(http);
this.http.get().addAll(http);
return (B) this;
}

Expand All @@ -114,7 +113,7 @@ public B http(List<String> http) {
@SuppressWarnings("unchecked")
public B eventStreamHttp(List<String> eventStreamHttp) {
this.eventStreamHttp.clear();
this.eventStreamHttp.addAll(eventStreamHttp);
this.eventStreamHttp.get().addAll(eventStreamHttp);
return (B) this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,14 @@

package software.amazon.smithy.build;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.validation.Severity;
import software.amazon.smithy.model.validation.ValidationEvent;
import software.amazon.smithy.utils.ListUtils;
import software.amazon.smithy.utils.MapUtils;
import software.amazon.smithy.utils.BuilderRef;
import software.amazon.smithy.utils.SmithyBuilder;

/**
Expand All @@ -41,8 +37,8 @@ public final class ProjectionResult {
private ProjectionResult(Builder builder) {
this.projectionName = SmithyBuilder.requiredState("projectionName", builder.projectionName);
this.model = SmithyBuilder.requiredState("model", builder.model);
this.events = ListUtils.copyOf(builder.events);
this.pluginManifests = MapUtils.copyOf(builder.pluginManifests);
this.events = builder.events.copy();
this.pluginManifests = builder.pluginManifests.copy();
}

/**
Expand Down Expand Up @@ -115,8 +111,8 @@ public Optional<FileManifest> getPluginManifest(String pluginName) {
public static final class Builder implements SmithyBuilder<ProjectionResult> {
private String projectionName;
private Model model;
private final Map<String, FileManifest> pluginManifests = new HashMap<>();
private final Collection<ValidationEvent> events = new ArrayList<>();
private final BuilderRef<Map<String, FileManifest>> pluginManifests = BuilderRef.forUnorderedMap();
private final BuilderRef<List<ValidationEvent>> events = BuilderRef.forList();

@Override
public ProjectionResult build() {
Expand Down Expand Up @@ -153,7 +149,7 @@ public Builder projectionName(String projectionName) {
* @return Returns the builder.
*/
public Builder addPluginManifest(String pluginName, FileManifest manifest) {
pluginManifests.put(pluginName, manifest);
pluginManifests.get().put(pluginName, manifest);
return this;
}

Expand All @@ -164,7 +160,7 @@ public Builder addPluginManifest(String pluginName, FileManifest manifest) {
* @return Returns the builder.
*/
public Builder addEvent(ValidationEvent event) {
events.add(Objects.requireNonNull(event));
events.get().add(Objects.requireNonNull(event));
return this;
}

Expand Down
30 changes: 15 additions & 15 deletions smithy-model/src/main/java/software/amazon/smithy/model/Model.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
import software.amazon.smithy.model.traits.TraitDefinition;
import software.amazon.smithy.model.traits.TraitFactory;
import software.amazon.smithy.model.validation.ValidatorFactory;
import software.amazon.smithy.utils.MapUtils;
import software.amazon.smithy.utils.BuilderRef;
import software.amazon.smithy.utils.SmithyBuilder;
import software.amazon.smithy.utils.ToSmithyBuilder;

Expand Down Expand Up @@ -96,8 +96,8 @@ public final class Model implements ToSmithyBuilder<Model> {
private int hash;

private Model(Builder builder) {
shapeMap = MapUtils.copyOf(builder.shapeMap);
metadata = builder.metadata.isEmpty() ? MapUtils.of() : MapUtils.copyOf(builder.metadata);
shapeMap = builder.shapeMap.copy();
metadata = builder.metadata.copy();
}

/**
Expand Down Expand Up @@ -846,24 +846,24 @@ public <T extends KnowledgeIndex> T getKnowledge(Class<T> type, Function<Model,
* Builder used to create a Model.
*/
public static final class Builder implements SmithyBuilder<Model> {
private final Map<String, Node> metadata = new HashMap<>();
private final Map<ShapeId, Shape> shapeMap = new HashMap<>();
private final BuilderRef<Map<String, Node>> metadata = BuilderRef.forUnorderedMap();
private final BuilderRef<Map<ShapeId, Shape>> shapeMap = BuilderRef.forUnorderedMap();

private Builder() {}

public Builder metadata(Map<String, Node> metadata) {
clearMetadata();
this.metadata.putAll(metadata);
this.metadata.get().putAll(metadata);
return this;
}

public Builder putMetadataProperty(String key, Node value) {
metadata.put(Objects.requireNonNull(key), Objects.requireNonNull(value));
metadata.get().put(Objects.requireNonNull(key), Objects.requireNonNull(value));
return this;
}

public Builder removeMetadataProperty(String key) {
metadata.remove(key);
metadata.get().remove(key);
return this;
}

Expand All @@ -887,10 +887,10 @@ public Builder clearMetadata() {
public Builder addShape(Shape shape) {
// Members must be added by their containing shapes.
if (!shape.isMemberShape()) {
shapeMap.put(shape.getId(), shape);
shapeMap.get().put(shape.getId(), shape);
// Automatically add members of the shape.
for (MemberShape memberShape : shape.members()) {
shapeMap.put(memberShape.getId(), memberShape);
shapeMap.get().put(memberShape.getId(), memberShape);
}
}

Expand All @@ -904,7 +904,7 @@ public Builder addShape(Shape shape) {
* @return Returns the builder.
*/
public Builder addShapes(Model model) {
shapeMap.putAll(model.shapeMap);
shapeMap.get().putAll(model.shapeMap);
return this;
}

Expand Down Expand Up @@ -945,13 +945,13 @@ public Builder addShapes(Shape... shapes) {
* @return Returns the builder.
*/
public Builder removeShape(ShapeId shapeId) {
if (shapeMap.containsKey(shapeId)) {
Shape previous = shapeMap.get(shapeId);
shapeMap.remove(shapeId);
if (shapeMap.hasValue() && shapeMap.peek().containsKey(shapeId)) {
Shape previous = shapeMap.peek().get(shapeId);
shapeMap.get().remove(shapeId);

// Automatically remove any members contained in the shape.
for (MemberShape memberShape : previous.members()) {
shapeMap.remove(memberShape.getId());
shapeMap.get().remove(memberShape.getId());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@

package software.amazon.smithy.model.loader;

import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Consumer;
import software.amazon.smithy.model.SourceLocation;
import software.amazon.smithy.model.node.ArrayNode;
Expand Down Expand Up @@ -122,7 +118,8 @@ static Node parseTextBlock(IdlModelParser parser) {
static ObjectNode parseObjectNode(IdlModelParser parser, String parent) {
parser.increaseNestingLevel();
SourceLocation location = parser.currentLocation();
Map<StringNode, Node> entries = new LinkedHashMap<>();
ObjectNode.Builder builder = ObjectNode.builder()
.sourceLocation(location);
parser.expect('{');
parser.ws();

Expand All @@ -138,10 +135,10 @@ static ObjectNode parseObjectNode(IdlModelParser parser, String parent) {
parser.ws();
Node value = parseNode(parser);
StringNode keyNode = new StringNode(key, keyLocation);
Node previous = entries.put(keyNode, value);
if (previous != null) {
if (builder.hasMember(key)) {
throw parser.syntax("Duplicate member of " + parent + ": '" + keyNode.getValue() + '\'');
}
builder.withMember(keyNode, value);
parser.ws();
if (parser.peek() == ',') {
parser.skip();
Expand All @@ -154,7 +151,7 @@ static ObjectNode parseObjectNode(IdlModelParser parser, String parent) {

parser.expect('}');
parser.decreaseNestingLevel();
return new ObjectNode(entries, location);
return builder.build();
}

static String parseNodeObjectKey(IdlModelParser parser) {
Expand All @@ -168,7 +165,8 @@ static String parseNodeObjectKey(IdlModelParser parser) {
private static ArrayNode parseArrayNode(IdlModelParser parser) {
parser.increaseNestingLevel();
SourceLocation location = parser.currentLocation();
List<Node> items = new ArrayList<>();
ArrayNode.Builder builder = ArrayNode.builder()
.sourceLocation(location);
parser.expect('[');
parser.ws();

Expand All @@ -177,7 +175,7 @@ private static ArrayNode parseArrayNode(IdlModelParser parser) {
if (c == ']') {
break;
} else {
items.add(parseNode(parser));
builder.withValue(parseNode(parser));
parser.ws();
if (parser.peek() == ',') {
parser.skip();
Expand All @@ -190,6 +188,6 @@ private static ArrayNode parseArrayNode(IdlModelParser parser) {

parser.expect(']');
parser.decreaseNestingLevel();
return new ArrayNode(items, location);
return builder.build();
}
}
Loading

0 comments on commit 41774d6

Please sign in to comment.