From 0a6b26994740d321cc9d045da8673507d73c4725 Mon Sep 17 00:00:00 2001 From: Michael Dowling Date: Mon, 21 Feb 2022 12:39:14 -0800 Subject: [PATCH] Reduce copies in codegen-core and build 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. --- .../amazon/smithy/build/PluginContext.java | 12 ++--- .../amazon/smithy/build/TransformContext.java | 17 ++++--- .../smithy/build/model/ProjectionConfig.java | 29 ++++++------ .../smithy/build/model/SmithyBuildConfig.java | 44 +++++++++---------- .../amazon/smithy/codegen/core/Symbol.java | 17 ++++--- .../smithy/codegen/core/SymbolDependency.java | 2 +- .../smithy/codegen/core/SymbolReference.java | 28 ++++++------ .../codegen/core/TypedPropertiesBag.java | 17 ++++--- 8 files changed, 78 insertions(+), 88 deletions(-) diff --git a/smithy-build/src/main/java/software/amazon/smithy/build/PluginContext.java b/smithy-build/src/main/java/software/amazon/smithy/build/PluginContext.java index f1ba3fdf277..47d709f0f2f 100644 --- a/smithy-build/src/main/java/software/amazon/smithy/build/PluginContext.java +++ b/smithy-build/src/main/java/software/amazon/smithy/build/PluginContext.java @@ -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; @@ -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; @@ -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(); } /** @@ -262,7 +261,7 @@ public static final class Builder implements SmithyBuilder { private ObjectNode settings = Node.objectNode(); private FileManifest fileManifest; private ClassLoader pluginClassLoader; - private Set sources = Collections.emptySet(); + private BuilderRef> sources = BuilderRef.forOrderedSet(); private Builder() {} @@ -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 sources) { - this.sources = new HashSet<>(sources); + this.sources.clear(); + this.sources.get().addAll(sources); return this; } } diff --git a/smithy-build/src/main/java/software/amazon/smithy/build/TransformContext.java b/smithy-build/src/main/java/software/amazon/smithy/build/TransformContext.java index f5672e7c1d6..3450beebd8e 100644 --- a/smithy-build/src/main/java/software/amazon/smithy/build/TransformContext.java +++ b/smithy-build/src/main/java/software/amazon/smithy/build/TransformContext.java @@ -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; @@ -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; @@ -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(); } /** @@ -157,10 +155,10 @@ public static final class Builder implements SmithyBuilder { private ObjectNode settings = Node.objectNode(); private Model model; private Model originalModel; - private Set sources = Collections.emptySet(); + private BuilderRef> sources = BuilderRef.forOrderedSet(); private String projectionName = "source"; private ModelTransformer transformer; - private final List originalModelValidationEvents = new ArrayList<>(); + private final BuilderRef> originalModelValidationEvents = BuilderRef.forList(); private Builder() {} @@ -185,7 +183,8 @@ public Builder originalModel(Model originalModel) { } public Builder sources(Set sources) { - this.sources = Objects.requireNonNull(sources); + this.sources.clear(); + this.sources.get().addAll(sources); return this; } @@ -201,7 +200,7 @@ public Builder transformer(ModelTransformer transformer) { public Builder originalModelValidationEvents(List originalModelValidationEvents) { this.originalModelValidationEvents.clear(); - this.originalModelValidationEvents.addAll(originalModelValidationEvents); + this.originalModelValidationEvents.get().addAll(originalModelValidationEvents); return this; } } diff --git a/smithy-build/src/main/java/software/amazon/smithy/build/model/ProjectionConfig.java b/smithy-build/src/main/java/software/amazon/smithy/build/model/ProjectionConfig.java index 2ab4c980cad..0eead568d26 100644 --- a/smithy-build/src/main/java/software/amazon/smithy/build/model/ProjectionConfig.java +++ b/smithy-build/src/main/java/software/amazon/smithy/build/model/ProjectionConfig.java @@ -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; @@ -37,10 +34,10 @@ public final class ProjectionConfig implements ToSmithyBuilder private final Map 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"); @@ -97,9 +94,9 @@ public List getImports() { */ public static final class Builder implements SmithyBuilder { private boolean isAbstract; - private final List imports = new ArrayList<>(); - private final List transforms = new ArrayList<>(); - private final Map plugins = new HashMap<>(); + private final BuilderRef> imports = BuilderRef.forList(); + private final BuilderRef> transforms = BuilderRef.forList(); + private final BuilderRef> plugins = BuilderRef.forOrderedMap(); private Builder() {} @@ -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 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 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 plugins) { this.plugins.clear(); - this.plugins.putAll(plugins); + this.plugins.get().putAll(plugins); return this; } } diff --git a/smithy-build/src/main/java/software/amazon/smithy/build/model/SmithyBuildConfig.java b/smithy-build/src/main/java/software/amazon/smithy/build/model/SmithyBuildConfig.java index d4e7a0dcd82..95e83de336e 100644 --- a/smithy-build/src/main/java/software/amazon/smithy/build/model/SmithyBuildConfig.java +++ b/smithy-build/src/main/java/software/amazon/smithy/build/model/SmithyBuildConfig.java @@ -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; @@ -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()); - } } /** @@ -181,9 +174,9 @@ public boolean isIgnoreMissingPlugins() { * Builder used to create a {@link SmithyBuildConfig}. */ public static final class Builder implements SmithyBuilder { - private final List imports = new ArrayList<>(); - private final Map projections = new LinkedHashMap<>(); - private final Map plugins = new LinkedHashMap<>(); + private final BuilderRef> imports = BuilderRef.forList(); + private final BuilderRef> projections = BuilderRef.forOrderedMap(); + private final BuilderRef> plugins = BuilderRef.forOrderedMap(); private String version; private String outputDirectory; private boolean ignoreMissingPlugins; @@ -192,6 +185,11 @@ public static final class Builder implements SmithyBuilder { @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); } @@ -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()) { @@ -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 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 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 plugins) { this.plugins.clear(); - this.plugins.putAll(plugins); + this.plugins.get().putAll(plugins); return this; } diff --git a/smithy-codegen-core/src/main/java/software/amazon/smithy/codegen/core/Symbol.java b/smithy-codegen-core/src/main/java/software/amazon/smithy/codegen/core/Symbol.java index 3d33ff34b38..42e5b7603c4 100644 --- a/smithy-codegen-core/src/main/java/software/amazon/smithy/codegen/core/Symbol.java +++ b/smithy-codegen-core/src/main/java/software/amazon/smithy/codegen/core/Symbol.java @@ -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; @@ -70,14 +69,14 @@ public final class Symbol extends TypedPropertiesBag private final List 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(); } /** @@ -252,8 +251,8 @@ public static final class Builder private String namespaceDelimiter = ""; private String definitionFile = ""; private String declarationFile = ""; - private final List references = new ArrayList<>(); - private final List dependencies = new ArrayList<>(); + private final BuilderRef> references = BuilderRef.forList(); + private final BuilderRef> dependencies = BuilderRef.forList(); @Override public Symbol build() { @@ -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; } @@ -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; } diff --git a/smithy-codegen-core/src/main/java/software/amazon/smithy/codegen/core/SymbolDependency.java b/smithy-codegen-core/src/main/java/software/amazon/smithy/codegen/core/SymbolDependency.java index 48082498ea4..cceada12f3a 100644 --- a/smithy-codegen-core/src/main/java/software/amazon/smithy/codegen/core/SymbolDependency.java +++ b/smithy-codegen-core/src/main/java/software/amazon/smithy/codegen/core/SymbolDependency.java @@ -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); diff --git a/smithy-codegen-core/src/main/java/software/amazon/smithy/codegen/core/SymbolReference.java b/smithy-codegen-core/src/main/java/software/amazon/smithy/codegen/core/SymbolReference.java index edcab1571b6..b9d84ed4fe5 100644 --- a/smithy-codegen-core/src/main/java/software/amazon/smithy/codegen/core/SymbolReference.java +++ b/smithy-codegen-core/src/main/java/software/amazon/smithy/codegen/core/SymbolReference.java @@ -16,11 +16,12 @@ package software.amazon.smithy.codegen.core; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import software.amazon.smithy.utils.BuilderRef; +import software.amazon.smithy.utils.SetUtils; import software.amazon.smithy.utils.SmithyBuilder; import software.amazon.smithy.utils.ToSmithyBuilder; @@ -90,19 +91,15 @@ public SymbolReference(Symbol symbol, Map properties, Option... } private SymbolReference(Builder builder) { - super(builder.properties); + super(builder); this.symbol = SmithyBuilder.requiredState("symbol", builder.symbol); this.alias = builder.alias == null ? builder.symbol.getName() : builder.alias; - Set