From 6ca24e13af3ac37a6803a4aeb533750270aecb11 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Fri, 3 Aug 2018 15:22:30 -0700 Subject: [PATCH] Painless: Use LocalMethod Map For Lookup at Runtime (#32599) This modifies Def to use a Map to look up user-defined methods at runtime instead of writing constant methodhandles to do the reverse lookup. This creates a consistency between how LocalMethods are looked up at compile-time and run-time. This consistency will allow this code to be more maintainable moving forward. This will also allow FunctionReference to be cleaned up in a follow up PR. --- .../org/elasticsearch/painless/Compiler.java | 5 ++- .../java/org/elasticsearch/painless/Def.java | 38 +++++++---------- .../elasticsearch/painless/DefBootstrap.java | 23 ++++++---- .../org/elasticsearch/painless/Locals.java | 42 +++++++++---------- .../painless/WriterConstants.java | 2 +- .../lookup/PainlessLookupBuilder.java | 1 - .../painless/node/ECallLocal.java | 4 +- .../painless/node/EFunctionRef.java | 2 +- .../elasticsearch/painless/node/ELambda.java | 3 +- .../painless/node/SFunction.java | 14 ------- .../elasticsearch/painless/node/SSource.java | 12 ++++-- .../painless/DefBootstrapTests.java | 10 +++++ 12 files changed, 77 insertions(+), 79 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java index 2096d7d7970..97dddbdfe52 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java @@ -20,6 +20,7 @@ package org.elasticsearch.painless; import org.elasticsearch.bootstrap.BootstrapInfo; +import org.elasticsearch.painless.Locals.LocalMethod; import org.elasticsearch.painless.antlr.Walker; import org.elasticsearch.painless.lookup.PainlessLookup; import org.elasticsearch.painless.node.SSource; @@ -32,6 +33,7 @@ import java.net.URL; import java.security.CodeSource; import java.security.SecureClassLoader; import java.security.cert.Certificate; +import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; import static org.elasticsearch.painless.WriterConstants.CLASS_NAME; @@ -200,7 +202,7 @@ final class Compiler { ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, scriptClass); SSource root = Walker.buildPainlessTree(scriptClassInfo, reserved, name, source, settings, painlessLookup, null); - root.analyze(painlessLookup); + Map localMethods = root.analyze(painlessLookup); root.write(); try { @@ -209,6 +211,7 @@ final class Compiler { clazz.getField("$SOURCE").set(null, source); clazz.getField("$STATEMENTS").set(null, root.getStatements()); clazz.getField("$DEFINITION").set(null, painlessLookup); + clazz.getField("$LOCALS").set(null, localMethods); return clazz.getConstructors()[0]; } catch (Exception exception) { // Catch everything to let the user know this is something caused internally. diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java index 10806b64d0e..1db1e4f7cac 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java @@ -19,6 +19,7 @@ package org.elasticsearch.painless; +import org.elasticsearch.painless.Locals.LocalMethod; import org.elasticsearch.painless.lookup.PainlessClass; import org.elasticsearch.painless.lookup.PainlessLookup; import org.elasticsearch.painless.lookup.PainlessLookupUtility; @@ -232,8 +233,10 @@ public final class Def { * @throws IllegalArgumentException if no matching whitelisted method was found. * @throws Throwable if a method reference cannot be converted to an functional interface */ - static MethodHandle lookupMethod(PainlessLookup painlessLookup, MethodHandles.Lookup methodHandlesLookup, MethodType callSiteType, - Class receiverClass, String name, Object args[]) throws Throwable { + static MethodHandle lookupMethod(PainlessLookup painlessLookup, Map localMethods, + MethodHandles.Lookup methodHandlesLookup, MethodType callSiteType, Class receiverClass, String name, Object args[]) + throws Throwable { + String recipeString = (String) args[0]; int numArguments = callSiteType.parameterCount(); // simple case: no lambdas @@ -286,6 +289,7 @@ public final class Def { // the implementation is strongly typed, now that we know the interface type, // we have everything. filter = lookupReferenceInternal(painlessLookup, + localMethods, methodHandlesLookup, interfaceType, type, @@ -297,6 +301,7 @@ public final class Def { // this cache). It won't blow up since we never nest here (just references) MethodType nestedType = MethodType.methodType(interfaceType, captures); CallSite nested = DefBootstrap.bootstrap(painlessLookup, + localMethods, methodHandlesLookup, call, nestedType, @@ -324,8 +329,8 @@ public final class Def { * This is just like LambdaMetaFactory, only with a dynamic type. The interface type is known, * so we simply need to lookup the matching implementation method based on receiver type. */ - static MethodHandle lookupReference(PainlessLookup painlessLookup, MethodHandles.Lookup methodHandlesLookup, String interfaceClass, - Class receiverClass, String name) throws Throwable { + static MethodHandle lookupReference(PainlessLookup painlessLookup, Map localMethods, + MethodHandles.Lookup methodHandlesLookup, String interfaceClass, Class receiverClass, String name) throws Throwable { Class interfaceType = painlessLookup.canonicalTypeNameToType(interfaceClass); PainlessMethod interfaceMethod = painlessLookup.lookupPainlessClass(interfaceType).functionalMethod; if (interfaceMethod == null) { @@ -333,15 +338,14 @@ public final class Def { } int arity = interfaceMethod.typeParameters.size(); PainlessMethod implMethod = lookupMethodInternal(painlessLookup, receiverClass, name, arity); - return lookupReferenceInternal(painlessLookup, methodHandlesLookup, interfaceType, - PainlessLookupUtility.typeToCanonicalTypeName(implMethod.targetClass), + return lookupReferenceInternal(painlessLookup, localMethods, methodHandlesLookup, + interfaceType, PainlessLookupUtility.typeToCanonicalTypeName(implMethod.targetClass), implMethod.javaMethod.getName(), receiverClass); } /** Returns a method handle to an implementation of clazz, given method reference signature. */ - private static MethodHandle lookupReferenceInternal(PainlessLookup painlessLookup, MethodHandles.Lookup methodHandlesLookup, - Class clazz, String type, String call, Class... captures) - throws Throwable { + private static MethodHandle lookupReferenceInternal(PainlessLookup painlessLookup, Map localMethods, + MethodHandles.Lookup methodHandlesLookup, Class clazz, String type, String call, Class... captures) throws Throwable { final FunctionRef ref; if ("this".equals(type)) { // user written method @@ -351,13 +355,8 @@ public final class Def { "to [" + PainlessLookupUtility.typeToCanonicalTypeName(clazz) + "], not a functional interface"); } int arity = interfaceMethod.typeParameters.size() + captures.length; - final MethodHandle handle; - try { - MethodHandle accessor = methodHandlesLookup.findStaticGetter(methodHandlesLookup.lookupClass(), - getUserFunctionHandleFieldName(call, arity), - MethodHandle.class); - handle = (MethodHandle)accessor.invokeExact(); - } catch (NoSuchFieldException | IllegalAccessException e) { + LocalMethod localMethod = localMethods.get(Locals.buildLocalMethodKey(call, arity)); + if (localMethod == null) { // is it a synthetic method? If we generated the method ourselves, be more helpful. It can only fail // because the arity does not match the expected interface type. if (call.contains("$")) { @@ -366,7 +365,7 @@ public final class Def { } throw new IllegalArgumentException("Unknown call [" + call + "] with [" + arity + "] arguments."); } - ref = new FunctionRef(clazz, interfaceMethod, call, handle.type(), captures.length); + ref = new FunctionRef(clazz, interfaceMethod, call, localMethod.methodType, captures.length); } else { // whitelist lookup ref = FunctionRef.resolveFromLookup(painlessLookup, clazz, type, call, captures.length); @@ -385,11 +384,6 @@ public final class Def { return callSite.dynamicInvoker().asType(MethodType.methodType(clazz, captures)); } - /** gets the field name used to lookup up the MethodHandle for a function. */ - public static String getUserFunctionHandleFieldName(String name, int arity) { - return "handle$" + name + "$" + arity; - } - /** * Looks up handle for a dynamic field getter (field load) *

diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/DefBootstrap.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/DefBootstrap.java index 2fadaf30964..2488b6f218a 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/DefBootstrap.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/DefBootstrap.java @@ -20,6 +20,7 @@ package org.elasticsearch.painless; import org.elasticsearch.common.SuppressForbidden; +import org.elasticsearch.painless.Locals.LocalMethod; import org.elasticsearch.painless.lookup.PainlessLookup; import java.lang.invoke.CallSite; @@ -28,6 +29,7 @@ import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import java.lang.invoke.MutableCallSite; import java.lang.invoke.WrongMethodTypeException; +import java.util.Map; /** * Painless invokedynamic bootstrap for the call site. @@ -105,19 +107,21 @@ public final class DefBootstrap { static final int MAX_DEPTH = 5; private final PainlessLookup painlessLookup; + private final Map localMethods; private final MethodHandles.Lookup methodHandlesLookup; private final String name; private final int flavor; private final Object[] args; int depth; // pkg-protected for testing - PIC(PainlessLookup painlessLookup, MethodHandles.Lookup methodHandlesLookup, - String name, MethodType type, int initialDepth, int flavor, Object[] args) { + PIC(PainlessLookup painlessLookup, Map localMethods, + MethodHandles.Lookup methodHandlesLookup, String name, MethodType type, int initialDepth, int flavor, Object[] args) { super(type); if (type.parameterType(0) != Object.class) { throw new BootstrapMethodError("The receiver type (1st arg) of invokedynamic descriptor must be Object."); } this.painlessLookup = painlessLookup; + this.localMethods = localMethods; this.methodHandlesLookup = methodHandlesLookup; this.name = name; this.flavor = flavor; @@ -145,7 +149,7 @@ public final class DefBootstrap { private MethodHandle lookup(int flavor, String name, Class receiver) throws Throwable { switch(flavor) { case METHOD_CALL: - return Def.lookupMethod(painlessLookup, methodHandlesLookup, type(), receiver, name, args); + return Def.lookupMethod(painlessLookup, localMethods, methodHandlesLookup, type(), receiver, name, args); case LOAD: return Def.lookupGetter(painlessLookup, receiver, name); case STORE: @@ -157,7 +161,7 @@ public final class DefBootstrap { case ITERATOR: return Def.lookupIterator(receiver); case REFERENCE: - return Def.lookupReference(painlessLookup, methodHandlesLookup, (String) args[0], receiver, name); + return Def.lookupReference(painlessLookup, localMethods, methodHandlesLookup, (String) args[0], receiver, name); case INDEX_NORMALIZE: return Def.lookupIndexNormalize(receiver); default: throw new AssertionError(); @@ -432,8 +436,9 @@ public final class DefBootstrap { *

* see https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-6.html#jvms-6.5.invokedynamic */ - public static CallSite bootstrap(PainlessLookup painlessLookup, MethodHandles.Lookup methodHandlesLookup, String name, - MethodType type, int initialDepth, int flavor, Object... args) { + @SuppressWarnings("unchecked") + public static CallSite bootstrap(PainlessLookup painlessLookup, Map localMethods, + MethodHandles.Lookup methodHandlesLookup, String name, MethodType type, int initialDepth, int flavor, Object... args) { // validate arguments switch(flavor) { // "function-call" like things get a polymorphic cache @@ -452,7 +457,7 @@ public final class DefBootstrap { if (args.length != numLambdas + 1) { throw new BootstrapMethodError("Illegal number of parameters: expected " + numLambdas + " references"); } - return new PIC(painlessLookup, methodHandlesLookup, name, type, initialDepth, flavor, args); + return new PIC(painlessLookup, localMethods, methodHandlesLookup, name, type, initialDepth, flavor, args); case LOAD: case STORE: case ARRAY_LOAD: @@ -462,7 +467,7 @@ public final class DefBootstrap { if (args.length > 0) { throw new BootstrapMethodError("Illegal static bootstrap parameters for flavor: " + flavor); } - return new PIC(painlessLookup, methodHandlesLookup, name, type, initialDepth, flavor, args); + return new PIC(painlessLookup, localMethods, methodHandlesLookup, name, type, initialDepth, flavor, args); case REFERENCE: if (args.length != 1) { throw new BootstrapMethodError("Invalid number of parameters for reference call"); @@ -470,7 +475,7 @@ public final class DefBootstrap { if (args[0] instanceof String == false) { throw new BootstrapMethodError("Illegal parameter for reference call: " + args[0]); } - return new PIC(painlessLookup, methodHandlesLookup, name, type, initialDepth, flavor, args); + return new PIC(painlessLookup, localMethods, methodHandlesLookup, name, type, initialDepth, flavor, args); // operators get monomorphic cache, with a generic impl for a fallback case UNARY_OPERATOR: diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Locals.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Locals.java index f2c7e02c637..e07c016ddd0 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Locals.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Locals.java @@ -32,6 +32,9 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; + +import static org.elasticsearch.painless.lookup.PainlessLookupUtility.typeToJavaType; /** * Tracks user defined methods and variables across compilation phases. @@ -74,7 +77,10 @@ public final class Locals { /** Creates a new local variable scope (e.g. loop) inside the current scope */ public static Locals newLocalScope(Locals currentScope) { - return new Locals(currentScope); + Locals locals = new Locals(currentScope); + locals.methods = currentScope.methods; + + return locals; } /** @@ -82,9 +88,13 @@ public final class Locals { *

* This is just like {@link #newFunctionScope}, except the captured parameters are made read-only. */ - public static Locals newLambdaScope(Locals programScope, Class returnType, List parameters, + public static Locals newLambdaScope(Locals programScope, String name, Class returnType, List parameters, int captureCount, int maxLoopCounter) { Locals locals = new Locals(programScope, programScope.painlessLookup, returnType, KEYWORDS); + locals.methods = programScope.methods; + List> typeParameters = parameters.stream().map(parameter -> typeToJavaType(parameter.clazz)).collect(Collectors.toList()); + locals.methods.put(buildLocalMethodKey(name, parameters.size()), new LocalMethod(name, returnType, typeParameters, + MethodType.methodType(typeToJavaType(returnType), typeParameters))); for (int i = 0; i < parameters.size(); i++) { Parameter parameter = parameters.get(i); // TODO: allow non-captures to be r/w: @@ -104,6 +114,7 @@ public final class Locals { /** Creates a new function scope inside the current scope */ public static Locals newFunctionScope(Locals programScope, Class returnType, List parameters, int maxLoopCounter) { Locals locals = new Locals(programScope, programScope.painlessLookup, returnType, KEYWORDS); + locals.methods = programScope.methods; for (Parameter parameter : parameters) { locals.addVariable(parameter.location, parameter.clazz, parameter.name, false); } @@ -118,6 +129,7 @@ public final class Locals { public static Locals newMainMethodScope(ScriptClassInfo scriptClassInfo, Locals programScope, int maxLoopCounter) { Locals locals = new Locals( programScope, programScope.painlessLookup, scriptClassInfo.getExecuteMethodReturnType(), KEYWORDS); + locals.methods = programScope.methods; // This reference. Internal use only. locals.defineVariable(null, Object.class, THIS, true); @@ -136,6 +148,7 @@ public final class Locals { /** Creates a new program scope: the list of methods. It is the parent for all methods */ public static Locals newProgramScope(PainlessLookup painlessLookup, Collection methods) { Locals locals = new Locals(null, painlessLookup, null, null); + locals.methods = new HashMap<>(); for (LocalMethod method : methods) { locals.addMethod(method); } @@ -167,15 +180,8 @@ public final class Locals { } /** Looks up a method. Returns null if the method does not exist. */ - public LocalMethod getMethod(String key) { - LocalMethod method = lookupMethod(key); - if (method != null) { - return method; - } - if (parent != null) { - return parent.getMethod(key); - } - return null; + public LocalMethod getMethod(String methodName, int methodArity) { + return methods.get(buildLocalMethodKey(methodName, methodArity)); } /** Creates a new variable. Throws IAE if the variable has already been defined (even in a parent) or reserved. */ @@ -260,15 +266,10 @@ public final class Locals { return variables.get(name); } - /** Looks up a method at this scope only. Returns null if the method does not exist. */ - private LocalMethod lookupMethod(String key) { - if (methods == null) { - return null; - } - return methods.get(key); + public Map getMethods() { + return Collections.unmodifiableMap(methods); } - /** Defines a variable at this scope internally. */ private Variable defineVariable(Location location, Class type, String name, boolean readonly) { if (variables == null) { @@ -281,14 +282,9 @@ public final class Locals { } private void addMethod(LocalMethod method) { - if (methods == null) { - methods = new HashMap<>(); - } methods.put(buildLocalMethodKey(method.name, method.typeParameters.size()), method); - // TODO: check result } - private int getNextSlot() { return nextSlotNumber; } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterConstants.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterConstants.java index db3aeff0483..9c3d991080d 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterConstants.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterConstants.java @@ -120,7 +120,7 @@ public final class WriterConstants { DEF_BOOTSTRAP_METHOD.getDescriptor(), false); public static final Type DEF_BOOTSTRAP_DELEGATE_TYPE = Type.getType(DefBootstrap.class); public static final Method DEF_BOOTSTRAP_DELEGATE_METHOD = getAsmMethod(CallSite.class, "bootstrap", PainlessLookup.class, - MethodHandles.Lookup.class, String.class, MethodType.class, int.class, int.class, Object[].class); + Map.class, MethodHandles.Lookup.class, String.class, MethodType.class, int.class, int.class, Object[].class); public static final Type DEF_UTIL_TYPE = Type.getType(Def.class); public static final Method DEF_TO_BOOLEAN = getAsmMethod(boolean.class, "DefToboolean" , Object.class); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java index a0cab7f1a5b..799650c2c5d 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java @@ -570,7 +570,6 @@ public final class PainlessLookupBuilder { PainlessMethod painlessMethod = painlessClassBuilder.staticMethods.get(painlessMethodKey); if (painlessMethod == null) { - org.objectweb.asm.commons.Method asmMethod = org.objectweb.asm.commons.Method.getMethod(javaMethod); MethodHandle methodHandle; if (augmentedClass == null) { diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java index 7605a0c9f7f..1f9973df192 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java @@ -24,7 +24,6 @@ import org.elasticsearch.painless.Locals; import org.elasticsearch.painless.Locals.LocalMethod; import org.elasticsearch.painless.Location; import org.elasticsearch.painless.MethodWriter; -import org.elasticsearch.painless.lookup.PainlessLookupUtility; import org.objectweb.asm.commons.Method; import java.util.List; @@ -59,8 +58,7 @@ public final class ECallLocal extends AExpression { @Override void analyze(Locals locals) { - String methodKey = PainlessLookupUtility.buildPainlessMethodKey(name, arguments.size()); - method = locals.getMethod(methodKey); + method = locals.getMethod(name, arguments.size()); if (method == null) { throw createError(new IllegalArgumentException("Unknown call [" + name + "] with [" + arguments.size() + "] arguments.")); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java index ead2e0c5f70..692581d8118 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EFunctionRef.java @@ -71,7 +71,7 @@ public final class EFunctionRef extends AExpression implements ILambda { throw new IllegalArgumentException("Cannot convert function reference [" + type + "::" + call + "] " + "to [" + PainlessLookupUtility.typeToCanonicalTypeName(expected) + "], not a functional interface"); } - LocalMethod delegateMethod = locals.getMethod(Locals.buildLocalMethodKey(call, interfaceMethod.typeParameters.size())); + LocalMethod delegateMethod = locals.getMethod(call, interfaceMethod.typeParameters.size()); if (delegateMethod == null) { throw new IllegalArgumentException("Cannot convert function reference [" + type + "::" + call + "] " + "to [" + PainlessLookupUtility.typeToCanonicalTypeName(expected) + "], function not found"); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ELambda.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ELambda.java index e84ab006501..ecd11ce1bf7 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ELambda.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ELambda.java @@ -119,6 +119,7 @@ public final class ELambda extends AExpression implements ILambda { actualParamTypeStrs.add(type); } } + } else { // we know the method statically, infer return type and any unknown/def types interfaceMethod = locals.getPainlessLookup().lookupPainlessClass(expected).functionalMethod; @@ -173,7 +174,7 @@ public final class ELambda extends AExpression implements ILambda { desugared = new SFunction(reserved, location, PainlessLookupUtility.typeToCanonicalTypeName(returnType), name, paramTypes, paramNames, statements, true); desugared.generateSignature(locals.getPainlessLookup()); - desugared.analyze(Locals.newLambdaScope(locals.getProgramScope(), returnType, + desugared.analyze(Locals.newLambdaScope(locals.getProgramScope(), desugared.name, returnType, desugared.parameters, captures.size(), reserved.getMaxLoopCounter())); // setup method reference to synthetic method diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java index 8230b543697..4a844c7bc30 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java @@ -20,20 +20,16 @@ package org.elasticsearch.painless.node; import org.elasticsearch.painless.CompilerSettings; -import org.elasticsearch.painless.Constant; -import org.elasticsearch.painless.Def; import org.elasticsearch.painless.Globals; import org.elasticsearch.painless.Locals; import org.elasticsearch.painless.Locals.Parameter; import org.elasticsearch.painless.Locals.Variable; import org.elasticsearch.painless.Location; import org.elasticsearch.painless.MethodWriter; -import org.elasticsearch.painless.WriterConstants; import org.elasticsearch.painless.lookup.PainlessLookup; import org.elasticsearch.painless.lookup.PainlessLookupUtility; import org.elasticsearch.painless.node.SSource.Reserved; import org.objectweb.asm.ClassVisitor; -import org.objectweb.asm.Handle; import org.objectweb.asm.Opcodes; import java.lang.invoke.MethodType; @@ -46,7 +42,6 @@ import java.util.Set; import static java.util.Collections.emptyList; import static java.util.Collections.unmodifiableSet; -import static org.elasticsearch.painless.WriterConstants.CLASS_TYPE; /** * Represents a user-defined function. @@ -218,15 +213,6 @@ public final class SFunction extends AStatement { throw createError(new IllegalStateException("Illegal tree structure.")); } } - - String staticHandleFieldName = Def.getUserFunctionHandleFieldName(name, parameters.size()); - globals.addConstantInitializer(new Constant(location, WriterConstants.METHOD_HANDLE_TYPE, - staticHandleFieldName, this::initializeConstant)); - } - - private void initializeConstant(MethodWriter writer) { - final Handle handle = new Handle(Opcodes.H_INVOKESTATIC, CLASS_TYPE.getInternalName(), name, method.getDescriptor(), false); - writer.push(handle); } @Override diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SSource.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SSource.java index fe735c0db31..0f7445a38c4 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SSource.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SSource.java @@ -69,6 +69,7 @@ import static org.elasticsearch.painless.WriterConstants.EXCEPTION_TYPE; import static org.elasticsearch.painless.WriterConstants.GET_NAME_METHOD; import static org.elasticsearch.painless.WriterConstants.GET_SOURCE_METHOD; import static org.elasticsearch.painless.WriterConstants.GET_STATEMENTS_METHOD; +import static org.elasticsearch.painless.WriterConstants.MAP_TYPE; import static org.elasticsearch.painless.WriterConstants.OUT_OF_MEMORY_ERROR_TYPE; import static org.elasticsearch.painless.WriterConstants.PAINLESS_ERROR_TYPE; import static org.elasticsearch.painless.WriterConstants.PAINLESS_EXPLAIN_ERROR_GET_HEADERS_METHOD; @@ -163,7 +164,7 @@ public final class SSource extends AStatement { throw new IllegalStateException("Illegal tree structure."); } - public void analyze(PainlessLookup painlessLookup) { + public Map analyze(PainlessLookup painlessLookup) { Map methods = new HashMap<>(); for (SFunction function : functions) { @@ -177,7 +178,10 @@ public final class SSource extends AStatement { } } - analyze(Locals.newProgramScope(painlessLookup, methods.values())); + Locals locals = Locals.newProgramScope(painlessLookup, methods.values()); + analyze(locals); + + return locals.getMethods(); } @Override @@ -253,6 +257,7 @@ public final class SSource extends AStatement { globals.getStatements(), settings); bootstrapDef.visitCode(); bootstrapDef.getStatic(CLASS_TYPE, "$DEFINITION", DEFINITION_TYPE); + bootstrapDef.getStatic(CLASS_TYPE, "$LOCALS", MAP_TYPE); bootstrapDef.loadArgs(); bootstrapDef.invokeStatic(DEF_BOOTSTRAP_DELEGATE_TYPE, DEF_BOOTSTRAP_DELEGATE_METHOD); bootstrapDef.returnValue(); @@ -263,8 +268,9 @@ public final class SSource extends AStatement { visitor.visitField(Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, "$SOURCE", STRING_TYPE.getDescriptor(), null, null).visitEnd(); visitor.visitField(Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, "$STATEMENTS", BITSET_TYPE.getDescriptor(), null, null).visitEnd(); - // Write the static variable used by the method to bootstrap def calls + // Write the static variables used by the method to bootstrap def calls visitor.visitField(Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, "$DEFINITION", DEFINITION_TYPE.getDescriptor(), null, null).visitEnd(); + visitor.visitField(Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, "$LOCALS", MAP_TYPE.getDescriptor(), null, null).visitEnd(); org.objectweb.asm.commons.Method init; diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/DefBootstrapTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/DefBootstrapTests.java index 1ef855d561c..a9861341a84 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/DefBootstrapTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/DefBootstrapTests.java @@ -38,6 +38,7 @@ public class DefBootstrapTests extends ESTestCase { /** calls toString() on integers, twice */ public void testOneType() throws Throwable { CallSite site = DefBootstrap.bootstrap(painlessLookup, + Collections.emptyMap(), MethodHandles.publicLookup(), "toString", MethodType.methodType(String.class, Object.class), @@ -58,6 +59,7 @@ public class DefBootstrapTests extends ESTestCase { public void testTwoTypes() throws Throwable { CallSite site = DefBootstrap.bootstrap(painlessLookup, + Collections.emptyMap(), MethodHandles.publicLookup(), "toString", MethodType.methodType(String.class, Object.class), @@ -83,6 +85,7 @@ public class DefBootstrapTests extends ESTestCase { // if this changes, test must be rewritten assertEquals(5, DefBootstrap.PIC.MAX_DEPTH); CallSite site = DefBootstrap.bootstrap(painlessLookup, + Collections.emptyMap(), MethodHandles.publicLookup(), "toString", MethodType.methodType(String.class, Object.class), @@ -109,6 +112,7 @@ public class DefBootstrapTests extends ESTestCase { /** test that we revert to the megamorphic classvalue cache and that it works as expected */ public void testMegamorphic() throws Throwable { DefBootstrap.PIC site = (DefBootstrap.PIC) DefBootstrap.bootstrap(painlessLookup, + Collections.emptyMap(), MethodHandles.publicLookup(), "size", MethodType.methodType(int.class, Object.class), @@ -141,6 +145,7 @@ public class DefBootstrapTests extends ESTestCase { public void testNullGuardAdd() throws Throwable { DefBootstrap.MIC site = (DefBootstrap.MIC) DefBootstrap.bootstrap(painlessLookup, + Collections.emptyMap(), MethodHandles.publicLookup(), "add", MethodType.methodType(Object.class, Object.class, Object.class), @@ -153,6 +158,7 @@ public class DefBootstrapTests extends ESTestCase { public void testNullGuardAddWhenCached() throws Throwable { DefBootstrap.MIC site = (DefBootstrap.MIC) DefBootstrap.bootstrap(painlessLookup, + Collections.emptyMap(), MethodHandles.publicLookup(), "add", MethodType.methodType(Object.class, Object.class, Object.class), @@ -166,6 +172,7 @@ public class DefBootstrapTests extends ESTestCase { public void testNullGuardEq() throws Throwable { DefBootstrap.MIC site = (DefBootstrap.MIC) DefBootstrap.bootstrap(painlessLookup, + Collections.emptyMap(), MethodHandles.publicLookup(), "eq", MethodType.methodType(boolean.class, Object.class, Object.class), @@ -179,6 +186,7 @@ public class DefBootstrapTests extends ESTestCase { public void testNullGuardEqWhenCached() throws Throwable { DefBootstrap.MIC site = (DefBootstrap.MIC) DefBootstrap.bootstrap(painlessLookup, + Collections.emptyMap(), MethodHandles.publicLookup(), "eq", MethodType.methodType(boolean.class, Object.class, Object.class), @@ -197,6 +205,7 @@ public class DefBootstrapTests extends ESTestCase { public void testNoNullGuardAdd() throws Throwable { DefBootstrap.MIC site = (DefBootstrap.MIC) DefBootstrap.bootstrap(painlessLookup, + Collections.emptyMap(), MethodHandles.publicLookup(), "add", MethodType.methodType(Object.class, int.class, Object.class), @@ -211,6 +220,7 @@ public class DefBootstrapTests extends ESTestCase { public void testNoNullGuardAddWhenCached() throws Throwable { DefBootstrap.MIC site = (DefBootstrap.MIC) DefBootstrap.bootstrap(painlessLookup, + Collections.emptyMap(), MethodHandles.publicLookup(), "add", MethodType.methodType(Object.class, int.class, Object.class),