Skip to content

Commit

Permalink
Optimize the implementation of EqCache to minimize a significant pe…
Browse files Browse the repository at this point in the history
…rformance regression.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=221871956
  • Loading branch information
nreid260 authored and brad4d committed Nov 19, 2018
1 parent 5d04b3c commit 28951ad
Showing 1 changed file with 71 additions and 24 deletions.
95 changes: 71 additions & 24 deletions src/com/google/javascript/rhino/jstype/JSType.java
Expand Up @@ -50,7 +50,7 @@
import com.google.javascript.rhino.ErrorReporter; import com.google.javascript.rhino.ErrorReporter;
import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.JSDocInfo;
import java.io.Serializable; import java.io.Serializable;
import java.util.IdentityHashMap; import java.util.HashMap;
import java.util.Objects; import java.util.Objects;
import javax.annotation.Nullable; import javax.annotation.Nullable;


Expand Down Expand Up @@ -1891,7 +1891,7 @@ boolean isStructuralTyping() {
* cache used by equivalence check logic * cache used by equivalence check logic
*/ */
static class EqCache extends MatchCache { static class EqCache extends MatchCache {
private IdentityHashMap<Object, IdentityHashMap<Object, MatchStatus>> matchCache; private HashMap<Key, MatchStatus> matchCache;


static EqCache create() { static EqCache create() {
return new EqCache(true); return new EqCache(true);
Expand All @@ -1903,48 +1903,95 @@ static EqCache createWithoutStructuralTyping() {


private EqCache(boolean isStructuralTyping) { private EqCache(boolean isStructuralTyping) {
super(isStructuralTyping); super(isStructuralTyping);
this.matchCache = null;
} }


void updateCache(JSType left, JSType right, MatchStatus isMatch) { void updateCache(JSType left, JSType right, MatchStatus isMatch) {
updateCacheAnyType(left, right, isMatch); updateCacheAnyType(left, right, isMatch); // Defer Key instantiation.
} }


void updateCache(TemplateTypeMap left, TemplateTypeMap right, MatchStatus isMatch) { void updateCache(TemplateTypeMap left, TemplateTypeMap right, MatchStatus isMatch) {
updateCacheAnyType(left, right, isMatch); updateCacheAnyType(left, right, isMatch); // Defer Key instantiation.
} }


private void updateCacheAnyType(Object t1, Object t2, MatchStatus isMatch) { private void updateCacheAnyType(Object left, Object right, MatchStatus isMatch) {
IdentityHashMap<Object, MatchStatus> map = this.matchCache.get(t1); if (left == right) {
if (map == null) { // Recall that we never bother reading keys with equal elements.
map = new IdentityHashMap<>(); return;
} }
map.put(t2, isMatch);
this.matchCache.put(t1, map); getMatchCache().put(new Key(left, right), isMatch);
} }


@Nullable @Nullable
MatchStatus checkCache(JSType left, JSType right) { MatchStatus checkCache(JSType left, JSType right) {
return checkCacheAnyType(left, right); return checkCacheAnyType(left, right); // Defer Key instantiation.
} }


@Nullable @Nullable
MatchStatus checkCache(TemplateTypeMap left, TemplateTypeMap right) { MatchStatus checkCache(TemplateTypeMap left, TemplateTypeMap right) {
return checkCacheAnyType(left, right); return checkCacheAnyType(left, right); // Defer Key instantiation.
} }


private MatchStatus checkCacheAnyType(Object t1, Object t2) { private MatchStatus checkCacheAnyType(Object left, Object right) {
if (this.matchCache == null) { if (left == right) {
this.matchCache = new IdentityHashMap<>(); return MatchStatus.MATCH;
} }
// check the cache
if (this.matchCache.containsKey(t1) && this.matchCache.get(t1).containsKey(t2)) { return getMatchCache().putIfAbsent(new Key(left, right), MatchStatus.PROCESSING);
return this.matchCache.get(t1).get(t2); }
} else if (this.matchCache.containsKey(t2) && this.matchCache.get(t2).containsKey(t1)) {
return this.matchCache.get(t2).get(t1); private HashMap<Key, MatchStatus> getMatchCache() {
} else { if (matchCache == null) {
this.updateCacheAnyType(t1, t2, MatchStatus.PROCESSING); matchCache = new HashMap<>();
return null; }
return matchCache;
}

private static final class Key {
private final Object left;
private final Object right;
private final int hashCode; // Cache this calculation because it is made often.

@Override
public int hashCode() {
return hashCode;
}

@Override
@SuppressWarnings({
"ShortCircuitBoolean",
"ReferenceEquality",
"EqualsBrokenForNull",
"EqualsUnsafeCast"
})
public boolean equals(Object other) {
// Calling this with `null` or not a `Key` should cause a crash.
Key that = (Key) other;
if (this == other) {
return true;
}

// Recall that `Key` implements identity equality on `left` and `right`.
//
// Recall that `left` and `right` are not ordered.
//
// Use non-short circuiting operators to eliminate branches. Equality checks are
// side-effect-free and less expensive than branches.
return ((this.left == that.left) & (this.right == that.right))
| ((this.left == that.right) & (this.right == that.left));
}

@SuppressWarnings("ReferenceEquality")
private Key(Object left, Object right) {
this.left = left;
this.right = right;

// XOR the component hashcodes because:
// - It's a symmetric operator, so we don't have to worry about order.
// - It's assumed the inputs are already uniformly distributed and unrelated.
// - `left` and `right` should never be identical.
// Recall that `Key` implements identity equality on `left` and `right`.
this.hashCode = System.identityHashCode(left) ^ System.identityHashCode(right);
} }
} }
} }
Expand Down

0 comments on commit 28951ad

Please sign in to comment.