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 2022/12/04 08:32:14 UTC

[groovy] branch master updated (c73258ed0c -> b180b11557)

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

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


    from c73258ed0c GROOVY-10858: STC: uniform deferred visit of call's functional arguments
     new d037841e09 GROOVY-10772: Possible memory leak, CacheableCallSite retains objects across invocations (thanks to Kyle Moore and Sterling Greene)
     new b180b11557 GROOVY-10772: Possible memory leak, CacheableCallSite retains objects across invocations (additional tweaks)

The 2 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:
 .../groovy/vmplugin/v8/IndyGuardsFiltersAndSignatures.java   | 12 +++++-------
 src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java  |  8 ++++++--
 2 files changed, 11 insertions(+), 9 deletions(-)


[groovy] 01/02: GROOVY-10772: Possible memory leak, CacheableCallSite retains objects across invocations (thanks to Kyle Moore and Sterling Greene)

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

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

commit d037841e092e70f487463e77144f313636e54671
Author: Paul King <pa...@asert.com.au>
AuthorDate: Wed Nov 16 16:29:32 2022 +1000

    GROOVY-10772: Possible memory leak, CacheableCallSite retains objects across invocations (thanks to Kyle Moore and Sterling Greene)
---
 .../org/codehaus/groovy/vmplugin/v8/Selector.java  | 47 +++++++++-------------
 1 file changed, 19 insertions(+), 28 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
index 856df001fa..9dde46177e 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
@@ -932,35 +932,26 @@ public abstract class Selector {
 
             // guards for receiver and parameter
             Class<?>[] pt = handle.type().parameterArray();
-            if (Arrays.stream(args).anyMatch(arg -> null == arg)) {
-                for (int i = 0; i < args.length; i++) {
-                    Object arg = args[i];
-                    Class<?> paramType = pt[i];
-                    MethodHandle test;
-
-                    if (arg == null) {
-                        test = IS_NULL.asType(MethodType.methodType(boolean.class, paramType));
-                        if (LOG_ENABLED) LOG.info("added null argument check at pos " + i);
-                    } else {
-                        if (Modifier.isFinal(paramType.getModifiers())) {
-                            // primitive types are also `final`
-                            continue;
-                        }
-                        test = SAME_CLASS.
-                                bindTo(arg.getClass()).
-                                asType(MethodType.methodType(boolean.class, paramType));
-                        if (LOG_ENABLED) LOG.info("added same class check at pos " + i);
-                    }
-                    Class<?>[] drops = new Class[i];
-                    System.arraycopy(pt, 0, drops, 0, drops.length);
-                    test = MethodHandles.dropArguments(test, 0, drops);
-                    handle = MethodHandles.guardWithTest(test, handle, fallback);
+            for (int i = 0; i < args.length; i++) {
+                Object arg = args[i];
+                Class<?> paramType = pt[i];
+                MethodHandle test;
+
+                if (arg == null) {
+                    test = IS_NULL.asType(MethodType.methodType(boolean.class, paramType));
+                    if (LOG_ENABLED) LOG.info("added null argument check at pos " + i);
+                } else {
+                    Class<?> argClass = arg.getClass();
+                    if (paramType.isPrimitive()) continue;
+                    //if (Modifier.isFinal(argClass.getModifiers()) && TypeHelper.argumentClassIsParameterClass(argClass,pt[i])) continue;
+                    test = SAME_CLASS.
+                            bindTo(argClass).
+                            asType(MethodType.methodType(boolean.class, paramType));
+                    if (LOG_ENABLED) LOG.info("added same class check at pos " + i);
                 }
-            } else if (Arrays.stream(pt).anyMatch(paramType -> !Modifier.isFinal(paramType.getModifiers()))) {
-                // Avoid guards as possible as we could
-                MethodHandle test = SAME_CLASSES.bindTo(args)
-                        .asCollector(Object[].class, pt.length)
-                        .asType(MethodType.methodType(boolean.class, pt));
+                Class<?>[] drops = new Class[i];
+                System.arraycopy(pt, 0, drops, 0, drops.length);
+                test = MethodHandles.dropArguments(test, 0, drops);
                 handle = MethodHandles.guardWithTest(test, handle, fallback);
             }
         }


[groovy] 02/02: GROOVY-10772: Possible memory leak, CacheableCallSite retains objects across invocations (additional tweaks)

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

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

commit b180b1155777c3193e099054a7a3c19265db9f07
Author: Paul King <pa...@asert.com.au>
AuthorDate: Fri Nov 18 16:50:22 2022 +1000

    GROOVY-10772: Possible memory leak, CacheableCallSite retains objects across invocations (additional tweaks)
---
 .../v8/IndyGuardsFiltersAndSignatures.java         | 12 +++--
 .../org/codehaus/groovy/vmplugin/v8/Selector.java  | 51 ++++++++++++++--------
 2 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyGuardsFiltersAndSignatures.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyGuardsFiltersAndSignatures.java
index 2ab2767124..97ed448290 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyGuardsFiltersAndSignatures.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyGuardsFiltersAndSignatures.java
@@ -78,7 +78,7 @@ public class IndyGuardsFiltersAndSignatures {
     static {
         try {
             SAME_CLASS = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "sameClass", MethodType.methodType(boolean.class, Class.class, Object.class));
-            SAME_CLASSES = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "sameClasses", MethodType.methodType(boolean.class, Object[].class, Object[].class));
+            SAME_CLASSES = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "sameClasses", MethodType.methodType(boolean.class, Class[].class, Object[].class));
             SAME_MC = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "isSameMetaClass", MethodType.methodType(boolean.class, MetaClass.class, Object.class));
             IS_NULL = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "isNull", OBJECT_GUARD);
             UNWRAP_METHOD = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "unwrap", OBJECT_FILTER);
@@ -200,17 +200,15 @@ public class IndyGuardsFiltersAndSignatures {
 
     /**
      * Guard to check if the provided objects have the same
-     * class as the other provided objects.
+     * class as the provided classes.
      */
-    public static boolean sameClasses(Object[] cs, Object[] os) {
+    public static boolean sameClasses(Class<?>[] cs, Object[] os) {
         for (int i = 0; i < cs.length; i++) {
-            Object c = cs[i];
-            if (null == c) continue;
-
+            Class<?> c = cs[i];
             Object o = os[i];
             if (null == o) return false;
 
-            if (o.getClass() != c.getClass())
+            if (o.getClass() != c)
                 return false;
         }
         return true;
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
index 9dde46177e..be9cc77fbf 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
@@ -932,26 +932,39 @@ public abstract class Selector {
 
             // guards for receiver and parameter
             Class<?>[] pt = handle.type().parameterArray();
-            for (int i = 0; i < args.length; i++) {
-                Object arg = args[i];
-                Class<?> paramType = pt[i];
-                MethodHandle test;
-
-                if (arg == null) {
-                    test = IS_NULL.asType(MethodType.methodType(boolean.class, paramType));
-                    if (LOG_ENABLED) LOG.info("added null argument check at pos " + i);
-                } else {
-                    Class<?> argClass = arg.getClass();
-                    if (paramType.isPrimitive()) continue;
-                    //if (Modifier.isFinal(argClass.getModifiers()) && TypeHelper.argumentClassIsParameterClass(argClass,pt[i])) continue;
-                    test = SAME_CLASS.
-                            bindTo(argClass).
-                            asType(MethodType.methodType(boolean.class, paramType));
-                    if (LOG_ENABLED) LOG.info("added same class check at pos " + i);
+            if (Arrays.stream(args).anyMatch(arg -> null == arg)) {
+                for (int i = 0; i < args.length; i++) {
+                    Object arg = args[i];
+                    Class<?> paramType = pt[i];
+                    MethodHandle test;
+
+                    if (arg == null) {
+                        test = IS_NULL.asType(MethodType.methodType(boolean.class, paramType));
+                        if (LOG_ENABLED) LOG.info("added null argument check at pos " + i);
+                    } else {
+                        if (Modifier.isFinal(paramType.getModifiers())) {
+                            // primitive types are also `final`
+                            continue;
+                        }
+                        test = SAME_CLASS.
+                                bindTo(arg.getClass()).
+                                asType(MethodType.methodType(boolean.class, paramType));
+                        if (LOG_ENABLED) LOG.info("added same class check at pos " + i);
+                    }
+                    Class<?>[] drops = new Class[i];
+                    System.arraycopy(pt, 0, drops, 0, drops.length);
+                    test = MethodHandles.dropArguments(test, 0, drops);
+                    handle = MethodHandles.guardWithTest(test, handle, fallback);
+                }
+            } else if (Arrays.stream(pt).anyMatch(paramType -> !Modifier.isFinal(paramType.getModifiers()))) {
+                // Avoid guards as much as possible
+                Class<?>[] argClasses = new Class[args.length];
+                for (int i = 0; i < args.length; i++) {
+                    argClasses[i] = args[i].getClass();
                 }
-                Class<?>[] drops = new Class[i];
-                System.arraycopy(pt, 0, drops, 0, drops.length);
-                test = MethodHandles.dropArguments(test, 0, drops);
+                MethodHandle test = SAME_CLASSES.bindTo(argClasses)
+                        .asCollector(Object[].class, pt.length)
+                        .asType(MethodType.methodType(boolean.class, pt));
                 handle = MethodHandles.guardWithTest(test, handle, fallback);
             }
         }