Skip to content

Commit

Permalink
Avoid using MapRetrievalCache unless node lookup is expensive. The cl…
Browse files Browse the repository at this point in the history
…ass constructs "CacheEntry" objects when adding to the cache, which if the cache is not used makes us slower, and even if the cache is used is roughly a wash speed-wise if map lookup is fast (e.g. a HashMap or ImmutableMap, and an object with a reasonable hashCode()).

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=126874176
  • Loading branch information
Bezier89 authored and cpovirk committed Jul 8, 2016
1 parent f1f4c88 commit 8a2e184
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import java.util.Map;
import java.util.Set;
import java.util.TreeMap;

import javax.annotation.Nullable;

Expand Down Expand Up @@ -90,7 +91,10 @@ private static <N> Map<N, NodeAdjacencies<N>> getNodeMapforBuilder(
this.isDirected = builder.directed;
this.allowsSelfLoops = builder.allowsSelfLoops;
this.nodeOrder = builder.nodeOrder;
this.nodeConnections = new MapRetrievalCache<N, NodeAdjacencies<N>>(nodeConnections);
// Prefer the heavier "MapRetrievalCache" for nodes if lookup is expensive.
this.nodeConnections = (nodeConnections instanceof TreeMap)
? new MapRetrievalCache<N, NodeAdjacencies<N>>(nodeConnections)
: new MapIteratorCache<N, NodeAdjacencies<N>>(nodeConnections);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

import java.util.Map;
import java.util.Set;
import java.util.TreeMap;

import javax.annotation.Nullable;

Expand Down Expand Up @@ -124,9 +125,11 @@ private static <N, E> Map<E, N> getEdgeMapForBuilder(
this.allowsSelfLoops = builder.allowsSelfLoops;
this.nodeOrder = builder.nodeOrder;
this.edgeOrder = builder.edgeOrder;
// Prefer the heavier "MapRetrievalCache" for nodes to optimize for the case where methods
// accessing the same node(s) are called repeatedly, such as in Graphs.removeEdgesConnecting().
this.nodeConnections = new MapRetrievalCache<N, NodeConnections<N, E>>(nodeConnections);
// Prefer the heavier "MapRetrievalCache" for nodes if lookup is expensive. This optimizes
// methods that access the same node(s) repeatedly, such as Graphs.removeEdgesConnecting().
this.nodeConnections = (nodeConnections instanceof TreeMap)
? new MapRetrievalCache<N, NodeConnections<N, E>>(nodeConnections)
: new MapIteratorCache<N, NodeConnections<N, E>>(nodeConnections);
this.edgeToReferenceNode = new MapIteratorCache<E, N>(edgeToReferenceNode);
}

Expand Down

0 comments on commit 8a2e184

Please sign in to comment.