Skip to content

Commit

Permalink
Reduce copies in codegen-core and build
Browse files Browse the repository at this point in the history
This commit makes use of BuilderRef to reduce copies of lists, maps, and
sets created from builders. Most builders are one-time-use, so this
prevents needing to allocate a copy of lists/maps/sets created from
builders and instead has the builder create copies at the point in which
a copied reference is reused.
  • Loading branch information
mtdowling authored and Michael Dowling committed Feb 21, 2022
1 parent 9cd4e10 commit 0a6b269
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import java.nio.file.Path;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
Expand All @@ -31,7 +30,7 @@
import software.amazon.smithy.model.shapes.ToShapeId;
import software.amazon.smithy.model.transform.ModelTransformer;
import software.amazon.smithy.model.validation.ValidationEvent;
import software.amazon.smithy.utils.SetUtils;
import software.amazon.smithy.utils.BuilderRef;
import software.amazon.smithy.utils.SmithyBuilder;
import software.amazon.smithy.utils.ToSmithyBuilder;

Expand Down Expand Up @@ -59,7 +58,7 @@ private PluginContext(Builder builder) {
events = Collections.unmodifiableList(builder.events);
settings = builder.settings;
pluginClassLoader = builder.pluginClassLoader;
sources = SetUtils.copyOf(builder.sources);
sources = builder.sources.copy();
}

/**
Expand Down Expand Up @@ -262,7 +261,7 @@ public static final class Builder implements SmithyBuilder<PluginContext> {
private ObjectNode settings = Node.objectNode();
private FileManifest fileManifest;
private ClassLoader pluginClassLoader;
private Set<Path> sources = Collections.emptySet();
private BuilderRef<Set<Path>> sources = BuilderRef.forOrderedSet();

private Builder() {}

Expand Down Expand Up @@ -354,14 +353,15 @@ public Builder pluginClassLoader(ClassLoader pluginClassLoader) {
}

/**
* Sets the path to models that are considered "source" models of the
* Replaces the path to models that are considered "source" models of the
* package being built.
*
* @param sources Source models to set.
* @return Returns the builder.
*/
public Builder sources(Collection<Path> sources) {
this.sources = new HashSet<>(sources);
this.sources.clear();
this.sources.get().addAll(sources);
return this;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
package software.amazon.smithy.build;

import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
Expand All @@ -27,7 +25,7 @@
import software.amazon.smithy.model.node.ObjectNode;
import software.amazon.smithy.model.transform.ModelTransformer;
import software.amazon.smithy.model.validation.ValidationEvent;
import software.amazon.smithy.utils.SetUtils;
import software.amazon.smithy.utils.BuilderRef;
import software.amazon.smithy.utils.SmithyBuilder;
import software.amazon.smithy.utils.ToSmithyBuilder;

Expand All @@ -52,9 +50,9 @@ private TransformContext(Builder builder) {
transformer = builder.transformer != null ? builder.transformer : ModelTransformer.create();
settings = builder.settings;
originalModel = builder.originalModel;
sources = SetUtils.copyOf(builder.sources);
projectionName = builder.projectionName;
originalModelValidationEvents = builder.originalModelValidationEvents;
sources = builder.sources.copy();
originalModelValidationEvents = builder.originalModelValidationEvents.copy();
}

/**
Expand Down Expand Up @@ -157,10 +155,10 @@ public static final class Builder implements SmithyBuilder<TransformContext> {
private ObjectNode settings = Node.objectNode();
private Model model;
private Model originalModel;
private Set<Path> sources = Collections.emptySet();
private BuilderRef<Set<Path>> sources = BuilderRef.forOrderedSet();
private String projectionName = "source";
private ModelTransformer transformer;
private final List<ValidationEvent> originalModelValidationEvents = new ArrayList<>();
private final BuilderRef<List<ValidationEvent>> originalModelValidationEvents = BuilderRef.forList();

private Builder() {}

Expand All @@ -185,7 +183,8 @@ public Builder originalModel(Model originalModel) {
}

public Builder sources(Set<Path> sources) {
this.sources = Objects.requireNonNull(sources);
this.sources.clear();
this.sources.get().addAll(sources);
return this;
}

Expand All @@ -201,7 +200,7 @@ public Builder transformer(ModelTransformer transformer) {

public Builder originalModelValidationEvents(List<ValidationEvent> originalModelValidationEvents) {
this.originalModelValidationEvents.clear();
this.originalModelValidationEvents.addAll(originalModelValidationEvents);
this.originalModelValidationEvents.get().addAll(originalModelValidationEvents);
return this;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,12 @@

package software.amazon.smithy.build.model;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import software.amazon.smithy.build.SmithyBuildException;
import software.amazon.smithy.model.node.ObjectNode;
import software.amazon.smithy.utils.ListUtils;
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 All @@ -37,10 +34,10 @@ public final class ProjectionConfig implements ToSmithyBuilder<ProjectionConfig>
private final Map<String, ObjectNode> plugins;

private ProjectionConfig(Builder builder) {
this.imports = ListUtils.copyOf(builder.imports);
this.transforms = ListUtils.copyOf(builder.transforms);
this.imports = builder.imports.copy();
this.transforms = builder.transforms.copy();
this.isAbstract = builder.isAbstract;
this.plugins = MapUtils.copyOf(builder.plugins);
this.plugins = builder.plugins.copy();

if (isAbstract && (!plugins.isEmpty() || !imports.isEmpty())) {
throw new SmithyBuildException("Abstract projections must not define plugins or imports");
Expand Down Expand Up @@ -97,9 +94,9 @@ public List<String> getImports() {
*/
public static final class Builder implements SmithyBuilder<ProjectionConfig> {
private boolean isAbstract;
private final List<String> imports = new ArrayList<>();
private final List<TransformConfig> transforms = new ArrayList<>();
private final Map<String, ObjectNode> plugins = new HashMap<>();
private final BuilderRef<List<String>> imports = BuilderRef.forList();
private final BuilderRef<List<TransformConfig>> transforms = BuilderRef.forList();
private final BuilderRef<Map<String, ObjectNode>> plugins = BuilderRef.forOrderedMap();

private Builder() {}

Expand All @@ -126,38 +123,38 @@ public Builder setAbstract(boolean isAbstract) {
}

/**
* Sets the imports of the projection.
* Replaces the imports of the projection.
*
* @param imports Imports to set.
* @return Returns the builder.
*/
public Builder imports(Collection<String> imports) {
this.imports.clear();
this.imports.addAll(imports);
this.imports.get().addAll(imports);
return this;
}

/**
* Sets the transforms of the projection.
* Replaces the transforms of the projection.
*
* @param transforms Transform to set.
* @return Returns the builder.
*/
public Builder transforms(Collection<TransformConfig> transforms) {
this.transforms.clear();
this.transforms.addAll(transforms);
this.transforms.get().addAll(transforms);
return this;
}

/**
* Sets the plugins of the projection.
* Replaces the plugins of the projection.
*
* @param plugins Map of plugin name to plugin settings.
* @return Returns the builder.
*/
public Builder plugins(Map<String, ObjectNode> plugins) {
this.plugins.clear();
this.plugins.putAll(plugins);
this.plugins.get().putAll(plugins);
return this;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,15 @@
package software.amazon.smithy.build.model;

import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
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.Set;
import software.amazon.smithy.model.node.Node;
import software.amazon.smithy.model.node.ObjectNode;
import software.amazon.smithy.utils.ListUtils;
import software.amazon.smithy.utils.MapUtils;
import software.amazon.smithy.utils.BuilderRef;
import software.amazon.smithy.utils.SetUtils;
import software.amazon.smithy.utils.SmithyBuilder;
import software.amazon.smithy.utils.ToSmithyBuilder;
Expand All @@ -51,13 +47,10 @@ private SmithyBuildConfig(Builder builder) {
SmithyBuilder.requiredState("version", builder.version);
version = builder.version;
outputDirectory = builder.outputDirectory;
imports = ListUtils.copyOf(builder.imports);
projections = MapUtils.copyOf(builder.projections);
plugins = new HashMap<>(builder.plugins);
imports = builder.imports.copy();
projections = builder.projections.copy();
plugins = builder.plugins.copy();
ignoreMissingPlugins = builder.ignoreMissingPlugins;
for (String builtin : BUILTIN_PLUGINS) {
plugins.put(builtin, Node.objectNode());
}
}

/**
Expand Down Expand Up @@ -181,9 +174,9 @@ public boolean isIgnoreMissingPlugins() {
* Builder used to create a {@link SmithyBuildConfig}.
*/
public static final class Builder implements SmithyBuilder<SmithyBuildConfig> {
private final List<String> imports = new ArrayList<>();
private final Map<String, ProjectionConfig> projections = new LinkedHashMap<>();
private final Map<String, ObjectNode> plugins = new LinkedHashMap<>();
private final BuilderRef<List<String>> imports = BuilderRef.forList();
private final BuilderRef<Map<String, ProjectionConfig>> projections = BuilderRef.forOrderedMap();
private final BuilderRef<Map<String, ObjectNode>> plugins = BuilderRef.forOrderedMap();
private String version;
private String outputDirectory;
private boolean ignoreMissingPlugins;
Expand All @@ -192,6 +185,11 @@ public static final class Builder implements SmithyBuilder<SmithyBuildConfig> {

@Override
public SmithyBuildConfig build() {
// Add built-in plugins. This is done here to ensure that they cannot be unset.
for (String builtin : BUILTIN_PLUGINS) {
plugins.get().put(builtin, Node.objectNode());
}

return new SmithyBuildConfig(this);
}

Expand Down Expand Up @@ -225,9 +223,9 @@ public Builder load(Path config) {
public Builder merge(SmithyBuildConfig config) {
config.getOutputDirectory().ifPresent(this::outputDirectory);
version(config.getVersion());
imports.addAll(config.getImports());
projections.putAll(config.getProjections());
plugins.putAll(config.getPlugins());
imports.get().addAll(config.getImports());
projections.get().putAll(config.getProjections());
plugins.get().putAll(config.getPlugins());

// If either one wants to ignore missing plugins, then ignore them.
if (config.isIgnoreMissingPlugins()) {
Expand All @@ -249,38 +247,38 @@ public Builder outputDirectory(String outputDirectory) {
}

/**
* Sets imports on the config.
* Replaces imports on the config.
*
* @param imports Imports to set.
* @return Returns the builder.
*/
public Builder imports(Collection<String> imports) {
this.imports.clear();
this.imports.addAll(imports);
this.imports.get().addAll(imports);
return this;
}

/**
* Sets projections on the config.
* Replaces projections on the config.
*
* @param projections Projections to set.
* @return Returns the builder.
*/
public Builder projections(Map<String, ProjectionConfig> projections) {
this.projections.clear();
this.projections.putAll(projections);
this.projections.get().putAll(projections);
return this;
}

/**
* Sets plugins on the config.
* Replaces plugins on the config.
*
* @param plugins Plugins to set.
* @return Returns the builder.
*/
public Builder plugins(Map<String, ObjectNode> plugins) {
this.plugins.clear();
this.plugins.putAll(plugins);
this.plugins.get().putAll(plugins);
return this;
}

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

package software.amazon.smithy.codegen.core;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import software.amazon.smithy.utils.ListUtils;
import software.amazon.smithy.utils.BuilderRef;
import software.amazon.smithy.utils.SmithyBuilder;
import software.amazon.smithy.utils.ToSmithyBuilder;

Expand Down Expand Up @@ -70,14 +69,14 @@ public final class Symbol extends TypedPropertiesBag
private final List<SymbolDependency> dependencies;

private Symbol(Builder builder) {
super(builder.properties);
super(builder);
this.namespace = builder.namespace;
this.namespaceDelimiter = builder.namespaceDelimiter;
this.name = builder.name;
this.declarationFile = builder.declarationFile;
this.definitionFile = !builder.definitionFile.isEmpty() ? builder.definitionFile : declarationFile;
this.references = ListUtils.copyOf(builder.references);
this.dependencies = ListUtils.copyOf(builder.dependencies);
this.references = builder.references.copy();
this.dependencies = builder.dependencies.copy();
}

/**
Expand Down Expand Up @@ -252,8 +251,8 @@ public static final class Builder
private String namespaceDelimiter = "";
private String definitionFile = "";
private String declarationFile = "";
private final List<SymbolReference> references = new ArrayList<>();
private final List<SymbolDependency> dependencies = new ArrayList<>();
private final BuilderRef<List<SymbolReference>> references = BuilderRef.forList();
private final BuilderRef<List<SymbolDependency>> dependencies = BuilderRef.forList();

@Override
public Symbol build() {
Expand Down Expand Up @@ -345,7 +344,7 @@ public Builder addReference(Symbol reference) {
* @return Returns the builder.
*/
public Builder addReference(SymbolReference reference) {
references.add(Objects.requireNonNull(reference));
references.get().add(Objects.requireNonNull(reference));
return this;
}

Expand Down Expand Up @@ -379,7 +378,7 @@ public Builder dependencies(SymbolDependencyContainer container) {
* @return Returns the builder.
*/
public Builder addDependency(SymbolDependencyContainer dependency) {
dependencies.addAll(dependency.getDependencies());
dependencies.get().addAll(dependency.getDependencies());
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public final class SymbolDependency extends TypedPropertiesBag
private final String version;

private SymbolDependency(Builder builder) {
super(builder.properties);
super(builder);
this.dependencyType = builder.dependencyType == null ? "" : builder.dependencyType;
this.packageName = SmithyBuilder.requiredState("packageName", builder.packageName);
this.version = SmithyBuilder.requiredState("version", builder.version);
Expand Down
Loading

0 comments on commit 0a6b269

Please sign in to comment.