You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2022/05/03 16:33:25 UTC
[groovy] branch GROOVY_4_0_X updated: GROOVY-10535, GROOVY-10596: indy: cache [Bb]oolean cast for `null`, etc.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_4_0_X by this push:
new 909ae810ce GROOVY-10535, GROOVY-10596: indy: cache [Bb]oolean cast for `null`, etc.
909ae810ce is described below
commit 909ae810ce2312a3a161fbc121dc7a7b1aade5f0
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue May 3 09:19:27 2022 -0500
GROOVY-10535, GROOVY-10596: indy: cache [Bb]oolean cast for `null`, etc.
---
.../v8/IndyGuardsFiltersAndSignatures.java | 55 ++++++---------
.../org/codehaus/groovy/vmplugin/v8/Selector.java | 79 ++++++++++------------
src/test/groovy/bugs/Groovy10535.groovy | 45 ++++++++++++
3 files changed, 102 insertions(+), 77 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 ffdcc62d46..c2937943b7 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyGuardsFiltersAndSignatures.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyGuardsFiltersAndSignatures.java
@@ -54,61 +54,50 @@ import static org.codehaus.groovy.vmplugin.v8.IndyInterface.LOOKUP;
public class IndyGuardsFiltersAndSignatures {
private static final MethodType
- ZERO_GUARD = MethodType.methodType(boolean.class),
+ OBJECT_FILTER = MethodType.methodType(Object.class, Object.class),
OBJECT_GUARD = MethodType.methodType(boolean.class, Object.class),
- CLASS1_GUARD = MethodType.methodType(boolean.class, Class.class, Object.class),
- METACLASS1_GUARD = MethodType.methodType(boolean.class, MetaClass.class, Object.class),
-
- GRE_GUARD = MethodType.methodType(Object.class, GroovyRuntimeException.class),
-
- OBJECT_FILTER = MethodType.methodType(Object.class, Object.class),
-
- BOUND_INVOKER = MethodType.methodType(Object.class, Object[].class),
- ANO_INVOKER = MethodType.methodType(Object.class, Object.class, Object[].class),
- INVOKER = MethodType.methodType(Object.class, Object.class, String.class, Object[].class),
- GET_INVOKER = MethodType.methodType(Object.class, String.class);
+ INVOKER = MethodType.methodType(Object.class, Object.class, String.class, Object[].class);
protected static final MethodHandle
- SAME_CLASS, UNWRAP_METHOD,
- SAME_MC, IS_NULL,
- UNWRAP_EXCEPTION, META_METHOD_INVOKER,
- GROOVY_OBJECT_INVOKER, GROOVY_OBJECT_GET_PROPERTY,
+ SAME_CLASS, SAME_MC, IS_NULL,
+ UNWRAP_METHOD, UNWRAP_EXCEPTION,
HAS_CATEGORY_IN_CURRENT_THREAD_GUARD,
+ META_METHOD_INVOKER, GROOVY_OBJECT_INVOKER, GROOVY_OBJECT_GET_PROPERTY,
+ META_CLASS_INVOKE_STATIC_METHOD,
BEAN_CONSTRUCTOR_PROPERTY_SETTER,
META_PROPERTY_GETTER,
- SLOW_META_CLASS_FIND, META_CLASS_INVOKE_STATIC_METHOD,
+ SLOW_META_CLASS_FIND,
MOP_GET, MOP_INVOKE_CONSTRUCTOR, MOP_INVOKE_METHOD,
- INTERCEPTABLE_INVOKER,
- CLASS_FOR_NAME, BOOLEAN_IDENTITY,
+ INTERCEPTABLE_INVOKER, INVOKE_METHOD,
+ BOOLEAN_IDENTITY, CLASS_FOR_NAME,
DTT_CAST_TO_TYPE, SAM_CONVERSION,
- HASHSET_CONSTRUCTOR, ARRAYLIST_CONSTRUCTOR, GROOVY_CAST_EXCEPTION,
+ HASHSET_CONSTRUCTOR, ARRAYLIST_CONSTRUCTOR,
+ GROOVY_CAST_EXCEPTION,
EQUALS;
static {
try {
- SAME_CLASS = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "sameClass", CLASS1_GUARD);
- UNWRAP_METHOD = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "unwrap", OBJECT_FILTER);
- SAME_MC = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "isSameMetaClass", METACLASS1_GUARD);
+ SAME_CLASS = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "sameClass", 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_EXCEPTION = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "unwrap", GRE_GUARD);
+ UNWRAP_METHOD = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "unwrap", OBJECT_FILTER);
+ UNWRAP_EXCEPTION = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "unwrap", MethodType.methodType(Object.class, GroovyRuntimeException.class));
+ HAS_CATEGORY_IN_CURRENT_THREAD_GUARD = LOOKUP.findStatic(GroovyCategorySupport.class, "hasCategoryInCurrentThread", MethodType.methodType(boolean.class));
+ META_METHOD_INVOKER = LOOKUP.findVirtual(MetaMethod.class, "doMethodInvoke", MethodType.methodType(Object.class, Object.class, Object[].class));
GROOVY_OBJECT_INVOKER = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "invokeGroovyObjectInvoker", INVOKER.insertParameterTypes(0, MissingMethodException.class));
-
- META_METHOD_INVOKER = LOOKUP.findVirtual(MetaMethod.class, "doMethodInvoke", ANO_INVOKER);
- HAS_CATEGORY_IN_CURRENT_THREAD_GUARD = LOOKUP.findStatic(GroovyCategorySupport.class, "hasCategoryInCurrentThread", ZERO_GUARD);
- GROOVY_OBJECT_GET_PROPERTY = LOOKUP.findVirtual(GroovyObject.class, "getProperty", GET_INVOKER);
+ GROOVY_OBJECT_GET_PROPERTY = LOOKUP.findVirtual(GroovyObject.class, "getProperty", MethodType.methodType(Object.class, String.class));
META_CLASS_INVOKE_STATIC_METHOD = LOOKUP.findVirtual(MetaObjectProtocol.class, "invokeStaticMethod", INVOKER);
-
BEAN_CONSTRUCTOR_PROPERTY_SETTER = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "setBeanProperties", MethodType.methodType(Object.class, MetaClass.class, Object.class, Map.class));
META_PROPERTY_GETTER = LOOKUP.findVirtual(MetaProperty.class, "getProperty", OBJECT_FILTER);
+ SLOW_META_CLASS_FIND = LOOKUP.findStatic(InvokerHelper.class, "getMetaClass", MethodType.methodType(MetaClass.class, Object.class));
MOP_GET = LOOKUP.findVirtual(MetaObjectProtocol.class, "getProperty", MethodType.methodType(Object.class, Object.class, String.class));
- MOP_INVOKE_CONSTRUCTOR = LOOKUP.findVirtual(MetaObjectProtocol.class, "invokeConstructor", BOUND_INVOKER);
+ MOP_INVOKE_CONSTRUCTOR = LOOKUP.findVirtual(MetaObjectProtocol.class, "invokeConstructor", MethodType.methodType(Object.class, Object[].class));
MOP_INVOKE_METHOD = LOOKUP.findVirtual(MetaObjectProtocol.class, "invokeMethod", INVOKER);
- SLOW_META_CLASS_FIND = LOOKUP.findStatic(InvokerHelper.class, "getMetaClass", MethodType.methodType(MetaClass.class, Object.class));
INTERCEPTABLE_INVOKER = LOOKUP.findVirtual(GroovyObject.class, "invokeMethod", MethodType.methodType(Object.class, String.class, Object.class));
-
- CLASS_FOR_NAME = LOOKUP.findStatic(Class.class, "forName", MethodType.methodType(Class.class, String.class, boolean.class, ClassLoader.class));
+ INVOKE_METHOD = LOOKUP.findStatic(InvokerHelper.class, "invokeMethod", MethodType.methodType(Object.class, Object.class, String.class, Object.class));
BOOLEAN_IDENTITY = MethodHandles.identity(Boolean.class);
+ CLASS_FOR_NAME = LOOKUP.findStatic(Class.class, "forName", MethodType.methodType(Class.class, String.class, boolean.class, ClassLoader.class));
DTT_CAST_TO_TYPE = LOOKUP.findStatic(DefaultTypeTransformation.class, "castToType", MethodType.methodType(Object.class, Object.class, Class.class));
SAM_CONVERSION = LOOKUP.findStatic(CachedSAMClass.class, "coerceToSAM", MethodType.methodType(Object.class, Closure.class, Method.class, Class.class));
HASHSET_CONSTRUCTOR = LOOKUP.findConstructor(HashSet.class, MethodType.methodType(void.class, Collection.class));
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 c294e94d30..f4899bf77f 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
@@ -68,7 +68,6 @@ import java.util.HashSet;
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.ARRAYLIST_CONSTRUCTOR;
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.BEAN_CONSTRUCTOR_PROPERTY_SETTER;
-import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.BOOLEAN_IDENTITY;
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.CLASS_FOR_NAME;
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.DTT_CAST_TO_TYPE;
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.EQUALS;
@@ -78,6 +77,7 @@ import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.GRO
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.HASHSET_CONSTRUCTOR;
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.HAS_CATEGORY_IN_CURRENT_THREAD_GUARD;
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.INTERCEPTABLE_INVOKER;
+import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.INVOKE_METHOD;
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.IS_NULL;
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.META_CLASS_INVOKE_STATIC_METHOD;
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.META_METHOD_INVOKER;
@@ -162,16 +162,15 @@ public abstract class Selector {
private final Class<?> staticSourceType, staticTargetType;
public CastSelector(MutableCallSite callSite, Object[] arguments) {
- super(callSite, Selector.class, "", CallType.CAST, false, false, false, arguments);
+ super(callSite, Selector.class, "", CallType.CAST, Boolean.FALSE, Boolean.FALSE, Boolean.FALSE, arguments);
this.staticSourceType = callSite.type().parameterType(0);
this.staticTargetType = callSite.type().returnType();
}
@Override
public void setCallSiteTarget() {
- // targetTypes String, Enum and Class are handled
- // by the compiler already
- // Boolean / boolean
+ // target types String, Enum and Class are handled by the compiler
+
handleBoolean();
handleNullWithoutBoolean();
@@ -256,29 +255,24 @@ public abstract class Selector {
private void handleBoolean() {
if (handle != null) return;
+ boolean primitive = (staticTargetType == boolean.class);
+ if (!primitive && staticTargetType != Boolean.class) return;
+ // boolean->boolean, Boolean->boolean, boolean->Boolean are handled by the compiler
+ // which leaves (T)Z and (T)Boolean, where T is the static type but runtime type of T might be Boolean
- // boolean->boolean, Boolean->boolean, boolean->Boolean
- // is handled by compiler
- // that leaves (T)Z and (T)Boolean, where T is the static type
- // but runtime type of T might be Boolean
+ MethodHandle ifNull = IS_NULL.asType(MethodType.methodType(boolean.class, staticSourceType));
- boolean primitive = staticTargetType == boolean.class;
- if (!primitive && staticTargetType != Boolean.class) return;
- if (args[0] == null) {
- if (primitive) {
- handle = MethodHandles.constant(boolean.class, false);
- handle = MethodHandles.dropArguments(handle, 0, staticSourceType);
- } else {
- handle = BOOLEAN_IDENTITY;
- }
- } else if (args[0] instanceof Boolean) {
- // give value through or unbox
- handle = BOOLEAN_IDENTITY;
- } else {
- //call asBoolean
- name = "asBoolean";
- super.setCallSiteTarget();
+ MethodHandle thenZero;
+ if (primitive) { // false
+ thenZero = MethodHandles.dropArguments(MethodHandles.constant(boolean.class, Boolean.FALSE), 0, staticSourceType);
+ } else { // (Boolean)null
+ thenZero = MethodHandles.identity(staticSourceType).asType(MethodType.methodType(Boolean.class, staticSourceType));
}
+
+ MethodHandle elseCallAsBoolean = MethodHandles.insertArguments(INVOKE_METHOD, 1, "asBoolean", null).asType(targetType);
+
+ handle = MethodHandles.guardWithTest(ifNull, thenZero, elseCallAsBoolean);
+ callSite.setTarget(handle); // handle is re-useable for any argument
}
}
@@ -338,7 +332,7 @@ public abstract class Selector {
if (Modifier.isStatic(f.getModifiers())) {
// normally we would do the following
// handle = MethodHandles.dropArguments(handle,0,Class.class);
- // but because there is a bug in invokedynamic in all jdk7 versions
+ // but because there is a bug in invokedynamic in all jdk7 versions
// maybe use Unsafe.ensureClassInitialized
handle = META_PROPERTY_GETTER.bindTo(res);
}
@@ -403,8 +397,8 @@ public abstract class Selector {
*/
@Override
public MetaClass getMetaClass() {
- Object receiver = args[0];
- mc = GroovySystem.getMetaClassRegistry().getMetaClass((Class<?>) receiver);
+ mc = GroovySystem.getMetaClassRegistry().getMetaClass((Class<?>) args[0]);
+ if (LOG_ENABLED) LOG.info("meta class is " + mc);
return mc;
}
@@ -447,7 +441,7 @@ public abstract class Selector {
super.setHandleForMetaMethod();
}
if (beanConstructor) {
- // we have handle that takes no arguments to create the bean,
+ // we have handle that takes no arguments to create the bean,
// we have to use its return value to call #setBeanProperties with it
// and the meta class.
@@ -508,8 +502,8 @@ public abstract class Selector {
*/
private static class MethodSelector extends Selector {
private static final Object[] SINGLE_NULL_ARRAY = {null};
- protected MetaClass mc;
private boolean isCategoryMethod;
+ protected MetaClass mc;
public MethodSelector(MutableCallSite callSite, Class<?> sender, String methodName, CallType callType, Boolean safeNavigation, Boolean thisCall, Boolean spreadCall, Object[] arguments) {
this.callType = callType;
@@ -579,7 +573,7 @@ public abstract class Selector {
this.cache &= !ClassInfo.getClassInfo(receiver.getClass()).hasPerInstanceMetaClasses();
}
mc.initialize();
-
+ if (LOG_ENABLED) LOG.info("meta class is " + mc);
return mc;
}
@@ -648,7 +642,7 @@ public abstract class Selector {
handle = correctClassForNameAndUnReflectOtherwise(m);
if (LOG_ENABLED) LOG.info("successfully unreflected method");
if (isStaticCategoryTypeMethod) {
- handle = MethodHandles.insertArguments(handle, 0, new Object[]{null});
+ handle = MethodHandles.insertArguments(handle, 0, SINGLE_NULL_ARRAY);
handle = MethodHandles.dropArguments(handle, 0, targetType.parameterType(0));
} else if (!isCategoryTypeMethod && isStatic(m)) {
// we drop the receiver, which might be a Class (invocation on Class)
@@ -717,7 +711,7 @@ public abstract class Selector {
if (receiver instanceof GroovyObject) {
// if the meta class call fails we may still want to fall back to call
// GroovyObject#invokeMethod if the receiver is a GroovyObject
- if (LOG_ENABLED) LOG.info("add MissingMethod handler for GrooObject#invokeMethod fallback path");
+ if (LOG_ENABLED) LOG.info("add MissingMethod handler for GroovyObject#invokeMethod fallback path");
handle = MethodHandles.catchException(handle, MissingMethodException.class, GROOVY_OBJECT_INVOKER);
}
}
@@ -775,14 +769,14 @@ public abstract class Selector {
// so we really need to rewrap
handle = handle.asCollector(lastParam, 1);
} else if (params.length > args.length) {
- // we depend on the method selection having done a good
+ // we depend on the method selection having done a good
// job before already, so the only case for this here is, that
// we have no argument for the array, meaning params.length is
// args.length+1. In that case we have to fill in an empty array
handle = MethodHandles.insertArguments(handle, params.length - 1, Array.newInstance(lastParam.getComponentType(), 0));
if (LOG_ENABLED) LOG.info("added empty array for missing vargs part");
} else { //params.length < args.length
- // we depend on the method selection having done a good
+ // we depend on the method selection having done a good
// job before already, so the only case for this here is, that
// all trailing arguments belong into the vargs array
handle = handle.asCollector(
@@ -815,8 +809,8 @@ public abstract class Selector {
// the argument is an instance of the parameter type. We also
// exclude boxing, since the MethodHandles will do that part
// already for us. Another case is the conversion of a primitive
- // to another primitive or of the wrappers, or a combination of
- // these. This is also handled already. What is left is the
+ // to another primitive or of the wrappers, or a combination of
+ // these. This is also handled already. What is left is the
// GString conversion and the number conversions.
if (arg == null) continue;
@@ -830,7 +824,7 @@ public abstract class Selector {
if (wrappedPara == TypeHelper.getWrapperClass(got)) continue;
// equal in terms of an assignment in Java. That means according to Java widening rules, or
- // a subclass, interface, superclass relation, this case then handles also
+ // a subclass, interface, superclass relation, this case then handles also
// primitive to primitive conversion. Those case are also solved by explicitCastArguments.
if (parameters[i].isAssignableFrom(got)) continue;
@@ -863,7 +857,7 @@ public abstract class Selector {
*/
public void addExceptionHandler() {
//TODO: if we would know exactly which paths require the exceptions
- // and which paths not, we can sometimes save this guard
+ // and which paths not, we can sometimes save this guard
if (handle == null || !catchException) return;
Class<?> returnType = handle.type().returnType();
if (returnType != Object.class) {
@@ -973,8 +967,6 @@ public abstract class Selector {
public void setSelectionBase() {
if (thisCall) {
selectionBase = sender;
- } else if (args[0] == null) {
- selectionBase = NullObject.class;
} else {
selectionBase = mc.getTheClass();
}
@@ -985,8 +977,8 @@ public abstract class Selector {
* Sets a handle to call {@link GroovyInterceptable#invokeMethod(String, Object)}
*/
public boolean setInterceptor() {
- if (!(this.args[0] instanceof GroovyInterceptable)) return false;
- handle = MethodHandles.insertArguments(INTERCEPTABLE_INVOKER, 1, this.name);
+ if (!(args[0] instanceof GroovyInterceptable)) return false;
+ handle = MethodHandles.insertArguments(INTERCEPTABLE_INVOKER, 1, name);
handle = handle.asCollector(Object[].class, targetType.parameterCount() - 1);
handle = handle.asType(targetType);
return true;
@@ -1004,7 +996,6 @@ public abstract class Selector {
public void setCallSiteTarget() {
if (!setNullForSafeNavigation() && !setInterceptor()) {
getMetaClass();
- if (LOG_ENABLED) LOG.info("meta class is " + mc);
setSelectionBase();
MetaClassImpl mci = getMetaClassImpl(mc, callType != CallType.GET);
chooseMeta(mci);
diff --git a/src/test/groovy/bugs/Groovy10535.groovy b/src/test/groovy/bugs/Groovy10535.groovy
new file mode 100644
index 0000000000..e4afabd0fc
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy10535.groovy
@@ -0,0 +1,45 @@
+/*
+ * 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 org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+final class Groovy10535 {
+ @Test
+ void testBooleanTypecast_invokeDynamicOptimization() {
+ assertScript '''
+ @groovy.transform.CompileStatic
+ class C {
+ static main(args) {
+ Collection<String> strings = null
+ for (int i = 0; i <= 200_000; i += 1) { // vs groovy.indy.optimize.threshold
+ assert test(strings) === null
+ }
+ strings = ['x']
+ assert test(strings) !== null
+ }
+ static test(Collection<String> values) {
+ if (values) return 'thing'
+ }
+ }
+ '''
+ }
+}