From 41b6fd86decf73912ed7d857a5e5407531a191da Mon Sep 17 00:00:00 2001 From: peter Date: Wed, 10 Aug 2011 17:24:38 +0200 Subject: [PATCH] when memoizing, remember which computation caused reentrancy and when the same reentrant computation invokes the same memoized computation, use the memoized result. ResolveClassTest declared fixed and uncommented --- .../psi/resolve/ResolveClassTest.java | 4 +- .../openapi/util/RecursionManager.java | 89 ++++++++++++------- .../openapi/util/RecursionManagerTest.groovy | 16 ++++ 3 files changed, 73 insertions(+), 36 deletions(-) diff --git a/java/java-tests/testSrc/com/intellij/psi/resolve/ResolveClassTest.java b/java/java-tests/testSrc/com/intellij/psi/resolve/ResolveClassTest.java index 0a51aae4213c..9de0a78a00ec 100644 --- a/java/java-tests/testSrc/com/intellij/psi/resolve/ResolveClassTest.java +++ b/java/java-tests/testSrc/com/intellij/psi/resolve/ResolveClassTest.java @@ -155,14 +155,14 @@ public class ResolveClassTest extends ResolveTestCase { assertNull(target); } - public void _testStaticImportInTheSameClass() throws Exception { + public void testStaticImportInTheSameClass() throws Exception { PsiReference ref = configure(); long start = System.currentTimeMillis(); assertNull(ref.resolve()); PlatformTestUtil.assertTiming("exponent?", 20000, System.currentTimeMillis() - start); } - public void _testStaticImportNetwork() throws Exception { + public void testStaticImportNetwork() throws Exception { PsiReference ref = configure(); int count = 20; diff --git a/platform/util/src/com/intellij/openapi/util/RecursionManager.java b/platform/util/src/com/intellij/openapi/util/RecursionManager.java index b6dc999e0778..e088b45b4bb8 100644 --- a/platform/util/src/com/intellij/openapi/util/RecursionManager.java +++ b/platform/util/src/com/intellij/openapi/util/RecursionManager.java @@ -17,9 +17,8 @@ package com.intellij.openapi.util; import com.intellij.openapi.diagnostic.Logger; import com.intellij.reference.SoftReference; -import com.intellij.util.containers.MultiMap; -import com.intellij.util.containers.MultiMapBasedOnSet; import com.intellij.util.containers.SoftHashMap; +import gnu.trove.THashMap; import gnu.trove.THashSet; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; @@ -75,8 +74,11 @@ public class RecursionManager { if (memoize) { Object o = stack.getMemoizedValue(realKey); if (o != null) { - for (MyKey noCacheUntil : stack.toProhibitCachingOnMemo.get(realKey)) { - stack._prohibitResultCaching(noCacheUntil); + SoftHashMap map = stack.intermediateCache.get(realKey); + if (map != null) { + for (MyKey noCacheUntil : map.keySet()) { + stack.prohibitResultCaching(noCacheUntil); + } } //noinspection unchecked @@ -128,8 +130,10 @@ public class RecursionManager { @Override public void prohibitResultCaching(Object since) { - ourStack.get()._prohibitResultCaching(new MyKey(id, since)); - ourStack.get().memoizationStamp++; + MyKey realKey = new MyKey(id, since); + final CalculationStack stack = ourStack.get(); + stack.enableMemoization(realKey, stack.prohibitResultCaching(realKey)); + stack.memoizationStamp++; } }; @@ -147,13 +151,12 @@ public class RecursionManager { private int depth; private final LinkedHashMap progressMap = new LinkedHashMap(); private final Set toMemoize = new THashSet(); - private final MultiMap toClearMemoized = new MultiMapBasedOnSet(); - private final MultiMap toProhibitCachingOnMemo = new MultiMapBasedOnSet(); - private final SoftHashMap intermediateCache = new SoftHashMap(); + private final THashMap key2ReentrancyDuringItsCalculation = new THashMap(); + private final SoftHashMap> intermediateCache = new SoftHashMap>(); boolean checkReentrancy(MyKey realKey) { if (progressMap.containsKey(realKey)) { - _prohibitResultCaching(realKey); + enableMemoization(realKey, prohibitResultCaching(realKey)); return true; } @@ -162,13 +165,23 @@ public class RecursionManager { @Nullable Object getMemoizedValue(MyKey realKey) { - SoftReference reference = intermediateCache.get(realKey); - if (reference != null) { - if (depth == 0) { - throw new AssertionError("Memoized values with empty stack"); - } - return reference.get(); + SoftHashMap map = intermediateCache.get(realKey); + if (map == null) return null; + + if (depth == 0) { + throw new AssertionError("Memoized values with empty stack"); } + + for (MyKey key : map.keySet()) { + final SoftReference reference = map.get(key); + if (reference != null) { + final Object result = reference.get(); + if (result != null) { + return result; + } + } + } + return null; } @@ -193,7 +206,13 @@ public class RecursionManager { final void maybeMemoize(MyKey realKey, @NotNull Object result, int startStamp) { if (memoizationStamp == startStamp && toMemoize.contains(realKey)) { - intermediateCache.put(realKey, new SoftReference(result)); + SoftHashMap map = intermediateCache.get(realKey); + if (map == null) { + intermediateCache.put(realKey, map = new SoftHashMap()); + } + final MyKey reentered = key2ReentrancyDuringItsCalculation.get(realKey); + assert reentered != null; + map.put(reentered, new SoftReference(result)); } } @@ -209,16 +228,11 @@ public class RecursionManager { depth--; Integer value = progressMap.remove(realKey); toMemoize.remove(realKey); - final Collection stale = toClearMemoized.remove(realKey); - if (stale != null) { - intermediateCache.keySet().removeAll(stale); - toProhibitCachingOnMemo.keySet().removeAll(stale); - } + key2ReentrancyDuringItsCalculation.remove(realKey); if (depth == 0) { - assert intermediateCache.isEmpty() : "non-empty intermediateCache"; - assert toProhibitCachingOnMemo.isEmpty() : "non-empty toProhibitCachingOnMemo"; - assert toClearMemoized.isEmpty() : "non-empty toClearMemoized"; + intermediateCache.clear(); + assert key2ReentrancyDuringItsCalculation.isEmpty() : "non-empty key2ReentrancyDuringItsCalculation"; assert toMemoize.isEmpty() : "non-empty toMemoize"; } @@ -234,30 +248,37 @@ public class RecursionManager { checkDepth("4"); } - private void _prohibitResultCaching(MyKey realKey) { + private void enableMemoization(MyKey realKey, Set loop) { + toMemoize.addAll(loop); + List stack = new ArrayList(progressMap.keySet()); + + for (MyKey key : loop) { + final MyKey existing = key2ReentrancyDuringItsCalculation.get(key); + if (existing == null || stack.indexOf(realKey) >= stack.indexOf(key)) { + key2ReentrancyDuringItsCalculation.put(key, realKey); + } + } + } + + private Set prohibitResultCaching(MyKey realKey) { reentrancyCount++; checkZero(); - Set memo = new THashSet(); + Set loop = new THashSet(); boolean inLoop = false; for (Map.Entry entry: progressMap.entrySet()) { if (inLoop) { entry.setValue(reentrancyCount); - memo.add(entry.getKey()); + loop.add(entry.getKey()); } else if (entry.getKey().equals(realKey)) { inLoop = true; } } - toMemoize.addAll(memo); - for (MyKey key : memo) { - toProhibitCachingOnMemo.putValue(key, realKey); - } - toClearMemoized.putValues(realKey, memo); - checkZero(); + return loop; } private void checkDepth(String s) { diff --git a/platform/util/testSrc/com/intellij/openapi/util/RecursionManagerTest.groovy b/platform/util/testSrc/com/intellij/openapi/util/RecursionManagerTest.groovy index 2abf9f9593c3..e4f93aa0e097 100644 --- a/platform/util/testSrc/com/intellij/openapi/util/RecursionManagerTest.groovy +++ b/platform/util/testSrc/com/intellij/openapi/util/RecursionManagerTest.groovy @@ -118,4 +118,20 @@ public class RecursionManagerTest extends TestCase { } } + public void testFullGraphPerformance() throws Exception { + long start = System.currentTimeMillis() + int count = 20 + Closure cl + cl = { + for (i in 1..count) { + prevent("foo" + i, cl) + } + return "zoo" + } + + assert "zoo" == cl() + + assert System.currentTimeMillis() - start < 10000 + } + }