From 594fa02a9cf3efbcad44117a1b73417b1abe275c Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Sat, 30 Apr 2016 12:09:06 +0300 Subject: [PATCH] Optimize memory usage by LockBasedStorageManager inner classes Change all inner classes to be 'static' Use explicit reference to StorageManager instead of implicit reference to outer class. The main problem of inner classes here is that outer instance is being captured at each inheritance level, so e.g. some lazy value impls may store up to three duplicating references. --- .../storage/LockBasedStorageManager.java | 72 +++++++++++-------- 1 file changed, 42 insertions(+), 30 deletions(-) diff --git a/core/util.runtime/src/org/jetbrains/kotlin/storage/LockBasedStorageManager.java b/core/util.runtime/src/org/jetbrains/kotlin/storage/LockBasedStorageManager.java index 9df8d43631a..32a1b19e491 100644 --- a/core/util.runtime/src/org/jetbrains/kotlin/storage/LockBasedStorageManager.java +++ b/core/util.runtime/src/org/jetbrains/kotlin/storage/LockBasedStorageManager.java @@ -111,7 +111,7 @@ public class LockBasedStorageManager implements StorageManager { @NotNull Function1 compute, @NotNull ConcurrentMap map ) { - return new MapBasedMemoizedFunctionToNotNull(map, compute); + return new MapBasedMemoizedFunctionToNotNull(this, map, compute); } @NotNull @@ -126,13 +126,13 @@ public class LockBasedStorageManager implements StorageManager { @NotNull Function1 compute, @NotNull ConcurrentMap map ) { - return new MapBasedMemoizedFunction(map, compute); + return new MapBasedMemoizedFunction(this, map, compute); } @NotNull @Override public NotNullLazyValue createLazyValue(@NotNull Function0 computable) { - return new LockBasedNotNullLazyValue(computable); + return new LockBasedNotNullLazyValue(this, computable); } @NotNull @@ -140,7 +140,7 @@ public class LockBasedStorageManager implements StorageManager { public NotNullLazyValue createRecursionTolerantLazyValue( @NotNull Function0 computable, @NotNull final T onRecursiveCall ) { - return new LockBasedNotNullLazyValue(computable) { + return new LockBasedNotNullLazyValue(this, computable) { @NotNull @Override protected RecursionDetectedResult recursionDetected(boolean firstTime) { @@ -156,7 +156,7 @@ public class LockBasedStorageManager implements StorageManager { final Function1 onRecursiveCall, @NotNull final Function1 postCompute ) { - return new LockBasedNotNullLazyValue(computable) { + return new LockBasedNotNullLazyValue(this, computable) { @NotNull @Override protected RecursionDetectedResult recursionDetected(boolean firstTime) { @@ -176,13 +176,13 @@ public class LockBasedStorageManager implements StorageManager { @NotNull @Override public NullableLazyValue createNullableLazyValue(@NotNull Function0 computable) { - return new LockBasedLazyValue(computable); + return new LockBasedLazyValue(this, computable); } @NotNull @Override public NullableLazyValue createRecursionTolerantNullableLazyValue(@NotNull Function0 computable, final T onRecursiveCall) { - return new LockBasedLazyValue(computable) { + return new LockBasedLazyValue(this, computable) { @NotNull @Override protected RecursionDetectedResult recursionDetected(boolean firstTime) { @@ -196,7 +196,7 @@ public class LockBasedStorageManager implements StorageManager { public NullableLazyValue createNullableLazyValueWithPostCompute( @NotNull Function0 computable, @NotNull final Function1 postCompute ) { - return new LockBasedLazyValue(computable) { + return new LockBasedLazyValue(this, computable) { @Override protected void postCompute(@Nullable T value) { postCompute.invoke(value); @@ -270,14 +270,16 @@ public class LockBasedStorageManager implements StorageManager { RECURSION_WAS_DETECTED } - private class LockBasedLazyValue implements NullableLazyValue { - + // Being static is memory optimization to prevent capturing outer-class reference at each level of inheritance hierarchy + private static class LockBasedLazyValue implements NullableLazyValue { + private final LockBasedStorageManager storageManager; private final Function0 computable; @Nullable private volatile Object value = NotValue.NOT_COMPUTED; - public LockBasedLazyValue(@NotNull Function0 computable) { + public LockBasedLazyValue(@NotNull LockBasedStorageManager storageManager, @NotNull Function0 computable) { + this.storageManager = storageManager; this.computable = computable; } @@ -296,7 +298,7 @@ public class LockBasedStorageManager implements StorageManager { Object _value = value; if (!(_value instanceof NotValue)) return WrappedValues.unescapeThrowable(_value); - lock.lock(); + storageManager.lock.lock(); try { _value = value; if (!(_value instanceof NotValue)) return WrappedValues.unescapeThrowable(_value); @@ -328,11 +330,11 @@ public class LockBasedStorageManager implements StorageManager { // Store only if it's a genuine result, not something thrown through recursionDetected() value = WrappedValues.escapeThrowable(throwable); } - throw exceptionHandlingStrategy.handleException(throwable); + throw storageManager.exceptionHandlingStrategy.handleException(throwable); } } finally { - lock.unlock(); + storageManager.lock.unlock(); } } @@ -342,7 +344,7 @@ public class LockBasedStorageManager implements StorageManager { */ @NotNull protected RecursionDetectedResult recursionDetected(boolean firstTime) { - return recursionDetectedDefault(); + return storageManager.recursionDetectedDefault(); } protected void postCompute(T value) { @@ -350,10 +352,10 @@ public class LockBasedStorageManager implements StorageManager { } } - private class LockBasedNotNullLazyValue extends LockBasedLazyValue implements NotNullLazyValue { + private static class LockBasedNotNullLazyValue extends LockBasedLazyValue implements NotNullLazyValue { - public LockBasedNotNullLazyValue(@NotNull Function0 computable) { - super(computable); + public LockBasedNotNullLazyValue(@NotNull LockBasedStorageManager storageManager, @NotNull Function0 computable) { + super(storageManager, computable); } @Override @@ -365,11 +367,17 @@ public class LockBasedStorageManager implements StorageManager { } } - private class MapBasedMemoizedFunction implements MemoizedFunctionToNullable { + private static class MapBasedMemoizedFunction implements MemoizedFunctionToNullable { + private final LockBasedStorageManager storageManager; private final ConcurrentMap cache; private final Function1 compute; - public MapBasedMemoizedFunction(@NotNull ConcurrentMap map, @NotNull Function1 compute) { + public MapBasedMemoizedFunction( + @NotNull LockBasedStorageManager storageManager, + @NotNull ConcurrentMap map, + @NotNull Function1 compute + ) { + this.storageManager = storageManager; this.cache = map; this.compute = compute; } @@ -380,7 +388,7 @@ public class LockBasedStorageManager implements StorageManager { Object value = cache.get(input); if (value != null && value != NotValue.COMPUTING) return WrappedValues.unescapeExceptionOrNull(value); - lock.lock(); + storageManager.lock.lock(); try { value = cache.get(input); if (value == NotValue.COMPUTING) { @@ -406,25 +414,25 @@ public class LockBasedStorageManager implements StorageManager { return typedValue; } catch (Throwable throwable) { - if (throwable == error) throw exceptionHandlingStrategy.handleException(throwable); + if (throwable == error) throw storageManager.exceptionHandlingStrategy.handleException(throwable); Object oldValue = cache.put(input, WrappedValues.escapeThrowable(throwable)); if (oldValue != NotValue.COMPUTING) { throw raceCondition(input, oldValue); } - throw exceptionHandlingStrategy.handleException(throwable); + throw storageManager.exceptionHandlingStrategy.handleException(throwable); } } finally { - lock.unlock(); + storageManager.lock.unlock(); } } @NotNull private AssertionError recursionDetected(K input) { return sanitizeStackTrace( - new AssertionError("Recursion detected on input: " + input + " under " + LockBasedStorageManager.this) + new AssertionError("Recursion detected on input: " + input + " under " + storageManager) ); } @@ -432,7 +440,7 @@ public class LockBasedStorageManager implements StorageManager { private AssertionError raceCondition(K input, Object oldValue) { return sanitizeStackTrace( new AssertionError("Race condition detected on input " + input + ". Old value is " + oldValue + - " under " + LockBasedStorageManager.this) + " under " + storageManager) ); } @@ -441,22 +449,26 @@ public class LockBasedStorageManager implements StorageManager { Object value = cache.get(key); return value != null && value != NotValue.COMPUTING; } + + protected LockBasedStorageManager getStorageManager() { + return storageManager; + } } - private class MapBasedMemoizedFunctionToNotNull extends MapBasedMemoizedFunction implements MemoizedFunctionToNotNull { + private static class MapBasedMemoizedFunctionToNotNull extends MapBasedMemoizedFunction implements MemoizedFunctionToNotNull { public MapBasedMemoizedFunctionToNotNull( - @NotNull ConcurrentMap map, + @NotNull LockBasedStorageManager storageManager, @NotNull ConcurrentMap map, @NotNull Function1 compute ) { - super(map, compute); + super(storageManager, map, compute); } @NotNull @Override public V invoke(K input) { V result = super.invoke(input); - assert result != null : "compute() returned null under " + LockBasedStorageManager.this; + assert result != null : "compute() returned null under " + getStorageManager(); return result; } }