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 2020/07/11 07:19:27 UTC

[groovy] branch GROOVY-9631 updated (2785bfe -> 17e76d8)

This is an automated email from the ASF dual-hosted git repository.

sunlan pushed a change to branch GROOVY-9631
in repository https://gitbox.apache.org/repos/asf/groovy.git.


 discard 2785bfe  GROOVY-9631: Replace legacy data structure with Java collection
     new 17e76d8  GROOVY-9631: Replace legacy data structure with Java collection

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (2785bfe)
            \
             N -- N -- N   refs/heads/GROOVY-9631 (17e76d8)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 src/main/java/groovy/util/ProxyGenerator.java           |  2 +-
 .../org/codehaus/groovy/ast/tools/GenericsUtils.java    |  2 +-
 .../org/codehaus/groovy/control/ResolveVisitor.java     |  2 +-
 .../codehaus/groovy/runtime/memoize/CommonCache.java    |  2 +-
 .../groovy/runtime/memoize/ConcurrentCommonCache.java   |  2 +-
 .../org/codehaus/groovy/runtime/memoize/LRUCache.java   |  2 +-
 .../org/codehaus/groovy/runtime/memoize/Memoize.java    |  4 ++--
 .../codehaus/groovy/runtime/memoize/MemoizeCache.java   | 17 +----------------
 .../groovy/runtime/memoize/StampedCommonCache.java      |  4 ++--
 .../runtime/memoize/UnlimitedConcurrentCache.java       |  2 +-
 .../transform/stc/AbstractExtensionMethodCache.java     |  2 +-
 .../codehaus/groovy/vmplugin/v8/CacheableCallSite.java  |  2 +-
 .../groovy/runtime/memoize/CommonCacheTest.java         |  2 +-
 .../runtime/memoize/ConcurrentCommonCacheTest.java      |  4 ++--
 .../groovy/runtime/memoize/StampedCommonCacheTest.java  |  4 ++--
 .../runtime/memoize/UnlimitedConcurrentCacheTest.java   |  2 +-
 16 files changed, 20 insertions(+), 35 deletions(-)


[groovy] 01/01: GROOVY-9631: Replace legacy data structure with Java collection

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

sunlan pushed a commit to branch GROOVY-9631
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 17e76d8ef9a019126a501c6025f42cc9dfc7124c
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sat Jul 11 15:18:45 2020 +0800

    GROOVY-9631: Replace legacy data structure with Java collection
---
 .../java/org/codehaus/groovy/ast/ClassHelper.java  |   4 +-
 .../org/codehaus/groovy/reflection/ClassInfo.java  |   6 +-
 .../reflection/GroovyClassValuePreJava7.java       |   1 +
 .../groovy/reflection/MixinInMetaClass.java        |  19 ++-
 .../groovy/runtime/memoize/MemoizeCache.java       |   2 +-
 .../metaclass/ThreadManagedMetaBeanProperty.java   |  16 +--
 .../groovy/util/AbstractConcurrentMap.java         |   1 +
 .../groovy/util/AbstractConcurrentMapBase.java     |   1 +
 .../codehaus/groovy/util/ManagedConcurrentMap.java |   1 +
 .../groovy/util/ManagedIdentityConcurrentMap.java  | 149 +++++++++++++++++++++
 .../util/ManagedIdentityConcurrentMapTest.groovy   |  60 +++++++++
 11 files changed, 236 insertions(+), 24 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/ClassHelper.java b/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
index 17bcc8d..54b320f 100644
--- a/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
+++ b/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
@@ -52,7 +52,7 @@ import org.codehaus.groovy.runtime.GeneratedClosure;
 import org.codehaus.groovy.runtime.GeneratedLambda;
 import org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport;
 import org.codehaus.groovy.transform.trait.Traits;
-import org.codehaus.groovy.util.ManagedConcurrentMap;
+import org.codehaus.groovy.util.ManagedIdentityConcurrentMap;
 import org.codehaus.groovy.util.ReferenceBundle;
 import org.codehaus.groovy.vmplugin.VMPluginFactory;
 import org.objectweb.asm.Opcodes;
@@ -408,7 +408,7 @@ public class ClassHelper {
     }
 
     static class ClassHelperCache {
-        static ManagedConcurrentMap<Class, SoftReference<ClassNode>> classCache = new ManagedConcurrentMap<Class, SoftReference<ClassNode>>(ReferenceBundle.getWeakBundle());
+        static ManagedIdentityConcurrentMap<Class, SoftReference<ClassNode>> classCache = new ManagedIdentityConcurrentMap<Class, SoftReference<ClassNode>>(ReferenceBundle.getWeakBundle());
     }
 
     public static boolean isSAMType(final ClassNode type) {
diff --git a/src/main/java/org/codehaus/groovy/reflection/ClassInfo.java b/src/main/java/org/codehaus/groovy/reflection/ClassInfo.java
index e55b2ed..b0c00e1 100644
--- a/src/main/java/org/codehaus/groovy/reflection/ClassInfo.java
+++ b/src/main/java/org/codehaus/groovy/reflection/ClassInfo.java
@@ -46,7 +46,7 @@ import org.codehaus.groovy.util.Finalizable;
 import org.codehaus.groovy.util.LazyReference;
 import org.codehaus.groovy.util.LockableObject;
 import org.codehaus.groovy.util.ManagedConcurrentLinkedQueue;
-import org.codehaus.groovy.util.ManagedConcurrentMap;
+import org.codehaus.groovy.util.ManagedIdentityConcurrentMap;
 import org.codehaus.groovy.util.ManagedReference;
 import org.codehaus.groovy.util.ReferenceBundle;
 import org.codehaus.groovy.vmplugin.VMPluginFactory;
@@ -82,7 +82,7 @@ public class ClassInfo implements Finalizable {
     private ManagedReference<MetaClass> weakMetaClass;
     MetaMethod[] dgmMetaMethods = MetaMethod.EMPTY_ARRAY;
     MetaMethod[] newMetaMethods = MetaMethod.EMPTY_ARRAY;
-    private ManagedConcurrentMap<Object, MetaClass> perInstanceMetaClassMap;
+    private ManagedIdentityConcurrentMap<Object, MetaClass> perInstanceMetaClassMap;
 
     private static final ReferenceBundle softBundle = ReferenceBundle.getSoftBundle();
     private static final ReferenceBundle weakBundle = ReferenceBundle.getWeakBundle();
@@ -409,7 +409,7 @@ public class ClassInfo implements Finalizable {
 
         if (metaClass != null) {
             if (perInstanceMetaClassMap == null)
-              perInstanceMetaClassMap = new ManagedConcurrentMap<Object, MetaClass>(ReferenceBundle.getWeakBundle());
+              perInstanceMetaClassMap = new ManagedIdentityConcurrentMap<Object, MetaClass>(ReferenceBundle.getWeakBundle());
 
             perInstanceMetaClassMap.put(obj, metaClass);
         }
diff --git a/src/main/java/org/codehaus/groovy/reflection/GroovyClassValuePreJava7.java b/src/main/java/org/codehaus/groovy/reflection/GroovyClassValuePreJava7.java
index 02f893c..7b95d64 100644
--- a/src/main/java/org/codehaus/groovy/reflection/GroovyClassValuePreJava7.java
+++ b/src/main/java/org/codehaus/groovy/reflection/GroovyClassValuePreJava7.java
@@ -28,6 +28,7 @@ import org.codehaus.groovy.util.ReferenceBundle;
  *
  * @param <T>
  */
+@Deprecated
 class GroovyClassValuePreJava7<T> implements GroovyClassValue<T> {
 	private static final ReferenceBundle weakBundle = ReferenceBundle.getWeakBundle();
 
diff --git a/src/main/java/org/codehaus/groovy/reflection/MixinInMetaClass.java b/src/main/java/org/codehaus/groovy/reflection/MixinInMetaClass.java
index afaf956..f9763e1 100644
--- a/src/main/java/org/codehaus/groovy/reflection/MixinInMetaClass.java
+++ b/src/main/java/org/codehaus/groovy/reflection/MixinInMetaClass.java
@@ -32,7 +32,7 @@ import org.codehaus.groovy.runtime.metaclass.MixedInMetaClass;
 import org.codehaus.groovy.runtime.metaclass.MixinInstanceMetaMethod;
 import org.codehaus.groovy.runtime.metaclass.MixinInstanceMetaProperty;
 import org.codehaus.groovy.runtime.metaclass.NewInstanceMetaMethod;
-import org.codehaus.groovy.util.ManagedConcurrentMap;
+import org.codehaus.groovy.util.ManagedIdentityConcurrentMap;
 import org.codehaus.groovy.util.ReferenceBundle;
 
 import java.lang.reflect.Modifier;
@@ -40,19 +40,18 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
 
-public class MixinInMetaClass extends ManagedConcurrentMap {
+public class MixinInMetaClass {
     final ExpandoMetaClass emc;
     final CachedClass mixinClass;
     final CachedConstructor constructor;
-
-    private static final ReferenceBundle softBundle = ReferenceBundle.getSoftBundle();
+    private final ManagedIdentityConcurrentMap managedIdentityConcurrentMap =
+            new ManagedIdentityConcurrentMap(ReferenceBundle.getSoftBundle());
 
     public MixinInMetaClass(ExpandoMetaClass emc, CachedClass mixinClass) {
-        super(softBundle);
         this.emc = emc;
         this.mixinClass = mixinClass;
 
-        constructor = findDefaultConstructor(mixinClass);
+        this.constructor = findDefaultConstructor(mixinClass);
         emc.addMixinClass(this);
     }
 
@@ -70,20 +69,20 @@ public class MixinInMetaClass extends ManagedConcurrentMap {
     }
 
     public synchronized Object getMixinInstance(Object object) {
-        Object mixinInstance = get(object);
+        Object mixinInstance = managedIdentityConcurrentMap.get(object);
         if (mixinInstance == null) {
             mixinInstance = constructor.invoke(MetaClassHelper.EMPTY_ARRAY);
             new MixedInMetaClass(mixinInstance, object);
-            put(object, mixinInstance);
+            managedIdentityConcurrentMap.put(object, mixinInstance);
         }
         return mixinInstance;
     }
 
     public synchronized void setMixinInstance(Object object, Object mixinInstance) {
         if (mixinInstance == null) {
-            remove(object);
+            managedIdentityConcurrentMap.remove(object);
         } else {
-            put(object, mixinInstance);
+            managedIdentityConcurrentMap.put(object, mixinInstance);
         }
     }
 
diff --git a/src/main/java/org/codehaus/groovy/runtime/memoize/MemoizeCache.java b/src/main/java/org/codehaus/groovy/runtime/memoize/MemoizeCache.java
index c525919..dfa4438 100644
--- a/src/main/java/org/codehaus/groovy/runtime/memoize/MemoizeCache.java
+++ b/src/main/java/org/codehaus/groovy/runtime/memoize/MemoizeCache.java
@@ -44,7 +44,7 @@ public interface MemoizeCache<K, V> {
      * Try to get the value from cache.
      * If not found, create the value by {@link ValueProvider} and put it into the cache, at last return the value.
      *
-     * @param key
+     * @param key the key to look up
      * @param valueProvider provide the value if the associated value not found
      * @return the cached value
      */
diff --git a/src/main/java/org/codehaus/groovy/runtime/metaclass/ThreadManagedMetaBeanProperty.java b/src/main/java/org/codehaus/groovy/runtime/metaclass/ThreadManagedMetaBeanProperty.java
index 5074908..8cae32f 100644
--- a/src/main/java/org/codehaus/groovy/runtime/metaclass/ThreadManagedMetaBeanProperty.java
+++ b/src/main/java/org/codehaus/groovy/runtime/metaclass/ThreadManagedMetaBeanProperty.java
@@ -23,7 +23,7 @@ import groovy.lang.MetaBeanProperty;
 import groovy.lang.MetaMethod;
 import org.codehaus.groovy.reflection.CachedClass;
 import org.codehaus.groovy.reflection.ReflectionCache;
-import org.codehaus.groovy.util.ManagedConcurrentMap;
+import org.codehaus.groovy.util.ManagedIdentityConcurrentMap;
 import org.codehaus.groovy.util.ReferenceBundle;
 
 import java.lang.reflect.Modifier;
@@ -39,9 +39,9 @@ import java.util.concurrent.ConcurrentHashMap;
  * @since 1.5
  */
 public class ThreadManagedMetaBeanProperty extends MetaBeanProperty {
-    private static final ConcurrentHashMap<String,ManagedConcurrentMap> PROPNAME_TO_MAP = new ConcurrentHashMap<String, ManagedConcurrentMap>();
+    private static final ConcurrentHashMap<String,ManagedIdentityConcurrentMap> PROPNAME_TO_MAP = new ConcurrentHashMap<String, ManagedIdentityConcurrentMap>();
 
-    private final ManagedConcurrentMap instance2Prop;
+    private final ManagedIdentityConcurrentMap instance2Prop;
 
     private final Class declaringClass;
     private final ThreadBoundGetter getter;
@@ -117,11 +117,11 @@ public class ThreadManagedMetaBeanProperty extends MetaBeanProperty {
         instance2Prop = getInstance2PropName(name);
     }
 
-    private static ManagedConcurrentMap getInstance2PropName(String name) {
-        ManagedConcurrentMap res = PROPNAME_TO_MAP.get(name);
+    private static ManagedIdentityConcurrentMap getInstance2PropName(String name) {
+        ManagedIdentityConcurrentMap res = PROPNAME_TO_MAP.get(name);
         if (res == null) {
-            res = new ManagedConcurrentMap(SOFT_BUNDLE);
-            ManagedConcurrentMap ores = PROPNAME_TO_MAP.putIfAbsent(name, res);
+            res = new ManagedIdentityConcurrentMap(SOFT_BUNDLE);
+            ManagedIdentityConcurrentMap ores = PROPNAME_TO_MAP.putIfAbsent(name, res);
             if (ores != null)
               return ores;
         }
@@ -176,7 +176,7 @@ public class ThreadManagedMetaBeanProperty extends MetaBeanProperty {
            * @see groovy.lang.MetaMethod#invoke(java.lang.Object, java.lang.Object[])
            */
         public Object invoke(Object object, Object[] arguments) {
-            return instance2Prop.getOrPut(object, getInitialValue()).getValue();
+            return instance2Prop.getOrPut(object, getInitialValue());
         }
     }
 
diff --git a/src/main/java/org/codehaus/groovy/util/AbstractConcurrentMap.java b/src/main/java/org/codehaus/groovy/util/AbstractConcurrentMap.java
index 6168877..d67fd88 100644
--- a/src/main/java/org/codehaus/groovy/util/AbstractConcurrentMap.java
+++ b/src/main/java/org/codehaus/groovy/util/AbstractConcurrentMap.java
@@ -18,6 +18,7 @@
  */
 package org.codehaus.groovy.util;
 
+@Deprecated
 public abstract class AbstractConcurrentMap<K, V> extends AbstractConcurrentMapBase {
 
     public AbstractConcurrentMap(Object segmentInfo) {
diff --git a/src/main/java/org/codehaus/groovy/util/AbstractConcurrentMapBase.java b/src/main/java/org/codehaus/groovy/util/AbstractConcurrentMapBase.java
index 01f84af..57ac68e 100644
--- a/src/main/java/org/codehaus/groovy/util/AbstractConcurrentMapBase.java
+++ b/src/main/java/org/codehaus/groovy/util/AbstractConcurrentMapBase.java
@@ -21,6 +21,7 @@ package org.codehaus.groovy.util;
 import java.util.Collection;
 import java.util.LinkedList;
 
+@Deprecated
 public abstract class AbstractConcurrentMapBase {
     protected static final int MAXIMUM_CAPACITY = 1 << 30;
     static final int MAX_SEGMENTS = 1 << 16;
diff --git a/src/main/java/org/codehaus/groovy/util/ManagedConcurrentMap.java b/src/main/java/org/codehaus/groovy/util/ManagedConcurrentMap.java
index d1cf760..7329711 100644
--- a/src/main/java/org/codehaus/groovy/util/ManagedConcurrentMap.java
+++ b/src/main/java/org/codehaus/groovy/util/ManagedConcurrentMap.java
@@ -18,6 +18,7 @@
  */
 package org.codehaus.groovy.util;
 
+@Deprecated
 public class ManagedConcurrentMap<K,V> extends AbstractConcurrentMap<K,V> {
     protected ReferenceBundle bundle;
     public ManagedConcurrentMap(ReferenceBundle bundle) {
diff --git a/src/main/java/org/codehaus/groovy/util/ManagedIdentityConcurrentMap.java b/src/main/java/org/codehaus/groovy/util/ManagedIdentityConcurrentMap.java
new file mode 100644
index 0000000..2736244
--- /dev/null
+++ b/src/main/java/org/codehaus/groovy/util/ManagedIdentityConcurrentMap.java
@@ -0,0 +1,149 @@
+/*
+ *  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.util;
+
+import java.util.Collection;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * This is a basic implementation of a map able to forget its keys. This
+ * map uses internally a ConcurrentHashMap, thus should be safe for concurrency.
+ * hashcode and equals are used to find the entries and should thus be implemented
+ * properly for the keys. This map does not support null keys.
+ *
+ * @param <K> the key type
+ * @param <V> the value type
+ * @since 4.0.0
+ */
+public class ManagedIdentityConcurrentMap<K, V> {
+    private final ConcurrentHashMap<ManagedReference<K>, V> internalMap;
+    private final ReferenceBundle bundle;
+
+    public ManagedIdentityConcurrentMap(ReferenceBundle bundle) {
+        this.internalMap = new ConcurrentHashMap<>();
+        this.bundle = bundle;
+    }
+
+    /**
+     * Returns the value stored for the given key at the point of call.
+     *
+     * @param key a non null key
+     * @return the value stored in the map for the given key
+     */
+    public V get(K key) {
+        return internalMap.get(new ManagedIdentityKey<K>(key));
+    }
+
+    /**
+     * Sets a new value for a given key. an older value is overwritten.
+     *
+     * @param key   a non null key
+     * @param value the new value
+     */
+    public V put(final K key, V value) {
+        return internalMap.put(new ManagedIdentityKey<K>(key), value);
+    }
+
+    /**
+     * Returns the values of the map
+     */
+    public Collection<V> values() {
+        return internalMap.values();
+    }
+
+    /**
+     * Remove the key specified entry
+     *
+     * @param key the key to look up
+     * @return the removed value
+     */
+    public V remove(K key) {
+        return internalMap.remove(new ManagedIdentityKey<K>(key));
+    }
+
+    /**
+     * Get the key specified value, or put the default value and return it if the key is absent
+     *
+     * @param key the key to look up
+     * @param value the default value if the key is absent
+     * @return the value
+     */
+    public V getOrPut(K key, V value) {
+        return internalMap.computeIfAbsent(new ManagedIdentityKey<K>(key), k -> value);
+    }
+
+    /**
+     * Returns the map size
+     */
+    public Object size() {
+        return internalMap.size();
+    }
+
+    /**
+     * Check if the map is empty or not
+     *
+     * @return {@code true} when the map is empty, otherwise {@code false}
+     */
+    public boolean isEmpty() {
+        return internalMap.isEmpty();
+    }
+
+    /**
+     * Represents identity key of {@link ManagedIdentityConcurrentMap}
+     *
+     * @param <K> the key type
+     */
+    private class ManagedIdentityKey<K> extends ManagedReference<K> {
+        private final int hashCode;
+
+        private ManagedIdentityKey(K key) {
+            super(bundle, key);
+            this.hashCode = hash(key);
+        }
+
+        @Override
+        public void finalizeReference() {
+            internalMap.remove(this);
+            super.finalizeReference();
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (!(o instanceof ManagedIdentityKey)) return false;
+            return get() == ((ManagedIdentityKey<?>) o).get();
+        }
+
+        @Override
+        public int hashCode() {
+            return this.hashCode;
+        }
+
+        private int hash(K key) {
+            int h = (null == key) ? 0 : System.identityHashCode(key);
+            h += (h << 15) ^ 0xffffcd7d;
+            h ^= (h >>> 10);
+            h += (h << 3);
+            h ^= (h >>> 6);
+            h += (h << 2) + (h << 14);
+            h ^= (h >>> 16);
+            return h;
+        }
+    }
+}
diff --git a/src/test/org/codehaus/groovy/util/ManagedIdentityConcurrentMapTest.groovy b/src/test/org/codehaus/groovy/util/ManagedIdentityConcurrentMapTest.groovy
new file mode 100644
index 0000000..d454b3f
--- /dev/null
+++ b/src/test/org/codehaus/groovy/util/ManagedIdentityConcurrentMapTest.groovy
@@ -0,0 +1,60 @@
+/*
+ *  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.util
+
+
+import org.junit.Test
+
+class ManagedIdentityConcurrentMapTest {
+    @Test
+    void testRemovingEntriesFromMapAfterGC() {
+        def m = new ManagedIdentityConcurrentMap<Object, String>(ReferenceBundle.getWeakBundle())
+        def k1 = new Object()
+        m.put(k1, "a")
+        def k2 = new Object()
+        m.put(k2, "b")
+        def k3 = new Object()
+        m.put(k3, "c")
+
+        assert 3 == m.size()
+
+        // the related entries should be removed after GC happens
+        k1 = null
+        k2 = null
+        k3 = null
+
+        // finalize via GC, which is hard to predicate though it will happen at last
+        for (int i = 0; i < 20; i++) {
+            System.gc()
+
+            if (m.values().size() == 0) {
+                break
+            }
+
+            Thread.sleep(100)
+        }
+
+        // finalize manually
+        if (!m.isEmpty()) {
+            m.internalMap.keySet().stream().forEach(e -> e.finalizeReference())
+        }
+
+        assert m.isEmpty()
+    }
+}