From dd6b0e8b0f143d357784ddb4c16223a3ac125113 Mon Sep 17 00:00:00 2001 From: jasexton Date: Tue, 19 Apr 2016 15:22:35 -0700 Subject: [PATCH] Make mutable directed graphs about 40% smaller. Another nifty benefit: directedGraph.adjacentNodes(node).size() is now O(1). Remove (unused) equals/hashcode impl from NodeAdjacencies. Interestingly, this does make immutable directed graphs slightly larger. The exact % depends on the degree of the nodes. inDegree == 1 and outDegree == 1 is a particularly large increase because two SingletonImmutableSets is much smaller than one ImmutableMap with two entries. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=120276536 --- .../common/graph/ConfigurableGraph.java | 15 +- .../common/graph/DirectedNodeAdjacencies.java | 163 +++++++++++++----- .../google/common/graph/ImmutableGraph.java | 31 +++- .../google/common/graph/NodeAdjacencies.java | 24 +-- .../graph/UndirectedNodeAdjacencies.java | 45 ++--- 5 files changed, 168 insertions(+), 110 deletions(-) diff --git a/guava/src/com/google/common/graph/ConfigurableGraph.java b/guava/src/com/google/common/graph/ConfigurableGraph.java index 2a3caedb8474..a74372b8fdfa 100644 --- a/guava/src/com/google/common/graph/ConfigurableGraph.java +++ b/guava/src/com/google/common/graph/ConfigurableGraph.java @@ -103,16 +103,17 @@ public boolean addEdge(N node1, N node2) { @CanIgnoreReturnValue public boolean removeNode(Object node) { checkNotNull(node, "node"); - if (!nodes().contains(node)) { + NodeAdjacencies connections = nodeConnections.get(node); + if (connections == null) { return false; } - for (N successor : nodeConnections.get(node).successors()) { + for (N successor : connections.successors()) { if (!node.equals(successor)) { // don't remove the successor if it's the input node (=> CME); will be removed below nodeConnections.get(successor).removePredecessor(node); } } - for (N predecessor : nodeConnections.get(node).predecessors()) { + for (N predecessor : connections.predecessors()) { nodeConnections.get(predecessor).removeSuccessor(node); } nodeConnections.remove(node); @@ -125,13 +126,13 @@ public boolean removeEdge(Object node1, Object node2) { checkNotNull(node1, "node1"); checkNotNull(node2, "node2"); NodeAdjacencies connectionsN1 = nodeConnections.get(node1); - NodeAdjacencies connectionsN2 = nodeConnections.get(node2); - if (connectionsN1 == null || connectionsN2 == null) { + if (connectionsN1 == null || !connectionsN1.successors().contains(node2)) { return false; } - boolean result = connectionsN1.removeSuccessor(node2); + NodeAdjacencies connectionsN2 = nodeConnections.get(node2); + connectionsN1.removeSuccessor(node2); connectionsN2.removePredecessor(node1); - return result; + return true; } private NodeAdjacencies newNodeConnections() { diff --git a/guava/src/com/google/common/graph/DirectedNodeAdjacencies.java b/guava/src/com/google/common/graph/DirectedNodeAdjacencies.java index 24fe857fbf50..8879f1958327 100644 --- a/guava/src/com/google/common/graph/DirectedNodeAdjacencies.java +++ b/guava/src/com/google/common/graph/DirectedNodeAdjacencies.java @@ -17,17 +17,19 @@ package com.google.common.graph; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.graph.GraphConstants.EXPECTED_DEGREE; -import com.google.common.base.MoreObjects; -import com.google.common.base.Objects; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Sets; +import com.google.common.base.Predicate; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterators; +import com.google.common.collect.Maps; +import java.util.AbstractSet; import java.util.Collections; +import java.util.Iterator; +import java.util.Map; import java.util.Set; -import javax.annotation.Nullable; - /** * A class representing an origin node's adjacent nodes in a directed graph. * @@ -35,84 +37,153 @@ * @param Node parameter type */ final class DirectedNodeAdjacencies implements NodeAdjacencies { - private final Set predecessors; - private final Set successors; + enum Adjacency { + PRED, + SUCC, + BOTH; + } + + private final Map adjacentNodes; + + private int predecessorCount; + private int successorCount; - private DirectedNodeAdjacencies(Set predecessors, Set successors) { - this.predecessors = checkNotNull(predecessors, "predecessors"); - this.successors = checkNotNull(successors, "successors"); + private DirectedNodeAdjacencies( + Map adjacentNodes, int predecessorCount, int successorCount) { + this.adjacentNodes = checkNotNull(adjacentNodes, "adjacentNodes"); + this.predecessorCount = predecessorCount; + this.successorCount = successorCount; } static DirectedNodeAdjacencies of() { - // TODO(user): Enable users to specify the expected number of neighbors of a new node. - return new DirectedNodeAdjacencies(Sets.newHashSet(), Sets.newHashSet()); + return new DirectedNodeAdjacencies( + Maps.newHashMapWithExpectedSize(EXPECTED_DEGREE), 0, 0); } - static DirectedNodeAdjacencies ofImmutable(Set predecessors, Set successors) { + static DirectedNodeAdjacencies ofImmutable( + Map adjacentNodes, int predecessorCount, int successorCount) { return new DirectedNodeAdjacencies( - ImmutableSet.copyOf(predecessors), ImmutableSet.copyOf(successors)); + ImmutableMap.copyOf(adjacentNodes), predecessorCount, successorCount); } @Override public Set adjacentNodes() { - return Sets.union(predecessors(), successors()); + return Collections.unmodifiableSet(adjacentNodes.keySet()); } @Override public Set predecessors() { - return Collections.unmodifiableSet(predecessors); + // Don't simply use Sets.filter() or we'll end up with O(N) instead of O(1) size(). + return new AbstractSet() { + @Override + public Iterator iterator() { + return Iterators.filter(adjacentNodes().iterator(), new Predicate() { + @Override + public boolean apply(N node) { + return isPredecessor(node); + } + }); + } + + @Override + public int size() { + return predecessorCount; + } + + @Override + public boolean contains(Object o) { + return isPredecessor(o); + } + }; } @Override public Set successors() { - return Collections.unmodifiableSet(successors); + // Don't simply use Sets.filter() or we'll end up with O(N) instead of O(1) size(). + return new AbstractSet() { + @Override + public Iterator iterator() { + return Iterators.filter(adjacentNodes().iterator(), new Predicate() { + @Override + public boolean apply(N node) { + return isSuccessor(node); + } + }); + } + + @Override + public int size() { + return successorCount; + } + + @Override + public boolean contains(Object o) { + return isSuccessor(o); + } + }; } + @SuppressWarnings("unchecked") // Safe because we only cast if node is a key of Map @Override - public boolean removePredecessor(Object node) { + public void removePredecessor(Object node) { checkNotNull(node, "node"); - return predecessors.remove(node); + Adjacency adjacency = adjacentNodes.get(node); + if (adjacency == Adjacency.BOTH) { + adjacentNodes.put((N) node, Adjacency.SUCC); + predecessorCount--; + } else if (adjacency == Adjacency.PRED) { + adjacentNodes.remove(node); + predecessorCount--; + } } + @SuppressWarnings("unchecked") // Safe because we only cast if node is a key of Map @Override - public boolean removeSuccessor(Object node) { + public void removeSuccessor(Object node) { checkNotNull(node, "node"); - return successors.remove(node); + Adjacency adjacency = adjacentNodes.get(node); + if (adjacency == Adjacency.BOTH) { + adjacentNodes.put((N) node, Adjacency.PRED); + successorCount--; + } else if (adjacency == Adjacency.SUCC) { + adjacentNodes.remove(node); + successorCount--; + } } @Override - public boolean addPredecessor(N node) { + public void addPredecessor(N node) { checkNotNull(node, "node"); - return predecessors.add(node); + Adjacency adjacency = adjacentNodes.get(node); + if (adjacency == null) { + adjacentNodes.put(node, Adjacency.PRED); + predecessorCount++; + } else if (adjacency == Adjacency.SUCC) { + adjacentNodes.put(node, Adjacency.BOTH); + predecessorCount++; + } } @Override - public boolean addSuccessor(N node) { + public void addSuccessor(N node) { checkNotNull(node, "node"); - return successors.add(node); + Adjacency adjacency = adjacentNodes.get(node); + if (adjacency == null) { + adjacentNodes.put(node, Adjacency.SUCC); + successorCount++; + } else if (adjacency == Adjacency.PRED) { + adjacentNodes.put(node, Adjacency.BOTH); + successorCount++; + } } - // For now, hashCode() and equals() are unused by any graph implementation. - @Override - public int hashCode() { - return Objects.hashCode(predecessors, successors); + private boolean isPredecessor(Object node) { + Adjacency adjacency = adjacentNodes.get(node); + return (adjacency == Adjacency.PRED || adjacency == Adjacency.BOTH); } - @Override - public boolean equals(@Nullable Object object) { - if (object instanceof DirectedNodeAdjacencies) { - DirectedNodeAdjacencies that = (DirectedNodeAdjacencies) object; - return this.predecessors.equals(that.predecessors) - && this.successors.equals(that.successors); - } - return false; - } - - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("predecessors", predecessors) - .add("successors", successors) - .toString(); + private boolean isSuccessor(Object node) { + Adjacency adjacency = adjacentNodes.get(node); + return (adjacency == Adjacency.SUCC || adjacency == Adjacency.BOTH); } } diff --git a/guava/src/com/google/common/graph/ImmutableGraph.java b/guava/src/com/google/common/graph/ImmutableGraph.java index 220fa1e7c71c..596cb7047b62 100644 --- a/guava/src/com/google/common/graph/ImmutableGraph.java +++ b/guava/src/com/google/common/graph/ImmutableGraph.java @@ -20,8 +20,9 @@ import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.collect.ImmutableMap; +import com.google.common.graph.DirectedNodeAdjacencies.Adjacency; -import java.util.Map; +import java.util.Set; /** * A {@link Graph} whose relationships are constant. Instances of this class may be obtained @@ -61,7 +62,7 @@ public static ImmutableGraph copyOf(ImmutableGraph graph) { return checkNotNull(graph); } - private static Map> getNodeConnections(Graph graph) { + private static ImmutableMap> getNodeConnections(Graph graph) { ImmutableMap.Builder> nodeConnections = ImmutableMap.builder(); for (N node : graph.nodes()) { nodeConnections.put(node, nodeConnectionsOf(graph, node)); @@ -71,7 +72,31 @@ private static Map> getNodeConnections(Graph graph) private static NodeAdjacencies nodeConnectionsOf(Graph graph, N node) { return graph.isDirected() - ? DirectedNodeAdjacencies.ofImmutable(graph.predecessors(node), graph.successors(node)) + ? DirectedNodeAdjacencies.ofImmutable(createAdjacencyMap( + graph, node), graph.predecessors(node).size(), graph.successors(node).size()) : UndirectedNodeAdjacencies.ofImmutable(graph.adjacentNodes(node)); } + + private static ImmutableMap createAdjacencyMap(Graph graph, N node) { + Set predecessors = graph.predecessors(node); + Set successors = graph.successors(node); + ImmutableMap.Builder nodeAdjacencies = ImmutableMap.builder(); + for (N adjacentNode : graph.adjacentNodes(node)) { + nodeAdjacencies.put(adjacentNode, + getAdjacency(predecessors.contains(adjacentNode), successors.contains(adjacentNode))); + } + return nodeAdjacencies.build(); + } + + private static Adjacency getAdjacency(boolean isPredecessor, boolean isSuccesor) { + if (isPredecessor && isSuccesor) { + return Adjacency.BOTH; + } else if (isPredecessor) { + return Adjacency.PRED; + } else if (isSuccesor) { + return Adjacency.SUCC; + } else { + throw new IllegalStateException(); + } + } } diff --git a/guava/src/com/google/common/graph/NodeAdjacencies.java b/guava/src/com/google/common/graph/NodeAdjacencies.java index 813adff7ef41..bb22a649a68f 100644 --- a/guava/src/com/google/common/graph/NodeAdjacencies.java +++ b/guava/src/com/google/common/graph/NodeAdjacencies.java @@ -16,11 +16,9 @@ package com.google.common.graph; -import com.google.errorprone.annotations.CanIgnoreReturnValue; - import java.util.Set; /** - * An interface for representing an origin node's adjacent nodes in a network. + * An interface for representing an origin node's adjacent nodes in a graph. * * @author James Sexton * @param Node parameter type @@ -35,35 +33,23 @@ interface NodeAdjacencies { /** * Remove {@code node} from the set of predecessors. - * - * @return true iff the adjacency relationships changed */ - @CanIgnoreReturnValue - boolean removePredecessor(Object node); + void removePredecessor(Object node); /** * Remove {@code node} from the set of successors. - * - * @return true iff the adjacency relationships changed */ - @CanIgnoreReturnValue - boolean removeSuccessor(Object node); + void removeSuccessor(Object node); /** * Add {@code node} as a predecessor to the origin node. * In the case of an undirected graph, it also becomes a successor. - * - * @return true iff the adjacency relationships changed */ - @CanIgnoreReturnValue - boolean addPredecessor(N node); + void addPredecessor(N node); /** * Add {@code node} as a successor to the origin node. * In the case of an undirected graph, it also becomes a predecessor. - * - * @return true iff the adjacency relationships changed */ - @CanIgnoreReturnValue - boolean addSuccessor(N node); + void addSuccessor(N node); } diff --git a/guava/src/com/google/common/graph/UndirectedNodeAdjacencies.java b/guava/src/com/google/common/graph/UndirectedNodeAdjacencies.java index ee7e9e0593ab..40500fe63eb5 100644 --- a/guava/src/com/google/common/graph/UndirectedNodeAdjacencies.java +++ b/guava/src/com/google/common/graph/UndirectedNodeAdjacencies.java @@ -17,17 +17,14 @@ package com.google.common.graph; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.graph.GraphConstants.EXPECTED_DEGREE; -import com.google.common.base.MoreObjects; -import com.google.common.base.Objects; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import java.util.Collections; import java.util.Set; -import javax.annotation.Nullable; - /** * A class representing an origin node's adjacent nodes in an undirected graph. * @@ -42,8 +39,7 @@ private UndirectedNodeAdjacencies(Set adjacentNodes) { } static UndirectedNodeAdjacencies of() { - // TODO(user): Enable users to specify the expected number of neighbors of a new node. - return new UndirectedNodeAdjacencies(Sets.newHashSet()); + return new UndirectedNodeAdjacencies(Sets.newHashSetWithExpectedSize(EXPECTED_DEGREE)); } static UndirectedNodeAdjacencies ofImmutable(Set adjacentNodes) { @@ -66,45 +62,24 @@ public Set successors() { } @Override - public boolean removePredecessor(Object node) { - return removeSuccessor(node); + public void removePredecessor(Object node) { + removeSuccessor(node); } @Override - public boolean removeSuccessor(Object node) { + public void removeSuccessor(Object node) { checkNotNull(node, "node"); - return adjacentNodes.remove(node); + adjacentNodes.remove(node); } @Override - public boolean addPredecessor(N node) { - return addSuccessor(node); + public void addPredecessor(N node) { + addSuccessor(node); } @Override - public boolean addSuccessor(N node) { + public void addSuccessor(N node) { checkNotNull(node, "node"); - return adjacentNodes.add(node); - } - - @Override - public int hashCode() { - return Objects.hashCode(adjacentNodes); - } - - @Override - public boolean equals(@Nullable Object object) { - if (object instanceof UndirectedNodeAdjacencies) { - UndirectedNodeAdjacencies that = (UndirectedNodeAdjacencies) object; - return this.adjacentNodes.equals(that.adjacentNodes); - } - return false; - } - - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("adjacentNodes", adjacentNodes) - .toString(); + adjacentNodes.add(node); } }