You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by us...@apache.org on 2023/12/05 10:54:04 UTC
(lucene) branch main updated: Rewrite JavaScriptCompiler to use modern JVM features (Java 17) (#12873)
This is an automated email from the ASF dual-hosted git repository.
uschindler pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/lucene.git
The following commit(s) were added to refs/heads/main by this push:
new 880d0ba1a8b Rewrite JavaScriptCompiler to use modern JVM features (Java 17) (#12873)
880d0ba1a8b is described below
commit 880d0ba1a8b70aa19fd1ea83500d845b2678c87f
Author: Uwe Schindler <us...@apache.org>
AuthorDate: Tue Dec 5 11:53:57 2023 +0100
Rewrite JavaScriptCompiler to use modern JVM features (Java 17) (#12873)
* Rewrite Javascript expression compiler to use hidden classes and MethodHandles for functions
* Use dynamic constants for MethodHandles
* Remove invokestatic code and handle everything through dynamic constants
* Rewrite code to patch stack trace (keep Expressions class unmodified)
* Improve generating of constant names
* Remove classloader test (no longer needed)
* Add benchmark
* use better exception in benchmark
* Add documentation, migration guide and a utility method to convert legacy function maps
* also ignore SecurityException here while checking compatibility (if it happens only an imprecise error message is thrown)
* Use Map.copyOf to not clone the map each time we compile an expression
* Add another test with same method multiple times
* Update ASM to 9.6 and set classfile version to Java 17
* Cleanup classloader permissions, unfortunately "createClassLoader" is still needed for Jacoco for God knows what
---
.../randomization/policies/replicator-tests.policy | 2 -
gradle/testing/randomization/policies/tests.policy | 5 +-
gradle/validation/jar-checks.gradle | 3 +
lucene/CHANGES.txt | 6 +
lucene/MIGRATE.md | 14 +
lucene/benchmark-jmh/build.gradle | 3 +-
lucene/benchmark-jmh/src/java/module-info.java | 1 +
.../lucene/benchmark/jmh/ExpressionsBenchmark.java | 153 +++++++++
.../lucene/expressions/js/JavascriptCompiler.java | 344 +++++++++++++--------
.../lucene/expressions/js/CompilerTestCase.java | 17 +-
.../lucene/expressions/js/TestCustomFunctions.java | 261 ++++++----------
.../expressions/js/TestJavascriptCompiler.java | 10 +-
.../expressions/js/TestJavascriptFunction.java | 5 +
lucene/licenses/asm-7.2.jar.sha1 | 1 -
lucene/licenses/asm-9.6.jar.sha1 | 1 +
lucene/licenses/asm-analysis-7.2.jar.sha1 | 1 -
lucene/licenses/asm-commons-7.2.jar.sha1 | 1 -
lucene/licenses/asm-commons-9.6.jar.sha1 | 1 +
lucene/licenses/asm-tree-7.2.jar.sha1 | 1 -
lucene/licenses/asm-tree-9.6.jar.sha1 | 1 +
lucene/licenses/opennlp-tools-1.9.4.jar.sha1 | 2 +-
versions.lock | 7 +-
versions.props | 2 +-
23 files changed, 513 insertions(+), 329 deletions(-)
diff --git a/gradle/testing/randomization/policies/replicator-tests.policy b/gradle/testing/randomization/policies/replicator-tests.policy
index 7deff688915..4e78f4650e5 100644
--- a/gradle/testing/randomization/policies/replicator-tests.policy
+++ b/gradle/testing/randomization/policies/replicator-tests.policy
@@ -23,8 +23,6 @@ grant {
// jetty-specific:
permission java.lang.RuntimePermission "getenv.JETTY_AVAILABLE_PROCESSORS";
permission java.lang.RuntimePermission "getenv.JETTY_WORKER_INSTANCE";
- // servlet stuff
- permission java.lang.RuntimePermission "setContextClassLoader";
// allow TestNRTReplication fork its jvm
permission java.io.FilePermission "${java.home}${/}-", "read,execute";
// read/write access to all system properties (required by jetty in these tests)
diff --git a/gradle/testing/randomization/policies/tests.policy b/gradle/testing/randomization/policies/tests.policy
index c32ae6cedd2..150fa970b97 100644
--- a/gradle/testing/randomization/policies/tests.policy
+++ b/gradle/testing/randomization/policies/tests.policy
@@ -50,14 +50,11 @@ grant {
permission java.lang.RuntimePermission "getStackTrace";
// needed for mock filesystems in tests
permission java.lang.RuntimePermission "fileSystemProvider";
- // analyzers/uima: needed by lucene expressions' JavascriptCompiler
- permission java.lang.RuntimePermission "createClassLoader";
// needed to test unmap hack on platforms that support it
permission java.lang.RuntimePermission "accessClassInPackage.sun.misc";
permission java.lang.reflect.ReflectPermission "suppressAccessChecks";
// needed by cyberneko usage by benchmarks on J9
permission java.lang.RuntimePermission "accessClassInPackage.org.apache.xerces.util";
- permission java.lang.RuntimePermission "getClassLoader";
// Needed for loading native library (lucene:misc:native) in lucene:misc
permission java.lang.RuntimePermission "getFileStoreAttributes";
@@ -111,6 +108,8 @@ grant {
permission java.lang.RuntimePermission "shutdownHooks";
// needed by jacoco to instrument classes
permission java.lang.RuntimePermission "defineClass";
+ // needed by jacoco for God knows what.
+ permission java.lang.RuntimePermission "createClassLoader";
};
// Grant all permissions to Gradle test runner classes.
diff --git a/gradle/validation/jar-checks.gradle b/gradle/validation/jar-checks.gradle
index f0fff470f35..7fa6cd3b487 100644
--- a/gradle/validation/jar-checks.gradle
+++ b/gradle/validation/jar-checks.gradle
@@ -59,6 +59,9 @@ allprojects {
}
subprojects {
+ // initialize empty, because no checks for benchmark-jmh module.
+ ext.jarInfos = []
+
// Configure jarValidation configuration for all projects. Any dependency
// declared on this configuration (or any configuration it extends from) will
// be verified.
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 4d24255ab07..3a1ac358fe3 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -67,6 +67,9 @@ API Changes
* GITHUB#11023: Adding -level param to CheckIndex, making the old -fast param the default behaviour. (Jakub Slowinski)
+* GITHUB#12873: Expressions module now uses MethodHandles to define custom functions. Support for
+ custom classloaders was removed. (Uwe Schindler)
+
New Features
---------------------
@@ -89,6 +92,9 @@ Improvements
* GITHUB#12447: Hunspell: speed up the dictionary enumeration on suggestion (Peter Gromov)
+* GITHUB#12873: Expressions module now uses JEP 371 "Hidden Classes" with JEP 309
+ "Dynamic Class-File Constants" to implement Javascript expressions. (Uwe Schindler)
+
Optimizations
---------------------
diff --git a/lucene/MIGRATE.md b/lucene/MIGRATE.md
index 97fd943926f..6960d435a00 100644
--- a/lucene/MIGRATE.md
+++ b/lucene/MIGRATE.md
@@ -108,6 +108,20 @@ Added a new parameter: `-level X`, to set the detail level of the index check. T
Sample `-level` usage: `1` (Default) - Checksum checks only, `2` - all level 1 checks as well as logical integrity checks, `3` - all
level 2 checks as well as slow checks.
+### Expressions module now uses `MethodHandle` and hidden classes (GITHUB#12873)
+
+Custom functions in the expressions module must now be passed in a `Map` using `MethodHandle` as values.
+To convert legacy code using maps of reflective `java.lang.reflect.Method`, use the converter method
+`JavascriptCompiler#convertLegacyFunctions`. This should make the mapping mostly compatible.
+The use of `MethodHandle` and [Dynamic Class-File Constants (JEP 309)](https://openjdk.org/jeps/309)
+now also allows to pass private methods or methods from different classloaders. It is also possible
+to adapt guards or filters using the `MethodHandles` class.
+
+The new implementation of the Javascript expressions compiler no longer supports use of custom
+`ClassLoader`, because it uses the new JDK 15 feature [hidden classes (JEP 371)](https://openjdk.org/jeps/371).
+Due to the use of `MethodHandle`, classloader isolation is no longer needed, because JS code can only call
+MHs that were resolved by the application before using the expressions module.
+
## Migration from Lucene 9.0 to Lucene 9.1
### Test framework package migration and module (LUCENE-10301)
diff --git a/lucene/benchmark-jmh/build.gradle b/lucene/benchmark-jmh/build.gradle
index 4a35d998dc9..2a0e0bffd01 100644
--- a/lucene/benchmark-jmh/build.gradle
+++ b/lucene/benchmark-jmh/build.gradle
@@ -23,6 +23,7 @@ description = 'Lucene JMH micro-benchmarking module'
dependencies {
moduleImplementation project(':lucene:core')
+ moduleImplementation project(':lucene:expressions')
moduleImplementation "org.openjdk.jmh:jmh-core:1.37"
annotationProcessor "org.openjdk.jmh:jmh-generator-annprocess:1.37"
@@ -42,7 +43,7 @@ tasks.matching { it.name == "forbiddenApisMain" }.configureEach {
tasks.matching { it.name in [
// Turn off JMH dependency checksums and licensing (it's GPL w/ classpath exception
// but this seems fine for test/build only tools).
- "validateJarChecksums", "validateJarLicenses",
+ "validateJarChecksums", "validateJarLicenses", "collectJarInfos",
// No special javadocs for JMH benchmarks.
"renderSiteJavadoc",
"renderJavadoc",
diff --git a/lucene/benchmark-jmh/src/java/module-info.java b/lucene/benchmark-jmh/src/java/module-info.java
index c212a42fb7e..d92164cfae1 100644
--- a/lucene/benchmark-jmh/src/java/module-info.java
+++ b/lucene/benchmark-jmh/src/java/module-info.java
@@ -20,6 +20,7 @@ module org.apache.lucene.benchmark.jmh {
requires jmh.core;
requires jdk.unsupported;
requires org.apache.lucene.core;
+ requires org.apache.lucene.expressions;
exports org.apache.lucene.benchmark.jmh;
exports org.apache.lucene.benchmark.jmh.jmh_generated;
diff --git a/lucene/benchmark-jmh/src/java/org/apache/lucene/benchmark/jmh/ExpressionsBenchmark.java b/lucene/benchmark-jmh/src/java/org/apache/lucene/benchmark/jmh/ExpressionsBenchmark.java
new file mode 100644
index 00000000000..e58807dcfa4
--- /dev/null
+++ b/lucene/benchmark-jmh/src/java/org/apache/lucene/benchmark/jmh/ExpressionsBenchmark.java
@@ -0,0 +1,153 @@
+/*
+ * 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.apache.lucene.benchmark.jmh;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.text.ParseException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.concurrent.TimeUnit;
+import org.apache.lucene.expressions.Expression;
+import org.apache.lucene.expressions.js.JavascriptCompiler;
+import org.apache.lucene.search.DoubleValues;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Level;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Param;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Warmup;
+
+@BenchmarkMode(Mode.Throughput)
+@OutputTimeUnit(TimeUnit.MILLISECONDS)
+@State(Scope.Benchmark)
+@Warmup(iterations = 5, time = 5)
+@Measurement(iterations = 12, time = 8)
+@Fork(value = 1)
+public class ExpressionsBenchmark {
+
+ /**
+ * Some extra functions to bench "identity" in various variants, another one is named
+ * "native_identity" (see below).
+ */
+ private static final Map<String, MethodHandle> FUNCTIONS = getFunctions();
+
+ private static final String NATIVE_IDENTITY_NAME = "native_identity";
+
+ private static Map<String, MethodHandle> getFunctions() {
+ try {
+ var lookup = MethodHandles.lookup();
+ Map<String, MethodHandle> m = new HashMap<>(JavascriptCompiler.DEFAULT_FUNCTIONS);
+ m.put(
+ "func_identity",
+ lookup.findStatic(
+ lookup.lookupClass(), "ident", MethodType.methodType(double.class, double.class)));
+ m.put("mh_identity", MethodHandles.identity(double.class));
+ return m;
+ } catch (ReflectiveOperationException e) {
+ throw new AssertionError(e);
+ }
+ }
+
+ @SuppressWarnings("unused")
+ private static double ident(double v) {
+ return v;
+ }
+
+ /** A native implementation of an expression to compare performance */
+ private static final Expression NATIVE_IDENTITY_EXPRESSION =
+ new Expression(NATIVE_IDENTITY_NAME, new String[] {"x"}) {
+ @Override
+ public double evaluate(DoubleValues[] functionValues) {
+ try {
+ return functionValues[0].doubleValue();
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ }
+ };
+
+ private double[] randomData;
+ private Expression expression;
+
+ @Param({"x", "func_identity(x)", "mh_identity", "native_identity", "cos(x)", "cos(x) + sin(x)"})
+ String js;
+
+ @Setup(Level.Iteration)
+ public void init() throws ParseException {
+ ThreadLocalRandom random = ThreadLocalRandom.current();
+ randomData = random.doubles().limit(1024).toArray();
+ expression =
+ Objects.equals(js, NATIVE_IDENTITY_NAME)
+ ? NATIVE_IDENTITY_EXPRESSION
+ : JavascriptCompiler.compile(js, FUNCTIONS);
+ }
+
+ @Benchmark
+ public double expression() throws IOException {
+ var it = new ValuesIterator(randomData);
+ var values = it.getDoubleValues();
+ double result = 0d;
+ while (it.next()) {
+ result += expression.evaluate(values);
+ }
+ return result;
+ }
+
+ static final class ValuesIterator {
+ final double[] data;
+ final DoubleValues[] dv;
+ int pos = -1;
+
+ ValuesIterator(double[] data) {
+ this.data = data;
+ var dv =
+ new DoubleValues() {
+ @Override
+ public double doubleValue() throws IOException {
+ return data[pos];
+ }
+
+ @Override
+ public boolean advanceExact(int doc) throws IOException {
+ throw new UnsupportedOperationException();
+ }
+ };
+ this.dv = new DoubleValues[] {dv};
+ }
+
+ boolean next() {
+ pos++;
+ return (pos < data.length);
+ }
+
+ DoubleValues[] getDoubleValues() {
+ return dv;
+ }
+ }
+}
diff --git a/lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java b/lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java
index e272eb9eb29..8196a2f5c57 100644
--- a/lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java
+++ b/lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java
@@ -18,11 +18,12 @@ package org.apache.lucene.expressions.js;
import java.io.IOException;
import java.io.Reader;
+import java.lang.constant.ConstantDescs;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandleInfo;
import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodHandles.Lookup;
import java.lang.invoke.MethodType;
-import java.lang.reflect.Constructor;
-import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
import java.nio.charset.StandardCharsets;
import java.text.ParseException;
import java.util.ArrayDeque;
@@ -32,7 +33,11 @@ import java.util.Deque;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
+import java.util.Objects;
import java.util.Properties;
+import java.util.function.Function;
+import java.util.function.Supplier;
+import java.util.stream.Stream;
import org.antlr.v4.runtime.ANTLRInputStream;
import org.antlr.v4.runtime.BaseErrorListener;
import org.antlr.v4.runtime.CommonTokenStream;
@@ -46,10 +51,13 @@ import org.apache.lucene.expressions.js.JavascriptParser.ExpressionContext;
import org.apache.lucene.search.DoubleValues;
import org.apache.lucene.util.IOUtils;
import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.ConstantDynamic;
+import org.objectweb.asm.Handle;
import org.objectweb.asm.Label;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.Type;
import org.objectweb.asm.commons.GeneratorAdapter;
+import org.objectweb.asm.commons.Method;
/**
* An expression compiler for javascript expressions.
@@ -63,68 +71,72 @@ import org.objectweb.asm.commons.GeneratorAdapter;
* <p>See the {@link org.apache.lucene.expressions.js package documentation} for the supported
* syntax and default functions.
*
- * <p>You can compile with an alternate set of functions via {@link #compile(String, Map,
- * ClassLoader)}. For example:
+ * <p>You can compile with an alternate set of functions via {@link #compile(String, Map)}. For
+ * example:
*
* <pre class="prettyprint">
- * Map<String,Method> functions = new HashMap<>();
+ * Map<String,MethodHandle> functions = new HashMap<>();
* // add all the default functions
* functions.putAll(JavascriptCompiler.DEFAULT_FUNCTIONS);
* // add cbrt()
- * functions.put("cbrt", Math.class.getMethod("cbrt", double.class));
+ * functions.put("cbrt", MethodHandles.publicLookup().findStatic(Math.class, "cbrt",
+ * MethodType.methodType(double.class, double.class)));
* // call compile with customized function map
* Expression foo = JavascriptCompiler.compile("cbrt(score)+ln(popularity)",
- * functions,
- * getClass().getClassLoader());
+ * functions);
* </pre>
*
+ * <p>It is possible to pass any {@link MethodHandle} as function that only takes {@code double}
+ * parameters and returns a {@code double}. The method does not need to be public, it just needs to
+ * be resolved correctly using a private {@link Lookup} instance. Ideally the methods should be
+ * {@code static}, but you can use {@link MethodHandle#bindTo(Object)} to bind it to a receiver.
+ *
* @lucene.experimental
*/
public final class JavascriptCompiler {
- static final class Loader extends ClassLoader {
- Loader(ClassLoader parent) {
- super(parent);
- }
- public Class<? extends Expression> define(String className, byte[] bytecode) {
- return defineClass(className, bytecode, 0, bytecode.length).asSubclass(Expression.class);
- }
- }
+ private static final Lookup LOOKUP = MethodHandles.lookup();
+
+ private static final int CLASSFILE_VERSION = Opcodes.V17;
- private static final int CLASSFILE_VERSION = Opcodes.V1_8;
+ private static final MethodType MT_EXPRESSION_CTOR_LOOKUP =
+ MethodType.methodType(void.class, String.class, String[].class);
- // We use the same class name for all generated classes as they all have their own class loader.
+ // We use the same class name for all generated classes (they are hidden anyways).
// The source code is displayed as "source file name" in stack trace.
private static final String COMPILED_EXPRESSION_CLASS =
JavascriptCompiler.class.getName() + "$CompiledExpression";
private static final String COMPILED_EXPRESSION_INTERNAL =
COMPILED_EXPRESSION_CLASS.replace('.', '/');
- static final Type EXPRESSION_TYPE = Type.getType(Expression.class);
- static final Type FUNCTION_VALUES_TYPE = Type.getType(DoubleValues.class);
-
- private static final org.objectweb.asm.commons.Method
+ private static final Type EXPRESSION_TYPE = Type.getType(Expression.class),
+ FUNCTION_VALUES_TYPE = Type.getType(DoubleValues.class),
+ METHOD_HANDLE_TYPE = Type.getType(MethodHandle.class),
+ JAVASCRIPT_COMPILER_TYPE = Type.getType(JavascriptCompiler.class),
+ THROWABLE_TYPE = Type.getType(Throwable.class);
+ private static final Method
EXPRESSION_CTOR = getAsmMethod(void.class, "<init>", String.class, String[].class),
- EVALUATE_METHOD = getAsmMethod(double.class, "evaluate", DoubleValues[].class);
-
- static final org.objectweb.asm.commons.Method DOUBLE_VAL_METHOD =
- getAsmMethod(double.class, "doubleValue");
+ EVALUATE_METHOD = getAsmMethod(double.class, "evaluate", DoubleValues[].class),
+ DOUBLE_VAL_METHOD = getAsmMethod(double.class, "doubleValue"),
+ PATCH_STACK_METHOD =
+ getAsmMethod(Throwable.class, "patchStackTrace", Throwable.class, Expression.class);
+ private static final Handle DYNAMIC_CONSTANT_BOOTSTRAP_HANDLE =
+ new Handle(
+ Opcodes.H_INVOKESTATIC,
+ JAVASCRIPT_COMPILER_TYPE.getInternalName(),
+ "dynamicConstantBootstrap",
+ MethodType.methodType(
+ MethodHandle.class, Lookup.class, String.class, Class.class, String.class)
+ .toMethodDescriptorString(),
+ false);
/** create an ASM Method object from return type, method name, and parameters. */
- private static org.objectweb.asm.commons.Method getAsmMethod(
- Class<?> rtype, String name, Class<?>... ptypes) {
- return new org.objectweb.asm.commons.Method(
- name, MethodType.methodType(rtype, ptypes).toMethodDescriptorString());
+ private static Method getAsmMethod(Class<?> rtype, String name, Class<?>... ptypes) {
+ return new Method(name, MethodType.methodType(rtype, ptypes).toMethodDescriptorString());
}
- // This maximum length is theoretically 65535 bytes, but as it's CESU-8 encoded we dont know how
- // large it is in bytes, so be safe
- // rcmuir: "If your ranking function is that large you need to check yourself into a mental
- // institution!"
- private static final int MAX_SOURCE_LENGTH = 16384;
-
final String sourceText;
- final Map<String, Method> functions;
+ final Map<String, MethodHandle> functions;
final boolean picky;
/**
@@ -135,7 +147,7 @@ public final class JavascriptCompiler {
* @throws ParseException on failure to compile
*/
public static Expression compile(String sourceText) throws ParseException {
- return compile(sourceText, DEFAULT_FUNCTIONS, JavascriptCompiler.class.getClassLoader());
+ return compile(sourceText, DEFAULT_FUNCTIONS);
}
/**
@@ -146,15 +158,36 @@ public final class JavascriptCompiler {
* {@code double} parameters.
*
* @param sourceText The expression to compile
- * @param functions map of String names to functions
- * @param parent a {@code ClassLoader} that should be used as the parent of the loaded class. It
- * must contain all classes referred to by the given {@code functions}.
+ * @param functions map of String names to {@link MethodHandle}s
* @return A new compiled expression
* @throws ParseException on failure to compile
*/
- public static Expression compile(
- String sourceText, Map<String, Method> functions, ClassLoader parent) throws ParseException {
- return compile(sourceText, functions, parent, false);
+ public static Expression compile(String sourceText, Map<String, MethodHandle> functions)
+ throws ParseException {
+ return compile(sourceText, functions, false);
+ }
+
+ /**
+ * Converts a legacy map with reflective {@link java.lang.reflect.Method} functions to {@code
+ * Map<String,MethodHandle} for use with {@link #compile(String, Map)}.
+ *
+ * @param functions a map with only public and accessible reflective methods
+ * @return a new (modifiable) map with the same function declarations, but converted to {@link
+ * MethodHandle}
+ * @throws IllegalAccessException if any of the methods in {@code functions} are not accessible by
+ * the public {@link Lookup}.
+ * @deprecated Only use this to convert Lucene 9.x or earlier legacy code. For new code use {@link
+ * MethodHandle}.
+ */
+ @Deprecated
+ public static Map<String, MethodHandle> convertLegacyFunctions(
+ Map<String, java.lang.reflect.Method> functions) throws IllegalAccessException {
+ final var lookup = MethodHandles.publicLookup();
+ final Map<String, MethodHandle> newMap = new HashMap<>();
+ for (var e : functions.entrySet()) {
+ newMap.put(e.getKey(), lookup.unreflect(e.getValue()));
+ }
+ return newMap;
}
/**
@@ -164,25 +197,18 @@ public final class JavascriptCompiler {
* {@code double} parameters.
*
* @param sourceText The expression to compile
- * @param functions map of String names to functions
- * @param parent a {@code ClassLoader} that should be used as the parent of the loaded class. It
- * must contain all classes referred to by the given {@code functions}.
+ * @param functions map of String names to {@link MethodHandle}s
* @param picky whether to throw exception on ambiguity or other internal parsing issues (this
* option makes things slower too, it is only for debugging).
* @return A new compiled expression
* @throws ParseException on failure to compile
*/
- static Expression compile(
- String sourceText, Map<String, Method> functions, ClassLoader parent, boolean picky)
+ static Expression compile(String sourceText, Map<String, MethodHandle> functions, boolean picky)
throws ParseException {
- if (parent == null) {
- throw new NullPointerException("A parent ClassLoader must be given.");
- }
- for (Method m : functions.values()) {
- checkFunctionClassLoader(m, parent);
+ for (MethodHandle m : functions.values()) {
checkFunction(m);
}
- return new JavascriptCompiler(sourceText, functions, picky).compileExpression(parent);
+ return new JavascriptCompiler(sourceText, functions, picky).compileExpression();
}
/**
@@ -190,9 +216,8 @@ public final class JavascriptCompiler {
* If this method fails to compile, you also have to change the byte code generator to correctly
* use the FunctionValues class.
*/
- @SuppressWarnings({"unused", "null"})
- private static void unusedTestCompile() throws IOException {
- DoubleValues f = null;
+ @SuppressWarnings({"unused"})
+ private static void unusedTestCompile(DoubleValues f) throws IOException {
f.doubleValue();
}
@@ -201,41 +226,37 @@ public final class JavascriptCompiler {
*
* @param sourceText The expression to compile
*/
- private JavascriptCompiler(String sourceText, Map<String, Method> functions, boolean picky) {
- if (sourceText == null) {
- throw new NullPointerException();
- }
- this.sourceText = sourceText;
- this.functions = functions;
+ private JavascriptCompiler(
+ String sourceText, Map<String, MethodHandle> functions, boolean picky) {
+ this.sourceText = Objects.requireNonNull(sourceText, "sourceText");
+ this.functions = Map.copyOf(functions);
this.picky = picky;
}
/**
- * Compiles the given expression with the specified parent classloader
+ * Compiles the given expression as hidden class.
*
* @return A new compiled expression
* @throws ParseException on failure to compile
*/
- private Expression compileExpression(ClassLoader parent) throws ParseException {
+ private Expression compileExpression() throws ParseException {
final Map<String, Integer> externalsMap = new LinkedHashMap<>();
final ClassWriter classWriter =
new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS);
try {
generateClass(getAntlrParseTree(), classWriter, externalsMap);
-
- final Class<? extends Expression> evaluatorClass =
- new Loader(parent).define(COMPILED_EXPRESSION_CLASS, classWriter.toByteArray());
- final Constructor<? extends Expression> constructor =
- evaluatorClass.getConstructor(String.class, String[].class);
-
- return constructor.newInstance(
- sourceText, externalsMap.keySet().toArray(new String[externalsMap.size()]));
} catch (RuntimeException re) {
if (re.getCause() instanceof ParseException) {
throw (ParseException) re.getCause();
}
throw re;
+ }
+
+ try {
+ final Lookup lookup =
+ LOOKUP.defineHiddenClassWithClassData(classWriter.toByteArray(), functions, true);
+ return invokeConstructor(lookup, lookup.lookupClass(), externalsMap);
} catch (ReflectiveOperationException exception) {
throw new IllegalStateException(
"An internal error occurred attempting to compile the expression (" + sourceText + ").",
@@ -243,6 +264,19 @@ public final class JavascriptCompiler {
}
}
+ private Expression invokeConstructor(
+ Lookup lookup, Class<?> expressionClass, Map<String, Integer> externalsMap)
+ throws ReflectiveOperationException {
+ final MethodHandle ctor = lookup.findConstructor(expressionClass, MT_EXPRESSION_CTOR_LOOKUP);
+ try {
+ return (Expression) ctor.invoke(sourceText, externalsMap.keySet().toArray(String[]::new));
+ } catch (RuntimeException | Error e) {
+ throw e;
+ } catch (Throwable t) {
+ throw new AssertionError(t);
+ }
+ }
+
/**
* Parses the sourceText into an ANTLR 4 parse tree
*
@@ -311,11 +345,6 @@ public final class JavascriptCompiler {
null,
EXPRESSION_TYPE.getInternalName(),
null);
- final String clippedSourceText =
- (sourceText.length() <= MAX_SOURCE_LENGTH)
- ? sourceText
- : (sourceText.substring(0, MAX_SOURCE_LENGTH - 3) + "...");
- classWriter.visitSource(clippedSourceText, null);
final GeneratorAdapter constructor =
new GeneratorAdapter(Opcodes.ACC_PUBLIC, EXPRESSION_CTOR, null, null, classWriter);
@@ -328,9 +357,15 @@ public final class JavascriptCompiler {
final GeneratorAdapter gen =
new GeneratorAdapter(Opcodes.ACC_PUBLIC, EVALUATE_METHOD, null, null, classWriter);
+ // add a try/catch block to rewrite stack trace of any Throwable
+ final Label beginTry = gen.newLabel(), endTry = gen.newLabel(), catchHandler = gen.newLabel();
+ gen.visitTryCatchBlock(beginTry, endTry, catchHandler, THROWABLE_TYPE.getInternalName());
+ gen.mark(beginTry);
+
// to completely hide the ANTLR visitor we use an anonymous impl:
new JavascriptBaseVisitor<Void>() {
private final Deque<Type> typeStack = new ArrayDeque<>();
+ private final Map<String, Integer> constantsMap = new HashMap<>();
@Override
public Void visitCompile(JavascriptParser.CompileContext ctx) {
@@ -369,11 +404,11 @@ public final class JavascriptCompiler {
String text = ctx.VARIABLE().getText();
int arguments = ctx.expression().size();
boolean parens = ctx.LP() != null && ctx.RP() != null;
- Method method = parens ? functions.get(text) : null;
+ MethodHandle mh = parens ? functions.get(text) : null;
try {
- if (method != null) {
- int arity = method.getParameterTypes().length;
+ if (mh != null) {
+ final int arity = mh.type().parameterCount();
if (arguments != arity) {
throw new ParseException(
@@ -389,30 +424,35 @@ public final class JavascriptCompiler {
ctx.start.getStartIndex());
}
- typeStack.push(Type.DOUBLE_TYPE);
+ // place dynamic constant with MethodHandle on top of stack
+ gen.visitLdcInsn(
+ new ConstantDynamic(
+ "func" + constantsMap.computeIfAbsent(text, k -> constantsMap.size()),
+ METHOD_HANDLE_TYPE.getDescriptor(),
+ DYNAMIC_CONSTANT_BOOTSTRAP_HANDLE,
+ text));
+ // add arguments:
+ typeStack.push(Type.DOUBLE_TYPE);
for (int argument = 0; argument < arguments; ++argument) {
visit(ctx.expression(argument));
}
-
typeStack.pop();
- gen.invokeStatic(
- Type.getType(method.getDeclaringClass()),
- org.objectweb.asm.commons.Method.getMethod(method));
+ // invoke MethodHandle of function:
+ gen.visitMethodInsn(
+ Opcodes.INVOKEVIRTUAL,
+ METHOD_HANDLE_TYPE.getInternalName(),
+ "invokeExact",
+ mh.type().descriptorString(),
+ false);
gen.cast(Type.DOUBLE_TYPE, typeStack.peek());
} else if (!parens || arguments == 0 && text.contains(".")) {
int index;
text = normalizeQuotes(ctx.getText());
-
- if (externalsMap.containsKey(text)) {
- index = externalsMap.get(text);
- } else {
- index = externalsMap.size();
- externalsMap.put(text, index);
- }
+ index = externalsMap.computeIfAbsent(text, k -> externalsMap.size());
gen.loadArg(0);
gen.push(index);
@@ -729,7 +769,13 @@ public final class JavascriptCompiler {
}
}.visit(parseTree);
+ gen.mark(endTry);
gen.returnValue();
+
+ gen.mark(catchHandler);
+ gen.loadThis();
+ gen.invokeStatic(JAVASCRIPT_COMPILER_TYPE, PATCH_STACK_METHOD);
+ gen.throwException();
gen.endMethod();
classWriter.visitEnd();
@@ -780,10 +826,11 @@ public final class JavascriptCompiler {
*
* <p>See the {@link org.apache.lucene.expressions.js package documentation} for a list.
*/
- public static final Map<String, Method> DEFAULT_FUNCTIONS;
+ public static final Map<String, MethodHandle> DEFAULT_FUNCTIONS = loadDefaultFunctions();
- static {
- Map<String, Method> map = new HashMap<>();
+ private static final Map<String, MethodHandle> loadDefaultFunctions() {
+ final Map<String, MethodHandle> map = new HashMap<>();
+ final Lookup publicLookup = MethodHandles.publicLookup();
try {
final Properties props = new Properties();
var name = JavascriptCompiler.class.getSimpleName() + ".properties";
@@ -802,61 +849,92 @@ public final class JavascriptCompiler {
final Class<?> clazz = Class.forName(vals[0].trim());
final String methodName = vals[1].trim();
final int arity = Integer.parseInt(vals[2].trim());
- @SuppressWarnings({"rawtypes", "unchecked"})
- Class[] args = new Class[arity];
- Arrays.fill(args, double.class);
- Method method = clazz.getMethod(methodName, args);
- checkFunction(method);
- map.put(call, method);
+ final MethodHandle mh =
+ publicLookup.findStatic(
+ clazz,
+ methodName,
+ MethodType.methodType(double.class, Collections.nCopies(arity, double.class)));
+ checkFunction(mh);
+ map.put(call, mh);
}
} catch (ReflectiveOperationException | IOException e) {
throw new Error("Cannot resolve function", e);
}
- DEFAULT_FUNCTIONS = Collections.unmodifiableMap(map);
+ return Map.copyOf(map);
}
/** Check Method signature for compatibility. */
- private static void checkFunction(Method method) {
- // check that the Method is public in some public reachable class:
- final MethodType type;
+ private static void checkFunction(MethodHandle method) {
+ Supplier<String> methodNameSupplier = method::toString;
+
+ // try to crack the handle and check if it is a static call:
+ int refKind;
try {
- type = MethodHandles.publicLookup().unreflect(method).type();
- } catch (IllegalAccessException iae) {
- throw new IllegalArgumentException(
- method + " is not accessible (declaring class or method not public).", iae);
+ MethodHandleInfo cracked = LOOKUP.revealDirect(method);
+ refKind = cracked.getReferenceKind();
+ // we have a much better name for the method so display it instead:
+ methodNameSupplier =
+ () ->
+ cracked.getDeclaringClass().getName()
+ + "#"
+ + cracked.getName()
+ + cracked.getMethodType();
+ } catch (@SuppressWarnings("unused") IllegalArgumentException | SecurityException iae) {
+ // can't check for static, we assume it is static
+ // (it does not matter as we call the MethodHandle directly if it is compatible):
+ refKind = MethodHandleInfo.REF_invokeStatic;
}
- // do some checks if the signature is "compatible":
- if (!Modifier.isStatic(method.getModifiers())) {
- throw new IllegalArgumentException(method + " is not static.");
+ if (refKind != MethodHandleInfo.REF_invokeStatic && refKind != MethodHandleInfo.REF_getStatic) {
+ throw new IllegalArgumentException(methodNameSupplier.get() + " is not static.");
}
+
+ // do some checks if the signature is "compatible":
+ final MethodType type = method.type();
for (int arg = 0, arity = type.parameterCount(); arg < arity; arg++) {
if (type.parameterType(arg) != double.class) {
- throw new IllegalArgumentException(method + " must take only double parameters.");
+ throw new IllegalArgumentException(
+ methodNameSupplier.get() + " must take only double parameters.");
}
}
if (type.returnType() != double.class) {
- throw new IllegalArgumentException(method + " does not return a double.");
+ throw new IllegalArgumentException(methodNameSupplier.get() + " does not return a double.");
}
}
/**
- * Cross check if declaring class of given method is the same as returned by the given parent
- * {@link ClassLoader} on string lookup. This prevents {@link NoClassDefFoundError}.
+ * Bootstrap method for dynamic constants. This returns a {@link MethodHandle} for the {@code
+ * functionName} from the class data passed via {@link Lookup#defineHiddenClassWithClassData}. The
+ * {@code constantName} is ignored.
*/
- private static void checkFunctionClassLoader(Method method, ClassLoader parent) {
- boolean ok = false;
- try {
- final Class<?> clazz = method.getDeclaringClass();
- ok = Class.forName(clazz.getName(), false, parent) == clazz;
- } catch (
- @SuppressWarnings("unused")
- ClassNotFoundException e) {
- ok = false;
- }
- if (!ok) {
- throw new IllegalArgumentException(
- method
- + " is not declared by a class which is accessible by the given parent ClassLoader.");
+ static MethodHandle dynamicConstantBootstrap(
+ Lookup lookup, String constantName, Class<?> type, String functionName)
+ throws IllegalAccessException {
+ if (type != MethodHandle.class) {
+ throw new IllegalArgumentException("Invalid type of constant: " + type.getName());
}
+ final var classData =
+ Objects.requireNonNull(
+ MethodHandles.classData(lookup, ConstantDescs.DEFAULT_NAME, Map.class),
+ "Missing class data for " + lookup);
+ return (MethodHandle)
+ Objects.requireNonNull(
+ classData.get(functionName), "Function does not exist: " + functionName);
+ }
+
+ /**
+ * Method called from try/catch handler in compiled expression. This patches the stack trace and
+ * adds back a hidden frame (including the source code of script as filename).
+ */
+ static Throwable patchStackTrace(Throwable t, Expression impl) {
+ var extra = new StackTraceElement(impl.getClass().getName(), "evaluate", impl.sourceText, -1);
+ var origStack = t.getStackTrace();
+ var myStack = new Throwable().getStackTrace();
+ var top = Arrays.stream(origStack).limit(Math.max(0, origStack.length - myStack.length + 1));
+ var tail = Arrays.stream(myStack).skip(1);
+ t.setStackTrace(
+ Stream.of(top, Stream.of(extra), tail)
+ .flatMap(Function.identity())
+ .toArray(StackTraceElement[]::new));
+ return t;
}
}
diff --git a/lucene/expressions/src/test/org/apache/lucene/expressions/js/CompilerTestCase.java b/lucene/expressions/src/test/org/apache/lucene/expressions/js/CompilerTestCase.java
index cc24d24fe1e..54547275a66 100644
--- a/lucene/expressions/src/test/org/apache/lucene/expressions/js/CompilerTestCase.java
+++ b/lucene/expressions/src/test/org/apache/lucene/expressions/js/CompilerTestCase.java
@@ -17,7 +17,7 @@
package org.apache.lucene.expressions.js;
-import java.lang.reflect.Method;
+import java.lang.invoke.MethodHandle;
import java.text.ParseException;
import java.util.Map;
import org.apache.lucene.expressions.Expression;
@@ -28,21 +28,12 @@ public abstract class CompilerTestCase extends LuceneTestCase {
/** compiles expression for sourceText with default functions list */
protected Expression compile(String sourceText) throws ParseException {
- return compile(
- sourceText,
- JavascriptCompiler.DEFAULT_FUNCTIONS,
- JavascriptCompiler.class.getClassLoader());
+ return compile(sourceText, JavascriptCompiler.DEFAULT_FUNCTIONS);
}
/** compiles expression for sourceText with custom functions list */
- protected Expression compile(String sourceText, Map<String, Method> functions)
+ protected Expression compile(String sourceText, Map<String, MethodHandle> functions)
throws ParseException {
- return compile(sourceText, functions, JavascriptCompiler.class.getClassLoader());
- }
-
- /** compiles expression for sourceText with custom functions list and parent classloader */
- protected Expression compile(String sourceText, Map<String, Method> functions, ClassLoader parent)
- throws ParseException {
- return JavascriptCompiler.compile(sourceText, functions, parent, true);
+ return JavascriptCompiler.compile(sourceText, functions, true);
}
}
diff --git a/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestCustomFunctions.java b/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestCustomFunctions.java
index e0273d2df7d..debece300d9 100644
--- a/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestCustomFunctions.java
+++ b/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestCustomFunctions.java
@@ -18,24 +18,24 @@ package org.apache.lucene.expressions.js;
import java.io.PrintWriter;
import java.io.StringWriter;
-import java.lang.reflect.Method;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodHandles.Lookup;
+import java.lang.invoke.MethodType;
import java.text.ParseException;
import java.util.Collections;
-import java.util.HashMap;
import java.util.Map;
import org.apache.lucene.expressions.Expression;
-import org.objectweb.asm.ClassWriter;
-import org.objectweb.asm.Opcodes;
-import org.objectweb.asm.Type;
-import org.objectweb.asm.commons.GeneratorAdapter;
+import org.apache.lucene.tests.util.LuceneTestCase;
/** Tests customing the function map */
public class TestCustomFunctions extends CompilerTestCase {
private static double DELTA = 0.0000001;
+ private static final Lookup LOOKUP = MethodHandles.lookup();
/** empty list of methods */
public void testEmpty() throws Exception {
- Map<String, Method> functions = Collections.emptyMap();
+ Map<String, MethodHandle> functions = Map.of();
ParseException expected =
expectThrows(
ParseException.class,
@@ -49,7 +49,7 @@ public class TestCustomFunctions extends CompilerTestCase {
/** using the default map explicitly */
public void testDefaultList() throws Exception {
- Map<String, Method> functions = JavascriptCompiler.DEFAULT_FUNCTIONS;
+ Map<String, MethodHandle> functions = JavascriptCompiler.DEFAULT_FUNCTIONS;
Expression expr = compile("sqrt(20)", functions);
assertEquals(Math.sqrt(20), expr.evaluate(null), DELTA);
}
@@ -58,10 +58,20 @@ public class TestCustomFunctions extends CompilerTestCase {
return 5;
}
+ private static MethodHandle localMethod(String name, MethodType type)
+ throws NoSuchMethodException, IllegalAccessException {
+ return LOOKUP.findStatic(LOOKUP.lookupClass(), name, type);
+ }
+
+ private static MethodHandle localMethod(String name, int arity)
+ throws NoSuchMethodException, IllegalAccessException {
+ return localMethod(
+ name, MethodType.methodType(double.class, Collections.nCopies(arity, double.class)));
+ }
+
/** tests a method with no arguments */
public void testNoArgMethod() throws Exception {
- Map<String, Method> functions = new HashMap<>();
- functions.put("foo", getClass().getMethod("zeroArgMethod"));
+ Map<String, MethodHandle> functions = Map.of("foo", localMethod("zeroArgMethod", 0));
Expression expr = compile("foo()", functions);
assertEquals(5, expr.evaluate(null), DELTA);
}
@@ -72,8 +82,7 @@ public class TestCustomFunctions extends CompilerTestCase {
/** tests a method with one arguments */
public void testOneArgMethod() throws Exception {
- Map<String, Method> functions = new HashMap<>();
- functions.put("foo", getClass().getMethod("oneArgMethod", double.class));
+ Map<String, MethodHandle> functions = Map.of("foo", localMethod("oneArgMethod", 1));
Expression expr = compile("foo(3)", functions);
assertEquals(6, expr.evaluate(null), DELTA);
}
@@ -84,18 +93,15 @@ public class TestCustomFunctions extends CompilerTestCase {
/** tests a method with three arguments */
public void testThreeArgMethod() throws Exception {
- Map<String, Method> functions = new HashMap<>();
- functions.put(
- "foo", getClass().getMethod("threeArgMethod", double.class, double.class, double.class));
+ Map<String, MethodHandle> functions = Map.of("foo", localMethod("threeArgMethod", 3));
Expression expr = compile("foo(3, 4, 5)", functions);
assertEquals(12, expr.evaluate(null), DELTA);
}
/** tests a map with 2 functions */
public void testTwoMethods() throws Exception {
- Map<String, Method> functions = new HashMap<>();
- functions.put("foo", getClass().getMethod("zeroArgMethod"));
- functions.put("bar", getClass().getMethod("oneArgMethod", double.class));
+ Map<String, MethodHandle> functions =
+ Map.of("foo", localMethod("zeroArgMethod", 0), "bar", localMethod("oneArgMethod", 1));
Expression expr = compile("foo() + bar(3)", functions);
assertEquals(11, expr.evaluate(null), DELTA);
}
@@ -142,8 +148,8 @@ public class TestCustomFunctions extends CompilerTestCase {
/** wrong return type: must be double */
public void testWrongReturnType() throws Exception {
- Map<String, Method> functions = new HashMap<>();
- functions.put("foo", getClass().getMethod("bogusReturnType"));
+ Map<String, MethodHandle> functions =
+ Map.of("foo", localMethod("bogusReturnType", MethodType.methodType(String.class)));
IllegalArgumentException expected =
expectThrows(
IllegalArgumentException.class,
@@ -159,8 +165,10 @@ public class TestCustomFunctions extends CompilerTestCase {
/** wrong param type: must be doubles */
public void testWrongParameterType() throws Exception {
- Map<String, Method> functions = new HashMap<>();
- functions.put("foo", getClass().getMethod("bogusParameterType", String.class));
+ Map<String, MethodHandle> functions =
+ Map.of(
+ "foo",
+ localMethod("bogusParameterType", MethodType.methodType(double.class, String.class)));
IllegalArgumentException expected =
expectThrows(
IllegalArgumentException.class,
@@ -176,8 +184,11 @@ public class TestCustomFunctions extends CompilerTestCase {
/** wrong modifiers: must be static */
public void testWrongNotStatic() throws Exception {
- Map<String, Method> functions = new HashMap<>();
- functions.put("foo", getClass().getMethod("nonStaticMethod"));
+ Map<String, MethodHandle> functions =
+ Map.of(
+ "foo",
+ LOOKUP.findVirtual(
+ LOOKUP.lookupClass(), "nonStaticMethod", MethodType.methodType(double.class)));
IllegalArgumentException expected =
expectThrows(
IllegalArgumentException.class,
@@ -188,147 +199,49 @@ public class TestCustomFunctions extends CompilerTestCase {
}
static double nonPublicMethod() {
- return 0;
+ return 7;
}
- /** wrong modifiers: must be public */
- public void testWrongNotPublic() throws Exception {
- Map<String, Method> functions = new HashMap<>();
- functions.put("foo", getClass().getDeclaredMethod("nonPublicMethod"));
- IllegalArgumentException expected =
- expectThrows(
- IllegalArgumentException.class,
- () -> {
- compile("foo()", functions);
- });
- assertTrue(expected.getMessage().contains("not public"));
+ /** non public methods work as the lookup allows access */
+ public void testNotPublic() throws Exception {
+ Map<String, MethodHandle> functions = Map.of("foo", localMethod("nonPublicMethod", 0));
+ Expression expr = compile("foo()", functions);
+ assertEquals(7, expr.evaluate(null), DELTA);
}
static class NestedNotPublic {
public static double method() {
- return 0;
+ return 41;
}
}
- /** wrong class modifiers: class containing method is not public */
- public void testWrongNestedNotPublic() throws Exception {
- Map<String, Method> functions = new HashMap<>();
- functions.put("foo", NestedNotPublic.class.getMethod("method"));
- IllegalArgumentException expected =
- expectThrows(
- IllegalArgumentException.class,
- () -> {
- compile("foo()", functions);
- });
- assertTrue(expected.getMessage().contains("not public"));
- }
-
- /**
- * Classloader that can be used to create a fake static class that has one method returning a
- * static var
- */
- static final class Loader extends ClassLoader implements Opcodes {
- Loader(ClassLoader parent) {
- super(parent);
- }
-
- public Class<?> createFakeClass() {
- String className = TestCustomFunctions.class.getName() + "$Foo";
- ClassWriter classWriter =
- new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS);
- classWriter.visit(
- Opcodes.V1_5,
- ACC_PUBLIC | ACC_SUPER | ACC_FINAL | ACC_SYNTHETIC,
- className.replace('.', '/'),
- null,
- Type.getInternalName(Object.class),
- null);
-
- org.objectweb.asm.commons.Method m =
- org.objectweb.asm.commons.Method.getMethod("void <init>()");
- GeneratorAdapter constructor =
- new GeneratorAdapter(ACC_PRIVATE | ACC_SYNTHETIC, m, null, null, classWriter);
- constructor.loadThis();
- constructor.loadArgs();
- constructor.invokeConstructor(Type.getType(Object.class), m);
- constructor.returnValue();
- constructor.endMethod();
-
- GeneratorAdapter gen =
- new GeneratorAdapter(
- ACC_STATIC | ACC_PUBLIC | ACC_SYNTHETIC,
- org.objectweb.asm.commons.Method.getMethod("double bar()"),
- null,
- null,
- classWriter);
- gen.push(2.0);
- gen.returnValue();
- gen.endMethod();
-
- byte[] bc = classWriter.toByteArray();
- return defineClass(className, bc, 0, bc.length);
- }
+ /** class containing method is not public */
+ public void testNestedNotPublic() throws Exception {
+ Map<String, MethodHandle> functions =
+ Map.of(
+ "foo",
+ LOOKUP.findStatic(
+ NestedNotPublic.class, "method", MethodType.methodType(double.class)));
+ Expression expr = compile("foo()", functions);
+ assertEquals(41, expr.evaluate(null), DELTA);
}
- /**
- * uses this test with a different classloader and tries to register it using the default
- * classloader, which should fail
- */
- public void testClassLoader() throws Exception {
- ClassLoader thisLoader = getClass().getClassLoader();
- Loader childLoader = new Loader(thisLoader);
- Class<?> fooClass = childLoader.createFakeClass();
-
- Method barMethod = fooClass.getMethod("bar");
- Map<String, Method> functions = Collections.singletonMap("bar", barMethod);
- assertNotSame(thisLoader, fooClass.getClassLoader());
- assertNotSame(thisLoader, barMethod.getDeclaringClass().getClassLoader());
-
- // this should pass:
- Expression expr = compile("bar()", functions, childLoader);
- assertEquals(2.0, expr.evaluate(null), DELTA);
-
- // use our classloader, not the foreign one, which should fail!
- IllegalArgumentException expected =
- expectThrows(
- IllegalArgumentException.class,
- () -> {
- compile("bar()", functions, thisLoader);
- });
- assertTrue(
- expected
- .getMessage()
- .contains(
- "is not declared by a class which is accessible by the given parent ClassLoader"));
-
- // mix foreign and default functions
- Map<String, Method> mixedFunctions = new HashMap<>(JavascriptCompiler.DEFAULT_FUNCTIONS);
- mixedFunctions.putAll(functions);
- expr = compile("bar()", mixedFunctions, childLoader);
- assertEquals(2.0, expr.evaluate(null), DELTA);
- expr = compile("sqrt(20)", mixedFunctions, childLoader);
- assertEquals(Math.sqrt(20), expr.evaluate(null), DELTA);
-
- // use our classloader, not the foreign one, which should fail!
- expected =
- expectThrows(
- IllegalArgumentException.class,
- () -> {
- compile("bar()", mixedFunctions, thisLoader);
- });
- assertTrue(
- expected
- .getMessage()
- .contains(
- "is not declared by a class which is accessible by the given parent ClassLoader"));
+ /** class containing method is not public */
+ public void testNonDirectMethodHandle() throws Exception {
+ Map<String, MethodHandle> functions =
+ Map.of(
+ "foo",
+ MethodHandles.constant(double.class, 9),
+ "bar",
+ MethodHandles.identity(double.class));
+ Expression expr = compile("foo() + bar(7)", functions);
+ assertEquals(16, expr.evaluate(null), DELTA);
}
static String MESSAGE = "This should not happen but it happens";
- public static class StaticThrowingException {
- public static double method() {
- throw new ArithmeticException(MESSAGE);
- }
+ public static double staticThrowingException() {
+ throw new ArithmeticException(MESSAGE);
}
/**
@@ -336,8 +249,7 @@ public class TestCustomFunctions extends CompilerTestCase {
* code of the expression as file name.
*/
public void testThrowingException() throws Exception {
- Map<String, Method> functions = new HashMap<>();
- functions.put("foo", StaticThrowingException.class.getMethod("method"));
+ Map<String, MethodHandle> functions = Map.of("foo", localMethod("staticThrowingException", 0));
String source = "3 * foo() / 5";
Expression expr = compile(source, functions);
ArithmeticException expected =
@@ -351,16 +263,49 @@ public class TestCustomFunctions extends CompilerTestCase {
PrintWriter pw = new PrintWriter(sw);
expected.printStackTrace(pw);
pw.flush();
- assertTrue(
- sw.toString().contains("JavascriptCompiler$CompiledExpression.evaluate(" + source + ")"));
+ String exceptionText = sw.toString();
+ if (LuceneTestCase.VERBOSE) {
+ System.err.println("Exception thrown with full stacktrace:");
+ System.err.println(exceptionText);
+ }
+ assertTrue(exceptionText.contains(expr.getClass().getName() + ".evaluate(" + source + ")"));
}
- /** test that namespaces work with custom expressions. */
- public void testNamespaces() throws Exception {
- Map<String, Method> functions = new HashMap<>();
- functions.put("foo.bar", getClass().getMethod("zeroArgMethod"));
+ /** test that namespaces work with custom expressions as direct method handle. */
+ public void testNamespacesWithDirectMH() throws Exception {
+ Map<String, MethodHandle> functions = Map.of("foo.bar", localMethod("zeroArgMethod", 0));
String source = "foo.bar()";
Expression expr = compile(source, functions);
assertEquals(5, expr.evaluate(null), DELTA);
}
+
+ /**
+ * test that namespaces work with general method handles (ensure field name of handle is correct).
+ */
+ public void testNamespacesWithoutDirectMH() throws Exception {
+ Map<String, MethodHandle> functions =
+ Map.of(
+ "foo.bar",
+ MethodHandles.constant(double.class, 9),
+ "bar.foo",
+ MethodHandles.identity(double.class));
+ Expression expr = compile("foo.bar() + bar.foo(7)", functions);
+ assertEquals(16, expr.evaluate(null), DELTA);
+ }
+
+ public void testLegacyFunctions() throws Exception {
+ var functions =
+ Map.of("foo", TestCustomFunctions.class.getMethod("oneArgMethod", double.class));
+ var newFunctions = JavascriptCompiler.convertLegacyFunctions(functions);
+ newFunctions.putAll(JavascriptCompiler.DEFAULT_FUNCTIONS);
+ Expression expr = compile("foo(3) + abs(-7)", newFunctions);
+ assertEquals(13, expr.evaluate(null), DELTA);
+ }
+
+ public void testInvalidLegacyFunctions() throws Exception {
+ var functions = Map.of("foo", TestCustomFunctions.class.getMethod("nonStaticMethod"));
+ var newFunctions = JavascriptCompiler.convertLegacyFunctions(functions);
+ newFunctions.putAll(JavascriptCompiler.DEFAULT_FUNCTIONS);
+ expectThrows(IllegalArgumentException.class, () -> compile("foo(3) + abs(-7)", newFunctions));
+ }
}
diff --git a/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestJavascriptCompiler.java b/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestJavascriptCompiler.java
index 69eab4cdfc1..402236d2e09 100644
--- a/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestJavascriptCompiler.java
+++ b/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestJavascriptCompiler.java
@@ -33,15 +33,7 @@ public class TestJavascriptCompiler extends CompilerTestCase {
expectThrows(
NullPointerException.class,
() -> {
- JavascriptCompiler.compile("100", null, getClass().getClassLoader());
- });
- }
-
- public void testNullLoader() throws Exception {
- expectThrows(
- NullPointerException.class,
- () -> {
- JavascriptCompiler.compile("100", JavascriptCompiler.DEFAULT_FUNCTIONS, null);
+ JavascriptCompiler.compile("100", null);
});
}
diff --git a/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestJavascriptFunction.java b/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestJavascriptFunction.java
index dbc7c046f01..577dc99a8f1 100644
--- a/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestJavascriptFunction.java
+++ b/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestJavascriptFunction.java
@@ -264,4 +264,9 @@ public class TestJavascriptFunction extends CompilerTestCase {
assertEvaluatesTo("tanh(-12.3456789)", -0.99999999996);
assertEvaluatesTo("tanh(12.3456789)", 0.99999999996);
}
+
+ /** checks that dynamic constants work and only produce one entry in constant pool */
+ public void testSameFunction() throws Exception {
+ assertEvaluatesTo("sqrt(49) * sqrt(25)", 7 * 5);
+ }
}
diff --git a/lucene/licenses/asm-7.2.jar.sha1 b/lucene/licenses/asm-7.2.jar.sha1
deleted file mode 100644
index ccb16fac6a0..00000000000
--- a/lucene/licenses/asm-7.2.jar.sha1
+++ /dev/null
@@ -1 +0,0 @@
-fa637eb67eb7628c915d73762b681ae7ff0b9731
diff --git a/lucene/licenses/asm-9.6.jar.sha1 b/lucene/licenses/asm-9.6.jar.sha1
new file mode 100644
index 00000000000..33936b0d06a
--- /dev/null
+++ b/lucene/licenses/asm-9.6.jar.sha1
@@ -0,0 +1 @@
+aa205cf0a06dbd8e04ece91c0b37c3f5d567546a
diff --git a/lucene/licenses/asm-analysis-7.2.jar.sha1 b/lucene/licenses/asm-analysis-7.2.jar.sha1
deleted file mode 100644
index b0a8b7b83ed..00000000000
--- a/lucene/licenses/asm-analysis-7.2.jar.sha1
+++ /dev/null
@@ -1 +0,0 @@
-b6e6abe057f23630113f4167c34bda7086691258
diff --git a/lucene/licenses/asm-commons-7.2.jar.sha1 b/lucene/licenses/asm-commons-7.2.jar.sha1
deleted file mode 100644
index 3db5daccb9b..00000000000
--- a/lucene/licenses/asm-commons-7.2.jar.sha1
+++ /dev/null
@@ -1 +0,0 @@
-ca2954e8d92a05bacc28ff465b25c70e0f512497
diff --git a/lucene/licenses/asm-commons-9.6.jar.sha1 b/lucene/licenses/asm-commons-9.6.jar.sha1
new file mode 100644
index 00000000000..4e0a1be573a
--- /dev/null
+++ b/lucene/licenses/asm-commons-9.6.jar.sha1
@@ -0,0 +1 @@
+f1a9e5508eff490744144565c47326c8648be309
diff --git a/lucene/licenses/asm-tree-7.2.jar.sha1 b/lucene/licenses/asm-tree-7.2.jar.sha1
deleted file mode 100644
index 077d6f9c411..00000000000
--- a/lucene/licenses/asm-tree-7.2.jar.sha1
+++ /dev/null
@@ -1 +0,0 @@
-3a23cc36edaf8fc5a89cb100182758ccb5991487
diff --git a/lucene/licenses/asm-tree-9.6.jar.sha1 b/lucene/licenses/asm-tree-9.6.jar.sha1
new file mode 100644
index 00000000000..b6c20f05696
--- /dev/null
+++ b/lucene/licenses/asm-tree-9.6.jar.sha1
@@ -0,0 +1 @@
+c0cdda9d211e965d2a4448aa3fd86110f2f8c2de
diff --git a/lucene/licenses/opennlp-tools-1.9.4.jar.sha1 b/lucene/licenses/opennlp-tools-1.9.4.jar.sha1
index b367a761a44..fa7e85dd48f 100644
--- a/lucene/licenses/opennlp-tools-1.9.4.jar.sha1
+++ b/lucene/licenses/opennlp-tools-1.9.4.jar.sha1
@@ -1 +1 @@
-dd5c4a6d82453bcccb78ba4ac90f166366dde12b
\ No newline at end of file
+dd5c4a6d82453bcccb78ba4ac90f166366dde12b
diff --git a/versions.lock b/versions.lock
index 40a53de1722..b532be08686 100644
--- a/versions.lock
+++ b/versions.lock
@@ -17,10 +17,9 @@ org.carrot2:morfologik-stemming:2.1.9 (2 constraints: 1312040d)
org.hamcrest:hamcrest:2.2 (1 constraints: a8041f2c)
org.locationtech.spatial4j:spatial4j:0.8 (1 constraints: ac041f2c)
org.openjdk.jmh:jmh-core:1.37 (1 constraints: df04fc30)
-org.ow2.asm:asm:7.2 (3 constraints: 2717d96b)
-org.ow2.asm:asm-analysis:7.2 (1 constraints: e409d9a5)
-org.ow2.asm:asm-commons:7.2 (1 constraints: ad042e2c)
-org.ow2.asm:asm-tree:7.2 (2 constraints: 2f14468c)
+org.ow2.asm:asm:9.6 (3 constraints: 3917ef6d)
+org.ow2.asm:asm-commons:9.6 (1 constraints: b304382c)
+org.ow2.asm:asm-tree:9.6 (1 constraints: ea09e3a5)
ua.net.nlp:morfologik-ukrainian-search:4.9.1 (1 constraints: 10051b36)
xerces:xercesImpl:2.12.0 (1 constraints: 3705353b)
diff --git a/versions.props b/versions.props
index 9d8540930ab..ed49d7065cf 100644
--- a/versions.props
+++ b/versions.props
@@ -15,6 +15,6 @@ org.carrot2:morfologik-*=2.1.9
org.hamcrest:*=2.2
org.locationtech.jts:jts-core=1.17.0
org.locationtech.spatial4j:*=0.8
-org.ow2.asm:*=7.2
+org.ow2.asm:*=9.6
ua.net.nlp:morfologik-ukrainian-search=4.9.1
xerces:xercesImpl=2.12.0