groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pa...@apache.org
Subject [groovy] branch master updated: GROOVY-8298: Slow Performance Caused by Invoke Dynamic (closes #1135)
Date Sat, 11 Jan 2020 01:59:33 GMT
This is an automated email from the ASF dual-hosted git repository.

paulk 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 c80df23  GROOVY-8298: Slow Performance Caused by Invoke Dynamic (closes #1135)
c80df23 is described below

commit c80df236c7624df51670ff5acdc5edd2be8388b0
Author: Daniel Sun <sunlan@apache.org>
AuthorDate: Sat Jan 11 01:16:08 2020 +0800

    GROOVY-8298: Slow Performance Caused by Invoke Dynamic (closes #1135)
---
 .../java/org/apache/groovy/util/SystemUtil.java    |  19 +++-
 .../groovy/vmplugin/v7/CacheableCallSite.java      |  92 +++++++++++++++
 .../groovy/vmplugin/v7/IndyArrayAccess.java        |  20 ++--
 .../v7/IndyGuardsFiltersAndSignatures.java         |   2 +-
 .../codehaus/groovy/vmplugin/v7/IndyInterface.java | 126 ++++++++++++++++++---
 .../org/codehaus/groovy/vmplugin/v7/IndyMath.java  |  19 ++--
 .../groovy/vmplugin/v7/MethodHandleWrapper.java    |  64 +++++++++++
 .../org/codehaus/groovy/vmplugin/v7/Selector.java  |  59 +++++-----
 .../codehaus/groovy/vmplugin/v7/TypeHelper.java    |  16 +--
 .../groovy/vmplugin/v7/TypeTransformers.java       |   8 +-
 src/test/groovy/bugs/Groovy8298.groovy             |  44 +++++++
 11 files changed, 394 insertions(+), 75 deletions(-)

diff --git a/src/main/java/org/apache/groovy/util/SystemUtil.java b/src/main/java/org/apache/groovy/util/SystemUtil.java
index 5eb398e..57cd91a 100644
--- a/src/main/java/org/apache/groovy/util/SystemUtil.java
+++ b/src/main/java/org/apache/groovy/util/SystemUtil.java
@@ -125,7 +125,7 @@ public class SystemUtil {
      *
      * @param name the name of the system property.
      * @param def the default value
-     * @return value of the Integer system property or false
+     * @return value of the Integer system property or the default value
      */
     public static Integer getIntegerSafe(String name, Integer def) {
         try {
@@ -136,4 +136,21 @@ public class SystemUtil {
 
         return def;
     }
+
+    /**
+     * Retrieves an Long System property
+     *
+     * @param name the name of the system property.
+     * @param def the default value
+     * @return value of the Long system property or the default value
+     */
+    public static Long getLongSafe(String name, Long def) {
+        try {
+            return Long.getLong(name, def);
+        } catch (SecurityException ignore) {
+            // suppress exception
+        }
+
+        return def;
+    }
 }
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v7/CacheableCallSite.java b/src/main/java/org/codehaus/groovy/vmplugin/v7/CacheableCallSite.java
new file mode 100644
index 0000000..af96a07
--- /dev/null
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v7/CacheableCallSite.java
@@ -0,0 +1,92 @@
+/*
+ *  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.vmplugin.v7;
+
+import org.apache.groovy.util.SystemUtil;
+import org.codehaus.groovy.GroovyBugError;
+import org.codehaus.groovy.runtime.memoize.LRUCache;
+import org.codehaus.groovy.runtime.memoize.MemoizeCache;
+
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodType;
+import java.lang.invoke.MutableCallSite;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
+
+/**
+ * Represents a cacheable call site, which can reduce the cost of resolving methods
+ *
+ * @since 3.0.0
+ */
+public class CacheableCallSite extends MutableCallSite {
+    private static final int CACHE_SIZE = SystemUtil.getIntegerSafe("groovy.indy.callsite.cache.size",
16);
+    private final MemoizeCache<String, MethodHandleWrapper> cache = new LRUCache<>(CACHE_SIZE);
+    private volatile MethodHandleWrapper latestHitMethodHandleWrapper = null;
+    private final AtomicLong fallbackCount = new AtomicLong(0);
+    private MethodHandle defaultTarget;
+    private MethodHandle fallbackTarget;
+
+    public CacheableCallSite(MethodType type) {
+        super(type);
+    }
+
+    public MethodHandleWrapper getAndPut(String className, MemoizeCache.ValueProvider<?
super String, ? extends MethodHandleWrapper> valueProvider) {
+        final MethodHandleWrapper result = cache.getAndPut(className, valueProvider);
+        final MethodHandleWrapper lhmh = latestHitMethodHandleWrapper;
+
+        if (lhmh == result) {
+            result.incrementLatestHitCount();
+        } else {
+            result.resetLatestHitCount();
+            if (null != lhmh) lhmh.resetLatestHitCount();
+
+            latestHitMethodHandleWrapper = result;
+        }
+
+        return result;
+    }
+
+    public MethodHandleWrapper put(String name, MethodHandleWrapper mhw) {
+        return cache.put(name, mhw);
+    }
+
+    public long incrementFallbackCount() {
+        return fallbackCount.incrementAndGet();
+    }
+
+    public void resetFallbackCount() {
+        fallbackCount.set(0);
+    }
+
+    public MethodHandle getDefaultTarget() {
+        return defaultTarget;
+    }
+
+    public void setDefaultTarget(MethodHandle defaultTarget) {
+        this.defaultTarget = defaultTarget;
+    }
+
+    public MethodHandle getFallbackTarget() {
+        return fallbackTarget;
+    }
+
+    public void setFallbackTarget(MethodHandle fallbackTarget) {
+        this.fallbackTarget = fallbackTarget;
+    }
+}
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v7/IndyArrayAccess.java b/src/main/java/org/codehaus/groovy/vmplugin/v7/IndyArrayAccess.java
index 84310a7..f79bc28 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v7/IndyArrayAccess.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v7/IndyArrayAccess.java
@@ -44,26 +44,26 @@ public class IndyArrayAccess {
         }
     }
 
-    private static final HashMap<Class, MethodHandle> getterMap, setterMap;
+    private static final HashMap<Class<?>, MethodHandle> getterMap, setterMap;
 
     static {
-        getterMap = new HashMap<Class, MethodHandle>();
-        Class[] classes = new Class[]{
+        getterMap = new HashMap<>();
+        Class<?>[] classes = new Class<?>[]{
                 int[].class, byte[].class, short[].class, long[].class,
                 double[].class, float[].class,
                 boolean[].class, char[].class, Object[].class};
-        for (Class arrayClass : classes) {
+        for (Class<?> arrayClass : classes) {
             MethodHandle handle = buildGetter(arrayClass);
             getterMap.put(arrayClass, handle);
         }
-        setterMap = new HashMap<Class, MethodHandle>();
-        for (Class arrayClass : classes) {
+        setterMap = new HashMap<>();
+        for (Class<?> arrayClass : classes) {
             MethodHandle handle = buildSetter(arrayClass);
             setterMap.put(arrayClass, handle);
         }
     }
 
-    private static MethodHandle buildGetter(Class arrayClass) {
+    private static MethodHandle buildGetter(Class<?> arrayClass) {
         MethodHandle get = MethodHandles.arrayElementGetter(arrayClass);
         MethodHandle fallback = MethodHandles.explicitCastArguments(get, get.type().changeParameterType(0,
Object.class));
 
@@ -81,7 +81,7 @@ public class IndyArrayAccess {
         return handle;
     }
 
-    private static MethodHandle buildSetter(Class arrayClass) {
+    private static MethodHandle buildSetter(Class<?> arrayClass) {
         MethodHandle set = MethodHandles.arrayElementSetter(arrayClass);
         MethodHandle fallback = MethodHandles.explicitCastArguments(set, set.type().changeParameterType(0,
Object.class));
 
@@ -122,7 +122,7 @@ public class IndyArrayAccess {
     }
 
     public static MethodHandle arrayGet(MethodType type) {
-        Class key = type.parameterType(0);
+        Class<?> key = type.parameterType(0);
         MethodHandle res = getterMap.get(key);
         if (res != null) return res;
         res = buildGetter(key);
@@ -131,7 +131,7 @@ public class IndyArrayAccess {
     }
 
     public static MethodHandle arraySet(MethodType type) {
-        Class key = type.parameterType(0);
+        Class<?> key = type.parameterType(0);
         MethodHandle res = setterMap.get(key);
         if (res != null) return res;
         res = buildSetter(key);
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v7/IndyGuardsFiltersAndSignatures.java
b/src/main/java/org/codehaus/groovy/vmplugin/v7/IndyGuardsFiltersAndSignatures.java
index 7bca41a..1fd4942 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v7/IndyGuardsFiltersAndSignatures.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v7/IndyGuardsFiltersAndSignatures.java
@@ -204,7 +204,7 @@ public class IndyGuardsFiltersAndSignatures {
      * class as the provided Class. This method will
      * return false if the Object is null.
      */
-    public static boolean sameClass(Class c, Object o) {
+    public static boolean sameClass(Class<?> c, Object o) {
         if (o == null) return false;
         return o.getClass() == c;
     }
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v7/IndyInterface.java b/src/main/java/org/codehaus/groovy/vmplugin/v7/IndyInterface.java
index 1ba019b..0b42ef6 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v7/IndyInterface.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v7/IndyInterface.java
@@ -19,7 +19,9 @@
 package org.codehaus.groovy.vmplugin.v7;
 
 import groovy.lang.GroovySystem;
+import org.apache.groovy.util.SystemUtil;
 import org.codehaus.groovy.GroovyBugError;
+import org.codehaus.groovy.runtime.NullObject;
 
 import java.lang.invoke.CallSite;
 import java.lang.invoke.ConstantCallSite;
@@ -30,6 +32,7 @@ import java.lang.invoke.MethodType;
 import java.lang.invoke.MutableCallSite;
 import java.lang.invoke.SwitchPoint;
 import java.util.Map;
+import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -44,6 +47,7 @@ import java.util.stream.Stream;
  * methods and classes.
  */
 public class IndyInterface {
+    private static final long INDY_OPTIMIZE_THRESHOLD = SystemUtil.getLongSafe("groovy.indy.optimize.threshold",
100_000L);
 
     /**
      * flags for method and property calls
@@ -132,14 +136,28 @@ public class IndyInterface {
      * LOOKUP constant used for for example unreflect calls
      */
     public static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup();
+
+    /**
+     * handle for the fromCache method
+     */
+    private static final MethodHandle FROM_CACHE_METHOD;
+
     /**
      * handle for the selectMethod method
      */
     private static final MethodHandle SELECT_METHOD;
 
     static {
-        MethodType mt = MethodType.methodType(Object.class, MutableCallSite.class, Class.class,
String.class, int.class, Boolean.class, Boolean.class, Boolean.class, Object.class, Object[].class);
+
+        try {
+            MethodType mt = MethodType.methodType(Object.class, MutableCallSite.class, Class.class,
String.class, int.class, Boolean.class, Boolean.class, Boolean.class, Object.class, Object[].class);
+            FROM_CACHE_METHOD = LOOKUP.findStatic(IndyInterface.class, "fromCache", mt);
+        } catch (Exception e) {
+            throw new GroovyBugError(e);
+        }
+
         try {
+            MethodType mt = MethodType.methodType(Object.class, MutableCallSite.class, Class.class,
String.class, int.class, Boolean.class, Boolean.class, Boolean.class, Object.class, Object[].class);
             SELECT_METHOD = LOOKUP.findStatic(IndyInterface.class, "selectMethod", mt);
         } catch (Exception e) {
             throw new GroovyBugError(e);
@@ -184,14 +202,13 @@ public class IndyInterface {
      * @since Groovy 2.1.0
      */
     public static CallSite bootstrap(Lookup caller, String callType, MethodType type, String
name, int flags) {
-        boolean safe = (flags & SAFE_NAVIGATION) != 0;
-        boolean thisCall = (flags & THIS_CALL) != 0;
-        boolean spreadCall = (flags & SPREAD_CALL) != 0;
-
         CallType ct = CallType.fromCallSiteName(callType);
         if (null == ct) throw new GroovyBugError("Unknown call type: " + callType);
 
         int callID = ct.ordinal();
+        boolean safe = (flags & SAFE_NAVIGATION) != 0;
+        boolean thisCall = (flags & THIS_CALL) != 0;
+        boolean spreadCall = (flags & SPREAD_CALL) != 0;
 
         return realBootstrap(caller, name, callID, type, safe, thisCall, spreadCall);
     }
@@ -201,12 +218,16 @@ public class IndyInterface {
      */
     private static CallSite realBootstrap(Lookup caller, String name, int callID, MethodType
type, boolean safe, boolean thisCall, boolean spreadCall) {
         // since indy does not give us the runtime types
-        // we produce first a dummy call site, which then changes the target to one,
+        // we produce first a dummy call site, which then changes the target to one when
INDY_OPTIMIZE_THRESHOLD is reached,
         // that does the method selection including the direct call to the
         // real method.
-        MutableCallSite mc = new MutableCallSite(type);
-        MethodHandle mh = makeFallBack(mc, caller.lookupClass(), name, callID, type, safe,
thisCall, spreadCall);
+        CacheableCallSite mc = new CacheableCallSite(type);
+        final Class<?> sender = caller.lookupClass();
+        MethodHandle mh = makeAdapter(mc, sender, name, callID, type, safe, thisCall, spreadCall);
         mc.setTarget(mh);
+        mc.setDefaultTarget(mh);
+        mc.setFallbackTarget(makeFallBack(mc, sender, name, callID, type, safe, thisCall,
spreadCall));
+
         return mc;
     }
 
@@ -214,22 +235,95 @@ public class IndyInterface {
      * Makes a fallback method for an invalidated method selection
      */
     protected static MethodHandle makeFallBack(MutableCallSite mc, Class<?> sender,
String name, int callID, MethodType type, boolean safeNavigation, boolean thisCall, boolean
spreadCall) {
-        MethodHandle mh = MethodHandles.insertArguments(SELECT_METHOD, 0, mc, sender, name,
callID, safeNavigation, thisCall, spreadCall, /*dummy receiver:*/ 1);
-        mh = mh.asCollector(Object[].class, type.parameterCount()).
-                asType(type);
-        return mh;
+        return make(mc, sender, name, callID, type, safeNavigation, thisCall, spreadCall,
SELECT_METHOD);
+    }
+
+    /**
+     * Makes an adapter method for method selection, i.e. get the cached methodhandle(fast
path) or fallback
+     */
+    private static MethodHandle makeAdapter(MutableCallSite mc, Class<?> sender, String
name, int callID, MethodType type, boolean safeNavigation, boolean thisCall, boolean spreadCall)
{
+        return make(mc, sender, name, callID, type, safeNavigation, thisCall, spreadCall,
FROM_CACHE_METHOD);
+    }
+
+    private static MethodHandle make(MutableCallSite mc, Class<?> sender, String name,
int callID, MethodType type, boolean safeNavigation, boolean thisCall, boolean spreadCall,
MethodHandle originalMH) {
+        MethodHandle mh = MethodHandles.insertArguments(originalMH, 0, mc, sender, name,
callID, safeNavigation, thisCall, spreadCall, /*dummy receiver:*/ 1);
+        return mh.asCollector(Object[].class, type.parameterCount()).asType(type);
+    }
+
+    /**
+     * 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 {
+        MethodHandleWrapper mhw =
+                doWithCallSite(
+                        callSite, arguments,
+                        (cs, receiver) ->
+                                cs.getAndPut(
+                                        receiver.getClass().getName(),
+                                        c -> fallback(callSite, sender, methodName, callID,
safeNavigation, thisCall, spreadCall, dummyReceiver, arguments)
+                                )
+                );
+
+        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");
+
+            mhw.resetLatestHitCount();
+        }
+
+        return mhw.getCachedMethodHandle().invokeExact(arguments);
     }
 
     /**
      * Core method for indy method selection using runtime types.
      */
-    public static Object selectMethod(MutableCallSite callSite, Class sender, String methodName,
int callID, Boolean safeNavigation, Boolean thisCall, Boolean spreadCall, Object dummyReceiver,
Object[] arguments) throws Throwable {
+    public static Object selectMethod(MutableCallSite callSite, Class<?> sender, String
methodName, int callID, Boolean safeNavigation, Boolean thisCall, Boolean spreadCall, Object
dummyReceiver, Object[] arguments) throws Throwable {
+        final MethodHandleWrapper mhw = fallback(callSite, sender, methodName, callID, safeNavigation,
thisCall, spreadCall, dummyReceiver, arguments);
+
+        if (callSite instanceof CacheableCallSite) {
+            CacheableCallSite cacheableCallSite = (CacheableCallSite) callSite;
+
+            final MethodHandle defaultTarget = cacheableCallSite.getDefaultTarget();
+            final long fallbackCount = cacheableCallSite.incrementFallbackCount();
+            if ((fallbackCount > INDY_OPTIMIZE_THRESHOLD) && (cacheableCallSite.getTarget()
!= defaultTarget)) {
+                cacheableCallSite.setTarget(defaultTarget);
+                if (LOG_ENABLED) LOG.info("call site target reset to default, preparing outside
invocation");
+
+                cacheableCallSite.resetFallbackCount();
+            }
+
+            if (defaultTarget == cacheableCallSite.getTarget()) {
+                // correct the stale methodhandle in the inline cache of callsite
+                // it is important but impacts the performance somehow when cache misses
frequently
+                doWithCallSite(callSite, arguments, (cs, receiver) -> cs.put(receiver.getClass().getName(),
mhw));
+            }
+        }
+
+        return mhw.getCachedMethodHandle().invokeExact(arguments);
+    }
+
+    private static MethodHandleWrapper fallback(MutableCallSite callSite, Class<?>
sender, String methodName, int callID, Boolean safeNavigation, Boolean thisCall, Boolean spreadCall,
Object dummyReceiver, Object[] arguments) {
         Selector selector = Selector.getSelector(callSite, sender, methodName, callID, safeNavigation,
thisCall, spreadCall, arguments);
         selector.setCallSiteTarget();
 
-        MethodHandle call = selector.handle.asSpreader(Object[].class, arguments.length);
-        call = call.asType(MethodType.methodType(Object.class, Object[].class));
-        return call.invokeExact(arguments);
+        return new MethodHandleWrapper(
+                selector.handle.asSpreader(Object[].class, arguments.length).asType(MethodType.methodType(Object.class,
Object[].class)),
+                selector.handle,
+                selector.cache
+        );
+    }
+
+    private static <T> T doWithCallSite(MutableCallSite callSite, Object[] arguments,
BiFunction<? super CacheableCallSite, ? super Object, T> f) {
+        if (callSite instanceof CacheableCallSite) {
+            CacheableCallSite cacheableCallSite = (CacheableCallSite) callSite;
+            Object receiver = arguments[0];
+
+            if (null == receiver) receiver = NullObject.getNullObject();
+
+            return f.apply(cacheableCallSite, receiver);
+        }
+
+        throw new GroovyBugError("CacheableCallSite is expected, but the actual callsite
is: " + callSite);
     }
 
     /**
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v7/IndyMath.java b/src/main/java/org/codehaus/groovy/vmplugin/v7/IndyMath.java
index 8b2dd7b..982f00f 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v7/IndyMath.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v7/IndyMath.java
@@ -61,14 +61,14 @@ public class IndyMath {
             OOV = MethodType.methodType(Void.TYPE, Object.class, Object.class);
 
     private static void makeMapEntry(String method, MethodType[] keys, MethodType[] values)
throws NoSuchMethodException, IllegalAccessException {
-        Map<MethodType, MethodHandle> xMap = new HashMap();
-        methods.put(method, xMap);
+        Map<MethodType, MethodHandle> xMap = new HashMap<>();
+        METHODS.put(method, xMap);
         for (int i = 0; i < keys.length; i++) {
             xMap.put(keys[i], LOOKUP.findStatic(IndyMath.class, method, values[i]));
         }
     }
 
-    private static Map<String, Map<MethodType, MethodHandle>> methods = new HashMap();
+    private static final Map<String, Map<MethodType, MethodHandle>> METHODS =
new HashMap<>();
 
     static {
         try {
@@ -107,7 +107,7 @@ public class IndyMath {
      * more efficient call path.
      */
     public static boolean chooseMathMethod(Selector info, MetaMethod metaMethod) {
-        Map<MethodType, MethodHandle> xmap = methods.get(info.name);
+        Map<MethodType, MethodHandle> xmap = METHODS.get(info.name);
         if (xmap == null) return false;
 
         MethodType type = replaceWithMoreSpecificType(info.args, info.targetType);
@@ -129,9 +129,10 @@ public class IndyMath {
      * parameters in the MethodType will have the same type.
      */
     private static MethodType widenOperators(MethodType mt) {
-        if (mt.parameterCount() == 2) {
-            Class leftType = mt.parameterType(0);
-            Class rightType = mt.parameterType(1);
+        final int parameterCount = mt.parameterCount();
+        if (parameterCount == 2) {
+            Class<?> leftType = mt.parameterType(0);
+            Class<?> rightType = mt.parameterType(1);
 
             if (isIntCategory(leftType) && isIntCategory(rightType)) return IIV;
             if (isLongCategory(leftType) && isLongCategory(rightType)) return LLV;
@@ -139,8 +140,8 @@ public class IndyMath {
             if (isDoubleCategory(leftType) && isDoubleCategory(rightType)) return
DDV;
 
             return OOV;
-        } else if (mt.parameterCount() == 1) {
-            Class leftType = mt.parameterType(0);
+        } else if (parameterCount == 1) {
+            Class<?> leftType = mt.parameterType(0);
             if (isIntCategory(leftType)) return IV;
             if (isLongCategory(leftType)) return LV;
             if (isBigDecCategory(leftType)) return GV;
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v7/MethodHandleWrapper.java b/src/main/java/org/codehaus/groovy/vmplugin/v7/MethodHandleWrapper.java
new file mode 100644
index 0000000..0319e93
--- /dev/null
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v7/MethodHandleWrapper.java
@@ -0,0 +1,64 @@
+/*
+ *  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.vmplugin.v7;
+
+import java.lang.invoke.MethodHandle;
+import java.util.concurrent.atomic.AtomicLong;
+
+/**
+ * Wrap method handles
+ *
+ * @since 3.0.0
+ */
+class MethodHandleWrapper {
+    private final MethodHandle cachedMethodHandle;
+    private final MethodHandle targetMethodHandle;
+    private final boolean canSetTarget;
+    private final AtomicLong latestHitCount = new AtomicLong(0);
+
+    public MethodHandleWrapper(MethodHandle cachedMethodHandle, MethodHandle targetMethodHandle,
boolean canSetTarget) {
+        this.cachedMethodHandle = cachedMethodHandle;
+        this.targetMethodHandle = targetMethodHandle;
+        this.canSetTarget = canSetTarget;
+    }
+
+    public MethodHandle getCachedMethodHandle() {
+        return cachedMethodHandle;
+    }
+
+    public MethodHandle getTargetMethodHandle() {
+        return targetMethodHandle;
+    }
+
+    public boolean isCanSetTarget() {
+        return canSetTarget;
+    }
+
+    public long incrementLatestHitCount() {
+        return latestHitCount.incrementAndGet();
+    }
+
+    public void resetLatestHitCount() {
+        latestHitCount.set(0);
+    }
+
+    public long getLatestHitCount() {
+        return latestHitCount.get();
+    }
+}
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v7/Selector.java b/src/main/java/org/codehaus/groovy/vmplugin/v7/Selector.java
index 42e6893..be01361 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v7/Selector.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v7/Selector.java
@@ -95,7 +95,6 @@ import static org.codehaus.groovy.vmplugin.v7.IndyGuardsFiltersAndSignatures.unw
 import static org.codehaus.groovy.vmplugin.v7.IndyInterface.LOG;
 import static org.codehaus.groovy.vmplugin.v7.IndyInterface.LOG_ENABLED;
 import static org.codehaus.groovy.vmplugin.v7.IndyInterface.LOOKUP;
-import static org.codehaus.groovy.vmplugin.v7.IndyInterface.makeFallBack;
 import static org.codehaus.groovy.vmplugin.v7.IndyInterface.switchPoint;
 
 public abstract class Selector {
@@ -106,12 +105,12 @@ public abstract class Selector {
     public MethodHandle handle;
     public boolean useMetaClass = false, cache = true;
     public MutableCallSite callSite;
-    public Class sender;
+    public Class<?> sender;
     public boolean isVargs;
     public boolean safeNavigation, safeNavigationOrig, spread;
     public boolean skipSpreadCollector;
     public boolean thisCall;
-    public Class selectionBase;
+    public Class<?> selectionBase;
     public boolean catchException = true;
     public CallType callType;
 
@@ -123,7 +122,7 @@ public abstract class Selector {
     /**
      * Returns the Selector
      */
-    public static Selector getSelector(MutableCallSite callSite, Class sender, String methodName,
int callID, boolean safeNavigation, boolean thisCall, boolean spreadCall, Object[] arguments)
{
+    public static Selector getSelector(MutableCallSite callSite, Class<?> sender, String
methodName, int callID, boolean safeNavigation, boolean thisCall, boolean spreadCall, Object[]
arguments) {
         CallType callType = CALL_TYPE_VALUES[callID];
         switch (callType) {
             case INIT:
@@ -221,7 +220,7 @@ public abstract class Selector {
             }
         }
 
-        private static boolean isAbstractClassOf(Class toTest, Class givenOnCallSite) {
+        private static boolean isAbstractClassOf(Class<?> toTest, Class<?> givenOnCallSite)
{
             if (!toTest.isAssignableFrom(givenOnCallSite)) return false;
             if (givenOnCallSite.isInterface()) return true;
             return Modifier.isAbstract(givenOnCallSite.getModifiers());
@@ -286,7 +285,7 @@ public abstract class Selector {
     private static class PropertySelector extends MethodSelector {
         private boolean insertName = false;
 
-        public PropertySelector(MutableCallSite callSite, Class sender, String methodName,
CallType callType, boolean safeNavigation, boolean thisCall, boolean spreadCall, Object[]
arguments) {
+        public PropertySelector(MutableCallSite callSite, Class<?> sender, String methodName,
CallType callType, boolean safeNavigation, boolean thisCall, boolean spreadCall, Object[]
arguments) {
             super(callSite, sender, methodName, callType, safeNavigation, thisCall, spreadCall,
arguments);
         }
 
@@ -305,7 +304,7 @@ public abstract class Selector {
         public void chooseMeta(MetaClassImpl mci) {
             Object receiver = getCorrectedReceiver();
             if (receiver instanceof GroovyObject) {
-                Class aClass = receiver.getClass();
+                Class<?> aClass = receiver.getClass();
                 try {
                     Method reflectionMethod = aClass.getMethod("getProperty", String.class);
                     if (!reflectionMethod.isSynthetic() && !isMarkedInternal(reflectionMethod))
{
@@ -322,7 +321,7 @@ public abstract class Selector {
             }
 
             if (method != null || mci == null) return;
-            Class chosenSender = this.sender;
+            Class<?> chosenSender = this.sender;
             if (mci.getTheClass() != chosenSender && GroovyCategorySupport.hasCategoryInCurrentThread())
{
                 chosenSender = mci.getTheClass();
             }
@@ -387,7 +386,7 @@ public abstract class Selector {
     private static class InitSelector extends MethodSelector {
         private boolean beanConstructor;
 
-        public InitSelector(MutableCallSite callSite, Class sender, String methodName, CallType
callType, boolean safeNavigation, boolean thisCall, boolean spreadCall, Object[] arguments)
{
+        public InitSelector(MutableCallSite callSite, Class<?> sender, String methodName,
CallType callType, boolean safeNavigation, boolean thisCall, boolean spreadCall, Object[]
arguments) {
             super(callSite, sender, methodName, callType, safeNavigation, thisCall, spreadCall,
arguments);
         }
 
@@ -405,7 +404,7 @@ public abstract class Selector {
         @Override
         public MetaClass getMetaClass() {
             Object receiver = args[0];
-            mc = GroovySystem.getMetaClassRegistry().getMetaClass((Class) receiver);
+            mc = GroovySystem.getMetaClassRegistry().getMetaClass((Class<?>) receiver);
             return mc;
         }
 
@@ -512,7 +511,7 @@ public abstract class Selector {
         protected MetaClass mc;
         private boolean isCategoryMethod;
 
-        public MethodSelector(MutableCallSite callSite, Class sender, String methodName,
CallType callType, Boolean safeNavigation, Boolean thisCall, Boolean spreadCall, Object[]
arguments) {
+        public MethodSelector(MutableCallSite callSite, Class<?> sender, String methodName,
CallType callType, Boolean safeNavigation, Boolean thisCall, Boolean spreadCall, Object[]
arguments) {
             this.callType = callType;
             this.targetType = callSite.type();
             this.name = methodName;
@@ -572,7 +571,7 @@ public abstract class Selector {
             } else if (receiver instanceof GroovyObject) {
                 mc = ((GroovyObject) receiver).getMetaClass();
             } else if (receiver instanceof Class) {
-                Class c = (Class) receiver;
+                Class<?> c = (Class<?>) receiver;
                 mc = GroovySystem.getMetaClassRegistry().getMetaClass(c);
                 this.cache &= !ClassInfo.getClassInfo(c).hasPerInstanceMetaClasses();
             } else {
@@ -689,7 +688,7 @@ public abstract class Selector {
          * Helper method to manipulate the given type to replace Wrapper with Object.
          */
         private MethodType removeWrapper(MethodType targetType) {
-            Class[] types = targetType.parameterArray();
+            Class<?>[] types = targetType.parameterArray();
             for (int i = 0; i < types.length; i++) {
                 if (types[i] == Wrapper.class) {
                     targetType = targetType.changeParameterType(i, Object.class);
@@ -735,11 +734,11 @@ public abstract class Selector {
          */
         public void correctWrapping() {
             if (useMetaClass) return;
-            Class[] pt = handle.type().parameterArray();
+            Class<?>[] pt = handle.type().parameterArray();
             if (currentType != null) pt = currentType.parameterArray();
             for (int i = 1; i < args.length; i++) {
                 if (args[i] instanceof Wrapper) {
-                    Class type = pt[i];
+                    Class<?> type = pt[i];
                     MethodType mt = MethodType.methodType(type, Wrapper.class);
                     handle = MethodHandles.filterArguments(handle, i, UNWRAP_METHOD.asType(mt));
                     if (LOG_ENABLED) LOG.info("added filter for Wrapper for argument at pos
" + i);
@@ -755,7 +754,7 @@ public abstract class Selector {
         public void correctParameterLength() {
             if (handle == null) return;
 
-            Class[] params = handle.type().parameterArray();
+            Class<?>[] params = handle.type().parameterArray();
             if (currentType != null) params = currentType.parameterArray();
             if (!isVargs) {
                 if (spread && useMetaClass) return;
@@ -765,7 +764,7 @@ public abstract class Selector {
                 return;
             }
 
-            Class lastParam = params[params.length - 1];
+            Class<?> lastParam = params[params.length - 1];
             Object lastArg = unwrapIfWrapped(args[args.length - 1]);
             if (params.length == args.length) {
                 // may need rewrap
@@ -801,7 +800,7 @@ public abstract class Selector {
         public void correctCoerce() {
             if (useMetaClass) return;
 
-            Class[] parameters = handle.type().parameterArray();
+            Class<?>[] parameters = handle.type().parameterArray();
             if (currentType != null) parameters = currentType.parameterArray();
             if (args.length != parameters.length) {
                 throw new GroovyBugError("At this point argument array length and parameter
array length should be the same");
@@ -821,12 +820,12 @@ public abstract class Selector {
                 // GString conversion and the number conversions.
 
                 if (arg == null) continue;
-                Class got = arg.getClass();
+                Class<?> got = arg.getClass();
 
                 // equal class, nothing to do
                 if (got == parameters[i]) continue;
 
-                Class wrappedPara = TypeHelper.getWrapperClass(parameters[i]);
+                Class<?> wrappedPara = TypeHelper.getWrapperClass(parameters[i]);
                 // equal class with one maybe a primitive, the later explicitCastArguments
will solve this case
                 if (wrappedPara == TypeHelper.getWrapperClass(got)) continue;
 
@@ -866,7 +865,7 @@ public abstract class Selector {
             //TODO: if we would know exactly which paths require the exceptions
             //      and which paths not, we can sometimes save this guard 
             if (handle == null || !catchException) return;
-            Class returnType = handle.type().returnType();
+            Class<?> returnType = handle.type().returnType();
             if (returnType != Object.class) {
                 MethodType mtype = MethodType.methodType(returnType, GroovyRuntimeException.class);
                 handle = MethodHandles.catchException(handle, GroovyRuntimeException.class,
UNWRAP_EXCEPTION.asType(mtype));
@@ -883,7 +882,12 @@ public abstract class Selector {
             if (handle == null) return;
             if (!cache) return;
 
-            MethodHandle fallback = makeFallBack(callSite, sender, name, callType.ordinal(),
targetType, safeNavigationOrig, thisCall, spread);
+            MethodHandle fallback;
+            if (callSite instanceof CacheableCallSite) {
+                fallback = ((CacheableCallSite) callSite).getFallbackTarget();
+            } else {
+                throw new GroovyBugError("CacheableCallSite is expected, but the actual callsite
is: " + callSite);
+            }
 
             // special guards for receiver
             if (receiver instanceof GroovyObject) {
@@ -923,7 +927,7 @@ public abstract class Selector {
             if (LOG_ENABLED) LOG.info("added switch point guard");
 
             // guards for receiver and parameter
-            Class[] pt = handle.type().parameterArray();
+            Class<?>[] pt = handle.type().parameterArray();
             for (int i = 0; i < args.length; i++) {
                 Object arg = args[i];
                 MethodHandle test;
@@ -931,7 +935,7 @@ public abstract class Selector {
                     test = IS_NULL.asType(MethodType.methodType(boolean.class, pt[i]));
                     if (LOG_ENABLED) LOG.info("added null argument check at pos " + i);
                 } else {
-                    Class argClass = arg.getClass();
+                    Class<?> argClass = arg.getClass();
                     if (pt[i].isPrimitive()) continue;
                     //if (Modifier.isFinal(argClass.getModifiers()) && TypeHelper.argumentClassIsParameterClass(argClass,pt[i]))
continue;
                     test = SAME_CLASS.
@@ -939,7 +943,7 @@ public abstract class Selector {
                             asType(MethodType.methodType(boolean.class, pt[i]));
                     if (LOG_ENABLED) LOG.info("added same class check at pos " + i);
                 }
-                Class[] drops = new Class[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);
@@ -950,12 +954,15 @@ public abstract class Selector {
          * do the actual call site target set, if the call is supposed to be cached
          */
         public void doCallSiteTargetSet() {
+            if (LOG_ENABLED) LOG.info("call site stays uncached");
+            /*
             if (!cache) {
                 if (LOG_ENABLED) LOG.info("call site stays uncached");
             } else {
                 callSite.setTarget(handle);
                 if (LOG_ENABLED) LOG.info("call site target set, preparing outside invocation");
             }
+            */
         }
 
         /**
@@ -1054,7 +1061,7 @@ public abstract class Selector {
      * none of these cases matches, this method returns null.
      */
     private static MetaClassImpl getMetaClassImpl(MetaClass mc, boolean includeEMC) {
-        Class mcc = mc.getClass();
+        Class<?> mcc = mc.getClass();
         boolean valid = mcc == MetaClassImpl.class ||
                 mcc == AdaptingMetaClass.class ||
                 mcc == ClosureMetaClass.class ||
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v7/TypeHelper.java b/src/main/java/org/codehaus/groovy/vmplugin/v7/TypeHelper.java
index fcd0738..8591fae 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v7/TypeHelper.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v7/TypeHelper.java
@@ -35,7 +35,7 @@ public class TypeHelper {
      * will be returned. If it is no primitive number type, we return the
      * class itself.
      */
-    protected static Class getWrapperClass(Class c) {
+    protected static Class<?> getWrapperClass(Class<?> c) {
         if (c == Integer.TYPE) {
             c = Integer.class;
         } else if (c == Byte.TYPE) {
@@ -63,7 +63,7 @@ public class TypeHelper {
      * the parameter class. For example the parameter is an int and the
      * argument class is a wrapper.
      */
-    protected static boolean argumentClassIsParameterClass(Class argumentClass, Class parameterClass)
{
+    protected static boolean argumentClassIsParameterClass(Class<?> argumentClass,
Class<?> parameterClass) {
         if (argumentClass == parameterClass) return true;
         return getWrapperClass(parameterClass) == argumentClass;
     }
@@ -78,32 +78,32 @@ public class TypeHelper {
             // if argument null, take the static type
             if (args[i] == null) continue;
             if (callSiteType.parameterType(i).isPrimitive()) continue;
-            Class argClass = args[i].getClass();
+            Class<?> argClass = args[i].getClass();
             callSiteType = callSiteType.changeParameterType(i, argClass);
         }
         return callSiteType;
     }
 
-    protected static boolean isIntCategory(Class x) {
+    protected static boolean isIntCategory(Class<?> x) {
         return x == Integer.class || x == int.class ||
                 x == Byte.class || x == byte.class ||
                 x == Short.class || x == short.class;
     }
 
-    protected static boolean isLongCategory(Class x) {
+    protected static boolean isLongCategory(Class<?> x) {
         return x == Long.class || x == long.class ||
                 isIntCategory(x);
     }
 
-    private static boolean isBigIntCategory(Class x) {
+    private static boolean isBigIntCategory(Class<?> x) {
         return x == BigInteger.class || isLongCategory(x);
     }
 
-    protected static boolean isBigDecCategory(Class x) {
+    protected static boolean isBigDecCategory(Class<?> x) {
         return x == BigDecimal.class || isBigIntCategory(x);
     }
 
-    protected static boolean isDoubleCategory(Class x) {
+    protected static boolean isDoubleCategory(Class<?> x) {
         return x == Float.class || x == float.class ||
                 x == Double.class || x == double.class ||
                 isBigDecCategory(x);
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v7/TypeTransformers.java b/src/main/java/org/codehaus/groovy/vmplugin/v7/TypeTransformers.java
index b945056..d4c92ad 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v7/TypeTransformers.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v7/TypeTransformers.java
@@ -124,7 +124,7 @@ public class TypeTransformers {
      * This method handles transformations to String from GString,
      * array transformations and number based transformations
      */
-    protected static MethodHandle addTransformer(MethodHandle handle, int pos, Object arg,
Class parameter) {
+    protected static MethodHandle addTransformer(MethodHandle handle, int pos, Object arg,
Class<?> parameter) {
         MethodHandle transformer = null;
         if (arg instanceof GString) {
             transformer = TO_STRING;
@@ -144,7 +144,7 @@ public class TypeTransformers {
      * creates a method handle able to transform the given Closure into a SAM type
      * if the given parameter is a SAM type
      */
-    private static MethodHandle createSAMTransform(Object arg, Class parameter) {
+    private static MethodHandle createSAMTransform(Object arg, Class<?> parameter)
{
         Method method = CachedSAMClass.getSAMMethod(parameter);
         if (method == null) return null;
         // TODO: have to think about how to optimize this!
@@ -199,7 +199,7 @@ public class TypeTransformers {
      */
     public static MethodHandle applyUnsharpFilter(MethodHandle handle, int pos, MethodHandle
transformer) {
         MethodType type = transformer.type();
-        Class given = handle.type().parameterType(pos);
+        Class<?> given = handle.type().parameterType(pos);
         if (type.returnType() != given || type.parameterType(0) != given) {
             transformer = transformer.asType(MethodType.methodType(given, type.parameterType(0)));
         }
@@ -210,7 +210,7 @@ public class TypeTransformers {
      * returns a transformer later applied as filter to transform one
      * number into another
      */
-    private static MethodHandle selectNumberTransformer(Class param, Object arg) {
+    private static MethodHandle selectNumberTransformer(Class<?> param, Object arg)
{
         param = TypeHelper.getWrapperClass(param);
 
         if (param == Byte.class) {
diff --git a/src/test/groovy/bugs/Groovy8298.groovy b/src/test/groovy/bugs/Groovy8298.groovy
new file mode 100644
index 0000000..8666d94
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy8298.groovy
@@ -0,0 +1,44 @@
+/*
+ *  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.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+@CompileStatic
+final class Groovy8298 {
+    @Test
+    void testSameReceiverDifferentArguments() {
+        assertScript '''
+            def same(String obj) { return obj }
+            def same(int obj) { return obj }
+            def same(float obj) { return obj }
+            [1, 1.0f, '1.0'].each { assert it == this.same(it) }
+        '''
+    }
+
+    @Test
+    void testDifferentReceivers() {
+        assertScript '''
+            [1, 1.0f, '1.0'].each { assert it.toString().contains('1') }
+        '''
+    }
+}


Mime
View raw message