You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2018/03/02 07:53:38 UTC
groovy git commit: Refine "GROOVY-8486 - Closure executed multiple
times even if memoized"
Repository: groovy
Updated Branches:
refs/heads/master 61e79d8dc -> e9d11c4e6
Refine "GROOVY-8486 - Closure executed multiple times even if memoized"
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/e9d11c4e
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/e9d11c4e
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/e9d11c4e
Branch: refs/heads/master
Commit: e9d11c4e60c004f0931d27ae0fec7f52962a64a6
Parents: 61e79d8
Author: sunlan <su...@apache.org>
Authored: Fri Mar 2 15:53:30 2018 +0800
Committer: sunlan <su...@apache.org>
Committed: Fri Mar 2 15:53:30 2018 +0800
----------------------------------------------------------------------
src/main/groovy/groovy/lang/Closure.java | 5 +-
.../groovy/runtime/memoize/CommonCache.java | 14 ++-
.../runtime/memoize/ConcurrentCommonCache.java | 47 +++++++++-
.../runtime/memoize/ConcurrentSoftCache.java | 94 ++++++++++++++++++++
.../runtime/memoize/LRUProtectionStorage.java | 2 +-
.../groovy/runtime/memoize/Memoize.java | 33 ++++---
.../runtime/memoize/ProtectionStorage.java | 4 +-
.../runtime/memoize/ValueConvertable.java | 36 ++++++++
.../runtime/memoize/MemoizeAtLeastTest.groovy | 14 +++
.../runtime/memoize/MemoizeAtMostTest.groovy | 14 +++
.../runtime/memoize/MemoizeBetweenTest.groovy | 14 +++
11 files changed, 251 insertions(+), 26 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/e9d11c4e/src/main/groovy/groovy/lang/Closure.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/groovy/lang/Closure.java b/src/main/groovy/groovy/lang/Closure.java
index 13e3cf2..11e2611 100644
--- a/src/main/groovy/groovy/lang/Closure.java
+++ b/src/main/groovy/groovy/lang/Closure.java
@@ -28,6 +28,7 @@ import org.codehaus.groovy.runtime.InvokerHelper;
import org.codehaus.groovy.runtime.InvokerInvocationException;
import org.codehaus.groovy.runtime.callsite.BooleanClosureWrapper;
import org.codehaus.groovy.runtime.memoize.ConcurrentCommonCache;
+import org.codehaus.groovy.runtime.memoize.ConcurrentSoftCache;
import org.codehaus.groovy.runtime.memoize.LRUCache;
import org.codehaus.groovy.runtime.memoize.Memoize;
@@ -750,7 +751,7 @@ public abstract class Closure<V> extends GroovyObjectSupport implements Cloneabl
public Closure<V> memoizeAtLeast(final int protectedCacheSize) {
if (protectedCacheSize < 0) throw new IllegalArgumentException("A non-negative number is required as the protectedCacheSize parameter for memoizeAtLeast.");
- return Memoize.buildSoftReferenceMemoizeFunction(protectedCacheSize, new ConcurrentCommonCache(), this);
+ return Memoize.buildSoftReferenceMemoizeFunction(protectedCacheSize, new ConcurrentSoftCache<Object, Object>(), this);
}
/**
@@ -782,7 +783,7 @@ public abstract class Closure<V> extends GroovyObjectSupport implements Cloneabl
if (maxCacheSize < 0) throw new IllegalArgumentException("A non-negative number is required as the maxCacheSize parameter for memoizeBetween.");
if (protectedCacheSize > maxCacheSize) throw new IllegalArgumentException("The maxCacheSize parameter to memoizeBetween is required to be greater or equal to the protectedCacheSize parameter.");
- return Memoize.buildSoftReferenceMemoizeFunction(protectedCacheSize, new LRUCache(maxCacheSize), this);
+ return Memoize.buildSoftReferenceMemoizeFunction(protectedCacheSize, new ConcurrentSoftCache<Object, Object>(maxCacheSize), this);
}
/**
http://git-wip-us.apache.org/repos/asf/groovy/blob/e9d11c4e/src/main/java/org/codehaus/groovy/runtime/memoize/CommonCache.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/runtime/memoize/CommonCache.java b/src/main/java/org/codehaus/groovy/runtime/memoize/CommonCache.java
index 29f048f..af193da 100644
--- a/src/main/java/org/codehaus/groovy/runtime/memoize/CommonCache.java
+++ b/src/main/java/org/codehaus/groovy/runtime/memoize/CommonCache.java
@@ -36,7 +36,7 @@ import java.util.Set;
* @param <V> type of the values
* @since 2.5.0
*/
-public class CommonCache<K, V> implements EvictableCache<K, V>, Serializable {
+public class CommonCache<K, V> implements EvictableCache<K, V>, ValueConvertable<V, Object>, Serializable {
private static final long serialVersionUID = 934699400232698324L;
/**
* The default load factor
@@ -130,12 +130,12 @@ public class CommonCache<K, V> implements EvictableCache<K, V>, Serializable {
public V getAndPut(K key, ValueProvider<? super K, ? extends V> valueProvider, boolean shouldCache) {
V value = get(key);
- if (null != value) {
+ if (null != convertValue(value)) {
return value;
}
value = null == valueProvider ? null : valueProvider.provide(key);
- if (shouldCache && null != value) {
+ if (shouldCache && null != convertValue(value)) {
put(key, value);
}
@@ -219,4 +219,12 @@ public class CommonCache<K, V> implements EvictableCache<K, V>, Serializable {
public String toString() {
return map.toString();
}
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public Object convertValue(V value) {
+ return value;
+ }
}
http://git-wip-us.apache.org/repos/asf/groovy/blob/e9d11c4e/src/main/java/org/codehaus/groovy/runtime/memoize/ConcurrentCommonCache.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/runtime/memoize/ConcurrentCommonCache.java b/src/main/java/org/codehaus/groovy/runtime/memoize/ConcurrentCommonCache.java
index f0a1e1c..80f6bdd 100644
--- a/src/main/java/org/codehaus/groovy/runtime/memoize/ConcurrentCommonCache.java
+++ b/src/main/java/org/codehaus/groovy/runtime/memoize/ConcurrentCommonCache.java
@@ -33,7 +33,7 @@ import java.util.concurrent.locks.ReentrantReadWriteLock;
* @since 2.5.0
*/
@ThreadSafe
-public class ConcurrentCommonCache<K, V> implements EvictableCache<K, V>, Serializable {
+public class ConcurrentCommonCache<K, V> implements EvictableCache<K, V>, ValueConvertable<V, Object>, Serializable {
private static final long serialVersionUID = -7352338549333024936L;
private final ReentrantReadWriteLock rwl = new ReentrantReadWriteLock();
@@ -129,7 +129,7 @@ public class ConcurrentCommonCache<K, V> implements EvictableCache<K, V>, Serial
readLock.lock();
try {
value = commonCache.get(key);
- if (null != value) {
+ if (null != convertValue(value)) {
return value;
}
} finally {
@@ -140,12 +140,12 @@ public class ConcurrentCommonCache<K, V> implements EvictableCache<K, V>, Serial
try {
// try to find the cached value again
value = commonCache.get(key);
- if (null != value) {
+ if (null != convertValue(value)) {
return value;
}
value = null == valueProvider ? null : valueProvider.provide(key);
- if (shouldCache && null != value) {
+ if (shouldCache && null != convertValue(value)) {
commonCache.put(key, value);
}
} finally {
@@ -245,4 +245,43 @@ public class ConcurrentCommonCache<K, V> implements EvictableCache<K, V>, Serial
writeLock.unlock();
}
}
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public Object convertValue(V value) {
+ return value;
+ }
+
+ /**
+ * deal with the backed cache guarded by write lock
+ * @param action the content to complete
+ */
+ public <R> R doWithWriteLock(Action<K, V, R> action) {
+ writeLock.lock();
+ try {
+ return action.doWith(commonCache);
+ } finally {
+ writeLock.unlock();
+ }
+ }
+
+ /**
+ * deal with the backed cache guarded by read lock
+ * @param action the content to complete
+ */
+ public <R> R doWithReadLock(Action<K, V, R> action) {
+ readLock.lock();
+ try {
+ return action.doWith(commonCache);
+ } finally {
+ readLock.unlock();
+ }
+ }
+
+ @FunctionalInterface
+ public interface Action<K, V, R> {
+ R doWith(CommonCache<K, V> commonCache);
+ }
}
http://git-wip-us.apache.org/repos/asf/groovy/blob/e9d11c4e/src/main/java/org/codehaus/groovy/runtime/memoize/ConcurrentSoftCache.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/runtime/memoize/ConcurrentSoftCache.java b/src/main/java/org/codehaus/groovy/runtime/memoize/ConcurrentSoftCache.java
new file mode 100644
index 0000000..a265810
--- /dev/null
+++ b/src/main/java/org/codehaus/groovy/runtime/memoize/ConcurrentSoftCache.java
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.codehaus.groovy.runtime.memoize;
+
+import java.lang.ref.SoftReference;
+import java.util.Map;
+
+/**
+ * Represents concurrent cache holding SoftReference instance as value
+ *
+ * @param <K> key type
+ * @param <V> real value type
+ */
+public class ConcurrentSoftCache<K, V> extends ConcurrentCommonCache<K, SoftReference<V>> {
+ private static final long serialVersionUID = 5646536868666351819L;
+
+
+ /**
+ * Constructs a cache with unlimited size
+ */
+ public ConcurrentSoftCache() {
+ super();
+ }
+
+ /**
+ * Constructs a cache with limited size
+ *
+ * @param initialCapacity initial capacity of the cache
+ * @param maxSize max size of the cache
+ * @param evictionStrategy LRU or FIFO, see {@link org.codehaus.groovy.runtime.memoize.EvictableCache.EvictionStrategy}
+ */
+ public ConcurrentSoftCache(int initialCapacity, int maxSize, EvictionStrategy evictionStrategy) {
+ super(initialCapacity, maxSize, evictionStrategy);
+ }
+
+ /**
+ * Constructs a LRU cache with the specified initial capacity and max size.
+ * The LRU cache is slower than {@link LRUCache}
+ *
+ * @param initialCapacity initial capacity of the LRU cache
+ * @param maxSize max size of the LRU cache
+ */
+ public ConcurrentSoftCache(int initialCapacity, int maxSize) {
+ super(initialCapacity, maxSize);
+ }
+
+ /**
+ * Constructs a LRU cache with the default initial capacity(16)
+ *
+ * @param maxSize max size of the LRU cache
+ * @see #ConcurrentSoftCache(int, int)
+ */
+ public ConcurrentSoftCache(int maxSize) {
+ super(maxSize);
+ }
+
+ /**
+ * Constructs a cache backed by the specified {@link java.util.Map} instance
+ *
+ * @param map the {@link java.util.Map} instance
+ */
+ public ConcurrentSoftCache(Map<K, SoftReference<V>> map) {
+ super(map);
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public Object convertValue(SoftReference<V> value) {
+ if (null == value) {
+ return null;
+ }
+
+ return value.get();
+ }
+}
http://git-wip-us.apache.org/repos/asf/groovy/blob/e9d11c4e/src/main/java/org/codehaus/groovy/runtime/memoize/LRUProtectionStorage.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/runtime/memoize/LRUProtectionStorage.java b/src/main/java/org/codehaus/groovy/runtime/memoize/LRUProtectionStorage.java
index 1a1e65d..b7b0637 100644
--- a/src/main/java/org/codehaus/groovy/runtime/memoize/LRUProtectionStorage.java
+++ b/src/main/java/org/codehaus/groovy/runtime/memoize/LRUProtectionStorage.java
@@ -42,7 +42,7 @@ final class LRUProtectionStorage extends LinkedHashMap<Object, Object> implement
* The eldest entry should be removed when we reached the maximum cache size
*/
@Override
- protected boolean removeEldestEntry(final Map.Entry<Object, Object> eldest) {
+ protected synchronized boolean removeEldestEntry(final Map.Entry<Object, Object> eldest) {
return size() > maxSize;
}
http://git-wip-us.apache.org/repos/asf/groovy/blob/e9d11c4e/src/main/java/org/codehaus/groovy/runtime/memoize/Memoize.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/runtime/memoize/Memoize.java b/src/main/java/org/codehaus/groovy/runtime/memoize/Memoize.java
index f9ab152..035228f 100644
--- a/src/main/java/org/codehaus/groovy/runtime/memoize/Memoize.java
+++ b/src/main/java/org/codehaus/groovy/runtime/memoize/Memoize.java
@@ -76,7 +76,7 @@ public abstract class Memoize {
* @param <V> The closure's return type
* @return A new memoized closure
*/
- public static <V> Closure<V> buildSoftReferenceMemoizeFunction(final int protectedCacheSize, final MemoizeCache<Object, Object> cache, final Closure<V> closure) {
+ public static <V> Closure<V> buildSoftReferenceMemoizeFunction(final int protectedCacheSize, final MemoizeCache<Object, SoftReference<Object>> cache, final Closure<V> closure) {
final ProtectionStorage lruProtectionStorage = protectedCacheSize > 0 ?
new LRUProtectionStorage(protectedCacheSize) :
new NullProtectionStorage(); // Nothing should be done when no elements need protection against eviction
@@ -120,13 +120,17 @@ public abstract class Memoize {
final MemoizeCache<Object, Object> cache;
final Closure<V> closure;
- MemoizeFunction(final MemoizeCache<Object, Object> cache, Closure<V> closure) {
+ MemoizeFunction(final MemoizeCache<Object, ?> cache, Closure<V> closure) {
super(closure.getOwner());
- this.cache = cache;
+ this.cache = coerce(cache);
this.closure = closure;
parameterTypes = closure.getParameterTypes();
maximumNumberOfParameters = closure.getMaximumNumberOfParameters();
}
+
+ private static MemoizeCache coerce(MemoizeCache<Object, ?> cache) {
+ return cache;
+ }
@Override
public V call(final Object... args) {
@@ -150,26 +154,27 @@ public abstract class Memoize {
final ProtectionStorage lruProtectionStorage;
final ReferenceQueue queue;
- SoftReferenceMemoizeFunction(final MemoizeCache<Object, Object> cache, Closure<V> closure,
+ SoftReferenceMemoizeFunction(final MemoizeCache<Object, SoftReference<Object>> cache, Closure<V> closure,
ProtectionStorage lruProtectionStorage, ReferenceQueue queue) {
super(cache, closure);
this.lruProtectionStorage = lruProtectionStorage;
this.queue = queue;
}
- @Override public V call(final Object... args) {
+ @Override
+ public V call(final Object... args) {
if (queue.poll() != null) cleanUpNullReferences(cache, queue); // if something has been evicted, do a clean-up
final Object key = generateKey(args);
- final SoftReference reference = (SoftReference) cache.get(key);
- Object result = reference != null ? reference.get() : null;
- if (result == null) {
- result = closure.call(args);
- if (result == null) {
- result = MEMOIZE_NULL;
- }
- cache.put(key, new SoftReference(result, queue));
- }
+
+ SoftReference reference = (SoftReference) cache.getAndPut(key, k -> {
+ Object r = closure.call(args);
+
+ return null != r ? new SoftReference<Object>(r, queue) : new SoftReference<Object>(MEMOIZE_NULL);
+ });
+
+ Object result = reference.get();
lruProtectionStorage.touch(key, result);
+
return result == MEMOIZE_NULL ? null : (V) result;
}
http://git-wip-us.apache.org/repos/asf/groovy/blob/e9d11c4e/src/main/java/org/codehaus/groovy/runtime/memoize/ProtectionStorage.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/runtime/memoize/ProtectionStorage.java b/src/main/java/org/codehaus/groovy/runtime/memoize/ProtectionStorage.java
index 31159a4..a820389 100644
--- a/src/main/java/org/codehaus/groovy/runtime/memoize/ProtectionStorage.java
+++ b/src/main/java/org/codehaus/groovy/runtime/memoize/ProtectionStorage.java
@@ -24,6 +24,6 @@ package org.codehaus.groovy.runtime.memoize;
*
* @author Vaclav Pech
*/
-interface ProtectionStorage {
- void touch(Object key, Object value);
+interface ProtectionStorage<K, V> {
+ void touch(K key, V value);
}
http://git-wip-us.apache.org/repos/asf/groovy/blob/e9d11c4e/src/main/java/org/codehaus/groovy/runtime/memoize/ValueConvertable.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/runtime/memoize/ValueConvertable.java b/src/main/java/org/codehaus/groovy/runtime/memoize/ValueConvertable.java
new file mode 100644
index 0000000..3f3f692
--- /dev/null
+++ b/src/main/java/org/codehaus/groovy/runtime/memoize/ValueConvertable.java
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.codehaus.groovy.runtime.memoize;
+
+/**
+ * To support caches whose values are convertable, e.g. SoftReference, WeakReference
+ *
+ * @param <V1> source value type, e.g. SoftReference, WeakReference
+ * @param <V2> target value type, e.g. value that SoftReference or WeakReference referenced
+ */
+public interface ValueConvertable<V1, V2> {
+ /**
+ * convert the original value to the target value
+ *
+ * @param value the original value
+ * @return the converted value
+ */
+ V2 convertValue(V1 value);
+}
http://git-wip-us.apache.org/repos/asf/groovy/blob/e9d11c4e/src/test/org/codehaus/groovy/runtime/memoize/MemoizeAtLeastTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/runtime/memoize/MemoizeAtLeastTest.groovy b/src/test/org/codehaus/groovy/runtime/memoize/MemoizeAtLeastTest.groovy
index 2535b81..dd1dda8 100644
--- a/src/test/org/codehaus/groovy/runtime/memoize/MemoizeAtLeastTest.groovy
+++ b/src/test/org/codehaus/groovy/runtime/memoize/MemoizeAtLeastTest.groovy
@@ -18,6 +18,8 @@
*/
package org.codehaus.groovy.runtime.memoize
+import java.util.concurrent.atomic.AtomicInteger
+
/**
* @author Vaclav Pech
*/
@@ -37,4 +39,16 @@ public class MemoizeAtLeastTest extends AbstractMemoizeTestCase {
[1, 2, 3, 4, 5, 6].each {mem(it)}
assert flag
}
+
+ public void testMemoizeAtLeastConcurrently() {
+ AtomicInteger cnt = new AtomicInteger(0)
+ Closure cl = {
+ cnt.incrementAndGet()
+ it * 2
+ }
+ Closure mem = cl.memoizeAtLeast(3)
+ [4, 5, 6, 4, 5, 6, 4, 5, 6].collect { num -> Thread.start { mem(num) } }*.join()
+
+ assert 3 == cnt.get()
+ }
}
http://git-wip-us.apache.org/repos/asf/groovy/blob/e9d11c4e/src/test/org/codehaus/groovy/runtime/memoize/MemoizeAtMostTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/runtime/memoize/MemoizeAtMostTest.groovy b/src/test/org/codehaus/groovy/runtime/memoize/MemoizeAtMostTest.groovy
index c8c38a4..23279c6 100644
--- a/src/test/org/codehaus/groovy/runtime/memoize/MemoizeAtMostTest.groovy
+++ b/src/test/org/codehaus/groovy/runtime/memoize/MemoizeAtMostTest.groovy
@@ -18,6 +18,8 @@
*/
package org.codehaus.groovy.runtime.memoize
+import java.util.concurrent.atomic.AtomicInteger
+
/**
* @author Vaclav Pech
*/
@@ -69,4 +71,16 @@ public class MemoizeAtMostTest extends AbstractMemoizeTestCase {
assert 10 == mem(5)
assert flag
}
+
+ public void testMemoizeAtMostConcurrently() {
+ AtomicInteger cnt = new AtomicInteger(0)
+ Closure cl = {
+ cnt.incrementAndGet()
+ it * 2
+ }
+ Closure mem = cl.memoizeAtMost(3)
+ [4, 5, 6, 4, 5, 6, 4, 5, 6, 4, 5, 6].collect { num -> Thread.start { mem(num) } }*.join()
+
+ assert 3 == cnt.get()
+ }
}
http://git-wip-us.apache.org/repos/asf/groovy/blob/e9d11c4e/src/test/org/codehaus/groovy/runtime/memoize/MemoizeBetweenTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/runtime/memoize/MemoizeBetweenTest.groovy b/src/test/org/codehaus/groovy/runtime/memoize/MemoizeBetweenTest.groovy
index 1ab472b..0f40c0d 100644
--- a/src/test/org/codehaus/groovy/runtime/memoize/MemoizeBetweenTest.groovy
+++ b/src/test/org/codehaus/groovy/runtime/memoize/MemoizeBetweenTest.groovy
@@ -18,6 +18,8 @@
*/
package org.codehaus.groovy.runtime.memoize
+import java.util.concurrent.atomic.AtomicInteger
+
/**
* @author Vaclav Pech
*/
@@ -83,4 +85,16 @@ public class MemoizeBetweenTest extends AbstractMemoizeTestCase {
assert 10 == mem(5)
assert flag
}
+
+ public void testMemoizeBetweenConcurrently() {
+ AtomicInteger cnt = new AtomicInteger(0)
+ Closure cl = {
+ cnt.incrementAndGet()
+ it * 2
+ }
+ Closure mem = cl.memoizeBetween(3, 3)
+ [4, 5, 6, 4, 5, 6, 4, 5, 6, 4, 5, 6].collect { num -> Thread.start { mem(num) } }*.join()
+
+ assert 3 == cnt.get()
+ }
}