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/04/20 10:33:32 UTC

[groovy] branch master updated: GROOVY-9515: MethodHandle with spread args should not be cached (#1227)

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


The following commit(s) were added to refs/heads/master by this push:
     new 33eee73  GROOVY-9515: MethodHandle with spread args should not be cached (#1227)
33eee73 is described below

commit 33eee738d60693f6085e3f156778fda1def30367
Author: Daniel.Sun <su...@apache.org>
AuthorDate: Mon Apr 20 18:33:18 2020 +0800

    GROOVY-9515: MethodHandle with spread args should not be cached (#1227)
---
 .../codehaus/groovy/vmplugin/v8/IndyInterface.java | 17 ++++++++-
 .../groovy/vmplugin/v8/MethodHandleWrapper.java    | 12 ++++++
 src/test/groovy/bugs/Groovy9515.groovy             | 43 ++++++++++++++++++++++
 3 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java
index 967cc5f..fca5159 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java
@@ -34,6 +34,7 @@ import java.lang.invoke.SwitchPoint;
 import java.util.Map;
 import java.util.function.BiFunction;
 import java.util.function.Function;
+import java.util.function.Supplier;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
@@ -57,6 +58,7 @@ public class IndyInterface {
             SAFE_NAVIGATION = 1, THIS_CALL = 2,
             GROOVY_OBJECT = 4, IMPLICIT_THIS = 8,
             SPREAD_CALL = 16, UNCACHED_CALL = 32;
+    private static final MethodHandleWrapper NULL_METHOD_HANDLE_WRAPPER = MethodHandleWrapper.getNullMethodHandleWrapper();
 
     /**
      * Enum for easy differentiation between call types
@@ -255,16 +257,29 @@ public class IndyInterface {
      * Get the cached methodhandle. if the related methodhandle is not found in the inline cache, cache and return it.
      */
     public static Object fromCache(MutableCallSite callSite, Class<?> sender, String methodName, int callID, Boolean safeNavigation, Boolean thisCall, Boolean spreadCall, Object dummyReceiver, Object[] arguments) throws Throwable {
+        Supplier<MethodHandleWrapper> fallbackSupplier = () -> fallback(callSite, sender, methodName, callID, safeNavigation, thisCall, spreadCall, dummyReceiver, arguments);
+
         MethodHandleWrapper mhw =
                 doWithCallSite(
                         callSite, arguments,
                         (cs, receiver) ->
                                 cs.getAndPut(
                                         receiver.getClass().getName(),
-                                        c -> fallback(callSite, sender, methodName, callID, safeNavigation, thisCall, spreadCall, dummyReceiver, arguments)
+                                        c -> {
+                                            MethodHandleWrapper fbMhw = fallbackSupplier.get();
+                                            return fbMhw.isCanSetTarget() ? fbMhw : NULL_METHOD_HANDLE_WRAPPER;
+                                        }
                                 )
                 );
 
+        if (NULL_METHOD_HANDLE_WRAPPER == mhw) {
+            final MethodHandleWrapper fbMhw = fallbackSupplier.get();
+            if (fbMhw.isCanSetTarget()) {
+                doWithCallSite(callSite, arguments, (cs, receiver) -> cs.put(receiver.getClass().getName(), fbMhw));
+            }
+            mhw = fbMhw;
+        }
+
         if (mhw.isCanSetTarget() && (callSite.getTarget() != mhw.getTargetMethodHandle()) && (mhw.getLatestHitCount() > INDY_OPTIMIZE_THRESHOLD)) {
             callSite.setTarget(mhw.getTargetMethodHandle());
             if (LOG_ENABLED) LOG.info("call site target set, preparing outside invocation");
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/MethodHandleWrapper.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/MethodHandleWrapper.java
index 8849fe6..b858464 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v8/MethodHandleWrapper.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/MethodHandleWrapper.java
@@ -61,4 +61,16 @@ class MethodHandleWrapper {
     public long getLatestHitCount() {
         return latestHitCount.get();
     }
+
+    public static MethodHandleWrapper getNullMethodHandleWrapper() {
+        return NullMethodHandleWrapper.INSTANCE;
+    }
+
+    private static class NullMethodHandleWrapper extends MethodHandleWrapper {
+        public static final NullMethodHandleWrapper INSTANCE = new NullMethodHandleWrapper(null, null, false);
+
+        private NullMethodHandleWrapper(MethodHandle cachedMethodHandle, MethodHandle targetMethodHandle, boolean canSetTarget) {
+            super(cachedMethodHandle, targetMethodHandle, canSetTarget);
+        }
+    }
 }
diff --git a/src/test/groovy/bugs/Groovy9515.groovy b/src/test/groovy/bugs/Groovy9515.groovy
new file mode 100644
index 0000000..d24da08
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy9515.groovy
@@ -0,0 +1,43 @@
+/*
+ *  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 groovy.bugs
+
+import groovy.transform.CompileStatic
+import org.codehaus.groovy.control.CompilerConfiguration
+import org.junit.Test
+
+@CompileStatic
+final class Groovy9515 {
+
+    @Test
+    void testSpreadArgsIndy() {
+        def config = new CompilerConfiguration()
+        config.optimizationOptions.indy = true
+        new GroovyShell(config).evaluate '''
+def x(int a) {a}
+def x(int a, int b) {a + b}
+def y(p) {
+    x(*p)
+}
+
+assert 1 == y([1])
+assert 3 == y([1, 2])
+'''
+    }
+}