WFCORE-6369 Optimize AbstractAddStepHandler.Parameters for bulk adds.

Collect into list rather than set for heap efficiency, and because AttributeDefinition does not implement hashCode/equals.
wildfly · Jun 5, 2023 · 2b4d132 · 2b4d132
1 parent 319c661
commit 2b4d132
Showing 1 changed file with 18 additions and 16 deletions.
diff --git a/controller/src/main/java/org/jboss/as/controller/AbstractAddStepHandler.java b/controller/src/main/java/org/jboss/as/controller/AbstractAddStepHandler.java
@@ -27,12 +27,12 @@
 import org.jboss.as.controller.registry.Resource;
 import org.jboss.dmr.ModelNode;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
 
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
 
@@ -74,7 +74,7 @@ public AbstractAddStepHandler(AttributeDefinition... attributes) {
     }
 
     public AbstractAddStepHandler(Parameters parameters) {
-        this.attributes = Collections.unmodifiableCollection(parameters.attributes);
+        this.attributes = parameters.attributes;
     }
 
     @Override
@@ -380,27 +380,29 @@ public Resource createResource(OperationContext context, ModelNode operation) {
     }
 
     public static class Parameters {
-        protected Set<AttributeDefinition> attributes = null;
+        protected Collection<AttributeDefinition> attributes = List.of();
 
         public Parameters() {
         }
 
-        public Parameters addAttribute(AttributeDefinition... attributeDefinitions) {
-            Set<AttributeDefinition> attributeSet = getOrCreateAttributes();
-            attributeSet.addAll(Arrays.asList(attributeDefinitions));
-            return this;
-        }
-
-        public Parameters addAttribute(Collection<AttributeDefinition> attributeDefinitions) {
-            getOrCreateAttributes().addAll(attributeDefinitions);
-            return this;
+        public Parameters addAttribute(AttributeDefinition... attributes) {
+            return this.addAttribute(List.of(attributes));
         }
 
-        private Set<AttributeDefinition> getOrCreateAttributes() {
-            if (attributes == null) {
-                attributes = new LinkedHashSet<>();
+        @SuppressWarnings("unchecked")
+        public Parameters addAttribute(Collection<? extends AttributeDefinition> attributes) {
+            if (this.attributes.isEmpty()) {
+                // Create defensive copy, if collection was not already immutable
+                this.attributes = (attributes instanceof Set) ? Set.copyOf((Set<AttributeDefinition>) attributes) : List.copyOf(attributes);
+            } else {
+                // Use copy-on-write semantics
+                // We expect most users to bulk-add attributes
+                Collection<AttributeDefinition> newAttributes = new ArrayList<>(this.attributes.size() + attributes.size());
+                newAttributes.addAll(this.attributes);
+                newAttributes.addAll(attributes);
+                this.attributes = Collections.unmodifiableCollection(newAttributes);
             }
-            return attributes;
+            return this;
         }
     }
 }