From e95797890ee50cdd85895c68e626cb58a4ec707c Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Tue, 23 Jan 2024 18:37:02 -0500 Subject: [PATCH] Use queue instead of streams in NodeQuery Changes NodeQuery to use a single instance of a Queue for keeping track of and aggregating query results to avoid extra allocations within queries and when joining the results. --- .../neighbor/IdRefShapeRelationships.java | 1 + .../smithy/model/neighbor/NodeQuery.java | 51 +++++++++++-------- .../smithy/model/neighbor/NodeQueryTest.java | 34 ++++++------- 3 files changed, 47 insertions(+), 39 deletions(-) diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/IdRefShapeRelationships.java b/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/IdRefShapeRelationships.java index b864cfdba38..7466afb6174 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/IdRefShapeRelationships.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/IdRefShapeRelationships.java @@ -33,6 +33,7 @@ */ final class IdRefShapeRelationships { private static final Selector WITH_ID_REF = Selector.parse("[trait|idRef]"); + private final Model model; private final Map> relationships = new HashMap<>(); diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NodeQuery.java b/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NodeQuery.java index 97ba84d3bad..c01738d1850 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NodeQuery.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NodeQuery.java @@ -5,12 +5,12 @@ package software.amazon.smithy.model.neighbor; +import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.Collection; import java.util.List; -import java.util.stream.Collectors; -import java.util.stream.Stream; +import java.util.Queue; import software.amazon.smithy.model.node.Node; -import software.amazon.smithy.utils.ListUtils; /** * Searches {@link Node}s to find matching children. Each search @@ -18,27 +18,27 @@ * and the results are aggregated. */ final class NodeQuery { - private static final Query SELF = Stream::of; + private static final Query SELF = (node, result) -> result.add(node); - private static final Query ANY_MEMBER = (node) -> { + private static final Query ANY_MEMBER = (node, result) -> { if (node == null || !node.isObjectNode()) { - return Stream.empty(); + return; } - return node.expectObjectNode().getMembers().values().stream(); + result.addAll(node.expectObjectNode().getMembers().values()); }; - private static final Query ANY_ELEMENT = (node) -> { + private static final Query ANY_ELEMENT = (node, result) -> { if (node == null || !node.isArrayNode()) { - return Stream.empty(); + return; } - return node.expectArrayNode().getElements().stream(); + result.addAll(node.expectArrayNode().getElements()); }; - private static final Query ANY_MEMBER_NAME = (node) -> { + private static final Query ANY_MEMBER_NAME = (node, result) -> { if (node == null || !node.isObjectNode()) { - return Stream.empty(); + return; } - return node.expectObjectNode().getMembers().keySet().stream(); + result.addAll(node.expectObjectNode().getMembers().keySet()); }; private final List queries = new ArrayList<>(); @@ -52,11 +52,11 @@ NodeQuery self() { } NodeQuery member(String name) { - queries.add(node -> { + queries.add((node, result) -> { if (node == null || !node.isObjectNode()) { - return Stream.empty(); + return; } - return node.expectObjectNode().getMember(name).map(Stream::of).orElse(Stream.empty()); + node.expectObjectNode().getMember(name).ifPresent(result::add); }); return this; } @@ -76,20 +76,27 @@ NodeQuery anyMemberName() { return this; } - List execute(Node node) { + Collection execute(Node node) { + Queue previousResult = new ArrayDeque<>(); + if (queries.isEmpty()) { - return ListUtils.of(); + return previousResult; } - Stream previousResult = Stream.of(node); + previousResult.add(node); for (Query query : queries) { - previousResult = previousResult.flatMap(query::run); + // Each time a query runs, it adds to the queue, but we only want it to + // run on the nodes added by the previous query. + for (int i = previousResult.size(); i > 0; i--) { + query.run(previousResult.poll(), previousResult); + } } - return previousResult.collect(Collectors.toList()); + + return previousResult; } @FunctionalInterface interface Query { - Stream run(Node node); + void run(Node node, Queue result); } } diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/neighbor/NodeQueryTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/neighbor/NodeQueryTest.java index cdfcb03caf2..51ab639251f 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/neighbor/NodeQueryTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/neighbor/NodeQueryTest.java @@ -4,7 +4,7 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.hasSize; -import java.util.List; +import java.util.Collection; import org.junit.jupiter.api.Test; import software.amazon.smithy.model.node.Node; import software.amazon.smithy.model.node.StringNode; @@ -14,7 +14,7 @@ public class NodeQueryTest { public void noQueriesGivesNoResults() { Node node = Node.from("{}"); - List result = new NodeQuery().execute(node); + Collection result = new NodeQuery().execute(node); assertThat(result, hasSize(0)); } @@ -23,7 +23,7 @@ public void noQueriesGivesNoResults() { public void self() { Node node = Node.from("{}"); - List result = new NodeQuery().self().execute(node); + Collection result = new NodeQuery().self().execute(node); assertThat(result, containsInAnyOrder(node)); } @@ -32,7 +32,7 @@ public void self() { public void selfCanBeAppliedMultipleTimes() { Node node = Node.from("{}"); - List result = new NodeQuery().self().self().self().execute(node); + Collection result = new NodeQuery().self().self().self().execute(node); assertThat(result, containsInAnyOrder(node)); } @@ -42,7 +42,7 @@ public void member() { Node member = StringNode.from("bar"); Node node = Node.objectNode().withMember("foo", member); - List result = new NodeQuery().member("foo").execute(node); + Collection result = new NodeQuery().member("foo").execute(node); assertThat(result, containsInAnyOrder(member)); } @@ -53,7 +53,7 @@ public void anyMember() { Node member2 = StringNode.from("member-two"); Node node = Node.objectNode().withMember("one", member1).withMember("two", member2); - List result = new NodeQuery().anyMember().execute(node); + Collection result = new NodeQuery().anyMember().execute(node); assertThat(result, containsInAnyOrder(member1, member2)); } @@ -64,7 +64,7 @@ public void anyElement() { Node element2 = StringNode.from("element-two"); Node node = Node.arrayNode(element1, element2); - List result = new NodeQuery().anyElement().execute(node); + Collection result = new NodeQuery().anyElement().execute(node); assertThat(result, containsInAnyOrder(element1, element2)); } @@ -77,7 +77,7 @@ public void anyMemberName() { Node member2 = StringNode.from("member-two"); Node node = Node.objectNode().withMember(key1, member1).withMember(key2, member2); - List result = new NodeQuery().anyMemberName().execute(node); + Collection result = new NodeQuery().anyMemberName().execute(node); assertThat(result, containsInAnyOrder(key1, key2)); } @@ -86,7 +86,7 @@ public void anyMemberName() { public void memberGivesNoResultsOnNonObjectNode() { Node node = Node.from("[{\"foo\": 0}]"); - List result = new NodeQuery().member("foo").execute(node); + Collection result = new NodeQuery().member("foo").execute(node); assertThat(result, hasSize(0)); } @@ -95,7 +95,7 @@ public void memberGivesNoResultsOnNonObjectNode() { public void memberGivesNoResultsIfMemberNameNotFound() { Node node = Node.from("{\"a\": 0, \"b\": 0}"); - List result = new NodeQuery().member("foo").execute(node); + Collection result = new NodeQuery().member("foo").execute(node); assertThat(result, hasSize(0)); } @@ -104,7 +104,7 @@ public void memberGivesNoResultsIfMemberNameNotFound() { public void anyMemberGivesNoResultsOnNonObjectNode() { Node node = Node.from("[{\"foo\": 0}]"); - List result = new NodeQuery().anyMember().execute(node); + Collection result = new NodeQuery().anyMember().execute(node); assertThat(result, hasSize(0)); } @@ -113,7 +113,7 @@ public void anyMemberGivesNoResultsOnNonObjectNode() { public void anyMemberGivesNoResultsOnEmptyObjectNode() { Node node = Node.from("{}"); - List result = new NodeQuery().anyMember().execute(node); + Collection result = new NodeQuery().anyMember().execute(node); assertThat(result, hasSize(0)); } @@ -122,7 +122,7 @@ public void anyMemberGivesNoResultsOnEmptyObjectNode() { public void anyElementGivesNoResultsOnNonArrayNode() { Node node = Node.from("{\"foo\": [0]}"); - List result = new NodeQuery().anyElement().execute(node); + Collection result = new NodeQuery().anyElement().execute(node); assertThat(result, hasSize(0)); } @@ -131,7 +131,7 @@ public void anyElementGivesNoResultsOnNonArrayNode() { public void anyElementGivesNoResultsOnEmptyArrayNode() { Node node = Node.from("[]"); - List result = new NodeQuery().anyElement().execute(node); + Collection result = new NodeQuery().anyElement().execute(node); assertThat(result, hasSize(0)); } @@ -140,7 +140,7 @@ public void anyElementGivesNoResultsOnEmptyArrayNode() { public void anyMemberNameGivesNoResultsOnNonObjectNode() { Node node = Node.from("1"); - List result = new NodeQuery().anyMemberName().execute(node); + Collection result = new NodeQuery().anyMemberName().execute(node); assertThat(result, hasSize(0)); } @@ -149,7 +149,7 @@ public void anyMemberNameGivesNoResultsOnNonObjectNode() { public void anyMemberNameGivesNoResultsOnEmptyObject() { Node node = Node.from("{}"); - List result = new NodeQuery().anyMemberName().execute(node); + Collection result = new NodeQuery().anyMemberName().execute(node); assertThat(result, hasSize(0)); } @@ -165,7 +165,7 @@ public void eachQueryExecuteOnResultOfPreviousQuery() { .withMember("arr3", Node.arrayNode(element3))); Node node = Node.arrayNode(obj, obj); - List result = new NodeQuery() + Collection result = new NodeQuery() .anyElement() .member("foo") .anyMember()