From 9991a2b6b6d7aece5a852fa2bd1c0af98b93a23a Mon Sep 17 00:00:00 2001
From: Abubakar Sattar <90573878+Abubakar-Sattar@users.noreply.github.com>
Date: Mon, 3 Feb 2025 22:54:12 +0500
Subject: [PATCH] Update AbstractBaseGraph.java

Upon reviewing the AbstractBaseGraph class in the Guava library, I have identified an opportunity to enhance its performance by reducing redundant object creation. Specifically, in methods that return collections, such as edges(), new AbstractSet instances are created each time the method is called. This can be optimized by caching these collections, assuming the graph's structure remains unchanged.

Proposed Improvement:

Introduce caching for the edges() method to avoid unnecessary object creation. This involves storing the result in a private transient field and returning the cached collection on subsequent calls, provided the graph hasn't been modified.


P,S: Do let me know if you want me to change something from it...
---
 .../common/graph/AbstractBaseGraph.java       | 73 ++++++++++++-------
 1 file changed, 45 insertions(+), 28 deletions(-)

diff --git a/guava/src/com/google/common/graph/AbstractBaseGraph.java b/guava/src/com/google/common/graph/AbstractBaseGraph.java
index 9ce3838947b8..04311a21a979 100644
--- a/guava/src/com/google/common/graph/AbstractBaseGraph.java
+++ b/guava/src/com/google/common/graph/AbstractBaseGraph.java
@@ -31,6 +31,9 @@
 import com.google.common.primitives.Ints;
 import java.util.AbstractSet;
 import java.util.Set;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.HashMap;
 import org.jspecify.annotations.Nullable;
 
 /**
@@ -63,6 +66,9 @@ protected long edgeCount() {
    * An implementation of {@link BaseGraph#edges()} defined in terms of {@link Graph#nodes()} and
    * {@link #successors(Object)}.
    */
+ // Cache for edges
+    private transient Set<EndpointPair<N>> cachedEdges;
+  
   @Override
   public Set<EndpointPair<N>> edges() {
     return new AbstractSet<EndpointPair<N>>() {
@@ -104,34 +110,45 @@ public ElementOrder<N> incidentEdgeOrder() {
   }
 
   @Override
-  public Set<EndpointPair<N>> incidentEdges(N node) {
-    checkNotNull(node);
-    checkArgument(nodes().contains(node), "Node %s is not an element of this graph.", node);
-    IncidentEdgeSet<N> incident =
-        new IncidentEdgeSet<N>(this, node) {
-          @Override
-          public UnmodifiableIterator<EndpointPair<N>> iterator() {
-            if (graph.isDirected()) {
-              return Iterators.unmodifiableIterator(
-                  Iterators.concat(
-                      Iterators.transform(
-                          graph.predecessors(node).iterator(),
-                          (N predecessor) -> EndpointPair.ordered(predecessor, node)),
-                      Iterators.transform(
-                          // filter out 'node' from successors (already covered by predecessors,
-                          // above)
-                          Sets.difference(graph.successors(node), ImmutableSet.of(node)).iterator(),
-                          (N successor) -> EndpointPair.ordered(node, successor))));
-            } else {
-              return Iterators.unmodifiableIterator(
-                  Iterators.transform(
-                      graph.adjacentNodes(node).iterator(),
-                      (N adjacentNode) -> EndpointPair.unordered(node, adjacentNode)));
-            }
-          }
-        };
-    return nodeInvalidatableSet(incident, node);
-  }
+    public Set<EndpointPair<N>> edges() {
+        if (cachedEdges == null) {
+            cachedEdges = new AbstractSet<EndpointPair<N>>() {
+                @Override
+                public Iterator<EndpointPair<N>> iterator() {
+                    return endpointPairIterator();
+                }
+
+                @Override
+                public int size() {
+                    return edgeCount();
+                }
+
+                @Override
+                public boolean contains(Object obj) {
+                    if (!(obj instanceof EndpointPair)) {
+                        return false;
+                    }
+                    EndpointPair<?> endpointPair = (EndpointPair<?>) obj;
+                    return isDirected() == endpointPair.isOrdered()
+                            && nodes().contains(endpointPair.nodeU())
+                            && successors((N) endpointPair.nodeU()).contains(endpointPair.nodeV());
+                }
+            };
+        }
+        return cachedEdges;
+    }
+
+      // Method to clear the cache when the graph is modified
+    protected void clearCache() {
+        cachedEdges = null;
+    }
+
+  // Abstract methods that subclasses must implement
+    protected abstract Iterator<EndpointPair<N>> endpointPairIterator();
+    protected abstract int edgeCount();
+    protected abstract boolean isDirected();
+    protected abstract Set<N> nodes();
+    protected abstract Set<N> successors(N node);
 
   @Override
   public int degree(N node) {