From 7de890467e57a78a02218bb885926ed14b618870 Mon Sep 17 00:00:00 2001
From: Michael Dowling <mtdowling@gmail.com>
Date: Mon, 5 Oct 2020 11:31:51 -0700
Subject: [PATCH 1/2] Optimize reverse neighbors for memory
This method previously needed lots of intermediate representations
stored in memory to create a Map<ShapeId, List<RelationShip>> that
contains only unique relationships. It was done using Stream,
distinct, and groupingBy. However, when trying to load ridiculously
large models, that approach consume tons of heap. This approach
allocates as little as possible (I think), but does require creating
an ArrayList copy of a Set each time neighbors are returned.
---
.../model/neighbor/NeighborProvider.java | 27 ++++++++++++++-----
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NeighborProvider.java b/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NeighborProvider.java
index 57ec6b0aaf1..a87fe743975 100644
--- a/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NeighborProvider.java
+++ b/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NeighborProvider.java
@@ -16,11 +16,13 @@
package software.amazon.smithy.model.neighbor;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
-import java.util.stream.Collectors;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.shapes.ShapeId;
@@ -122,13 +124,24 @@ static NeighborProvider reverse(Model model) {
* @return Returns the reverse neighbor provider.
*/
static NeighborProvider reverse(Model model, NeighborProvider forwardProvider) {
- Map<ShapeId, List<Relationship>> targetedFrom = model.shapes()
- .map(forwardProvider::getNeighbors)
- .flatMap(List::stream)
- .distinct()
- .collect(Collectors.groupingBy(Relationship::getNeighborShapeId, ListUtils.toUnmodifiableList()));
+ // Note: this method previously needed lots of intermediate representations
+ // stored in memory to create a Map<ShapeId, List<RelationShip>> that contains
+ // only unique relationships. It was done using Stream, distinct, and groupingBy.
+ // however, when trying to load ridiculously large models, that approach consume
+ // tons of heap. This approach allocates as little as possible (I think), but
+ // does require creating an ArrayList copy of a Set each time neighbors are returned.
+ Map<ShapeId, Set<Relationship>> targetedFrom = new HashMap<>();
- return shape -> targetedFrom.getOrDefault(shape.getId(), ListUtils.of());
+ for (Shape shape : model.toSet()) {
+ for (Relationship rel : forwardProvider.getNeighbors(shape)) {
+ targetedFrom.computeIfAbsent(rel.getNeighborShapeId(), id -> new HashSet<>()).add(rel);
+ }
+ }
+
+ return shape -> {
+ Set<Relationship> shapes = targetedFrom.get(shape.getId());
+ return shapes == null ? Collections.emptyList() : ListUtils.copyOf(shapes);
+ };
}
/**
From 9f5ae002509c6ee9d9edfda3662000a829e1644e Mon Sep 17 00:00:00 2001
From: Michael Dowling <mtdowling@gmail.com>
Date: Mon, 5 Oct 2020 11:49:14 -0700
Subject: [PATCH 2/2] Fix typo
Co-authored-by: Chase Coalwell <chasecoa@amazon.com>
---
.../software/amazon/smithy/model/neighbor/NeighborProvider.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NeighborProvider.java b/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NeighborProvider.java
index a87fe743975..93780e093fc 100644
--- a/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NeighborProvider.java
+++ b/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NeighborProvider.java
@@ -127,7 +127,7 @@ static NeighborProvider reverse(Model model, NeighborProvider forwardProvider) {
// Note: this method previously needed lots of intermediate representations
// stored in memory to create a Map<ShapeId, List<RelationShip>> that contains
// only unique relationships. It was done using Stream, distinct, and groupingBy.
- // however, when trying to load ridiculously large models, that approach consume
+ // However, when trying to load ridiculously large models, that approach consumes
// tons of heap. This approach allocates as little as possible (I think), but
// does require creating an ArrayList copy of a Set each time neighbors are returned.
Map<ShapeId, Set<Relationship>> targetedFrom = new HashMap<>();