Expression Goodie Bag (#553)

* Remove async expression eval. Implement timeout inline

* Remove static state from expr functions

* Remove now-unused TL stack

* Rework some expr handles

(cherry picked from commit 6bc1d4647cc6892ae4dca9fc0e2d239777903c38)
This commit is contained in:
Octavia Togami 2020-02-21 00:17:49 -08:00 committed by MattBDev
parent 88a5806b0f
commit e8bc0c0e1f
9 changed files with 131 additions and 170 deletions

View File

@ -19,9 +19,7 @@
package com.sk89q.worldedit.internal.expression;
import com.google.common.collect.SetMultimap;
import java.lang.invoke.MethodHandle;
import java.time.Instant;
import static java.util.Objects.requireNonNull;
@ -31,21 +29,33 @@ public class ExecutionData {
* Special execution context for evaluating constant values. As long as no variables are used,
* it can be considered constant.
*/
public static final ExecutionData CONSTANT_EVALUATOR = new ExecutionData(null, null);
public static final ExecutionData CONSTANT_EVALUATOR = new ExecutionData(null, null, Instant.MAX);
private final SlotTable slots;
private final SetMultimap<String, MethodHandle> functions;
private final Functions functions;
private final Instant deadline;
public ExecutionData(SlotTable slots, SetMultimap<String, MethodHandle> functions) {
public ExecutionData(SlotTable slots, Functions functions, Instant deadline) {
this.slots = slots;
this.functions = functions;
this.deadline = deadline;
}
public SlotTable getSlots() {
return requireNonNull(slots, "Cannot use variables in a constant");
}
public SetMultimap<String, MethodHandle> getFunctions() {
public Functions getFunctions() {
return requireNonNull(functions, "Cannot use functions in a constant");
}
public Instant getDeadline() {
return deadline;
}
public void checkDeadline() {
if (Instant.now().isAfter(deadline)) {
throw new ExpressionTimeoutException("Calculations exceeded time limit.");
}
}
}

View File

@ -19,32 +19,20 @@
package com.sk89q.worldedit.internal.expression;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.SetMultimap;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.sk89q.worldedit.WorldEdit;
import com.sk89q.worldedit.antlr.ExpressionLexer;
import com.sk89q.worldedit.antlr.ExpressionParser;
import com.sk89q.worldedit.internal.expression.invoke.ExpressionCompiler;
import com.sk89q.worldedit.session.request.Request;
import org.antlr.v4.runtime.CharStream;
import org.antlr.v4.runtime.CharStreams;
import org.antlr.v4.runtime.CommonTokenStream;
import org.antlr.v4.runtime.misc.ParseCancellationException;
import org.antlr.v4.runtime.tree.ParseTreeWalker;
import java.lang.invoke.MethodHandle;
import java.time.Instant;
import java.util.List;
import java.util.Objects;
import java.util.Stack;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
/**
* Compiles and evaluates expressions.
@ -74,19 +62,10 @@ import java.util.concurrent.TimeoutException;
*/
public class Expression {
private static final ThreadLocal<Stack<Expression>> instance = new ThreadLocal<>();
private static final ExecutorService evalThread = Executors.newFixedThreadPool(
Runtime.getRuntime().availableProcessors(),
new ThreadFactoryBuilder()
.setDaemon(true)
.setNameFormat("worldedit-expression-eval-%d")
.build());
private final SlotTable slots = new SlotTable();
private final List<String> providedSlots;
private final ExpressionParser.AllStatementsContext root;
private final SetMultimap<String, MethodHandle> functions = Functions.getFunctionMap();
private ExpressionEnvironment environment;
private final Functions functions = Functions.create();
private final CompiledExpression compiledExpression;
public static Expression compile(String expression, String... variableNames) throws ExpressionException {
@ -138,52 +117,9 @@ public class Expression {
slot.setValue(values[i]);
}
Instant deadline = Instant.now().plusMillis(timeout);
// evaluation exceptions are thrown out of this method
if (timeout < 0) {
return evaluateRoot();
}
return evaluateRootTimed(timeout);
}
private double evaluateRootTimed(int timeout) throws EvaluationException {
CountDownLatch startLatch = new CountDownLatch(1);
Request request = Request.request();
Future<Double> result = evalThread.submit(() -> {
Request local = Request.request();
local.setSession(request.getSession());
local.setWorld(request.getWorld());
local.setEditSession(request.getEditSession());
try {
startLatch.countDown();
return Expression.this.evaluateRoot();
} finally {
Request.reset();
}
});
try {
startLatch.await();
return result.get(timeout, TimeUnit.MILLISECONDS);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new RuntimeException(e);
} catch (TimeoutException e) {
result.cancel(true);
throw new ExpressionTimeoutException("Calculations exceeded time limit.");
} catch (ExecutionException e) {
Throwable cause = e.getCause();
Throwables.throwIfInstanceOf(cause, EvaluationException.class);
Throwables.throwIfUnchecked(cause);
throw new RuntimeException(cause);
}
}
private Double evaluateRoot() throws EvaluationException {
pushInstance();
try {
return compiledExpression.execute(new ExecutionData(slots, functions));
} finally {
popInstance();
}
return compiledExpression.execute(new ExecutionData(slots, functions, deadline));
}
public void optimize() {
@ -199,35 +135,12 @@ public class Expression {
return slots;
}
public static Expression getInstance() {
return instance.get().peek();
}
private void pushInstance() {
Stack<Expression> threadLocalExprStack = instance.get();
if (threadLocalExprStack == null) {
instance.set(threadLocalExprStack = new Stack<>());
}
threadLocalExprStack.push(this);
}
private void popInstance() {
Stack<Expression> threadLocalExprStack = instance.get();
threadLocalExprStack.pop();
if (threadLocalExprStack.isEmpty()) {
instance.set(null);
}
}
public ExpressionEnvironment getEnvironment() {
return environment;
return functions.getEnvironment();
}
public void setEnvironment(ExpressionEnvironment environment) {
this.environment = environment;
functions.setEnvironment(environment);
}
}

View File

@ -19,7 +19,6 @@
package com.sk89q.worldedit.internal.expression;
import com.google.common.collect.SetMultimap;
import com.sk89q.worldedit.antlr.ExpressionParser;
import org.antlr.v4.runtime.ParserRuleContext;
import org.antlr.v4.runtime.Token;
@ -60,16 +59,10 @@ public class ExpressionHelper {
check(iterations <= 256, ctx, "Loop exceeded 256 iterations");
}
public static void checkTimeout() {
if (Thread.interrupted()) {
throw new ExpressionTimeoutException("Calculations exceeded time limit.");
}
}
public static MethodHandle resolveFunction(SetMultimap<String, MethodHandle> functions,
public static MethodHandle resolveFunction(Functions functions,
ExpressionParser.FunctionCallContext ctx) {
String fnName = ctx.name.getText();
Set<MethodHandle> matchingFns = functions.get(fnName);
Set<MethodHandle> matchingFns = functions.getMap().get(fnName);
check(!matchingFns.isEmpty(), ctx, "Unknown function '" + fnName + "'");
for (MethodHandle function : matchingFns) {
MethodType type = function.type();

View File

@ -19,11 +19,9 @@
package com.sk89q.worldedit.internal.expression;
import com.google.common.collect.SetMultimap;
import com.sk89q.worldedit.antlr.ExpressionBaseListener;
import com.sk89q.worldedit.antlr.ExpressionParser;
import java.lang.invoke.MethodHandle;
import java.util.Collection;
import java.util.HashSet;
import java.util.Set;
@ -34,10 +32,10 @@ import static com.sk89q.worldedit.internal.expression.ExpressionHelper.resolveFu
class ExpressionValidator extends ExpressionBaseListener {
private final Set<String> variableNames = new HashSet<>();
private final SetMultimap<String, MethodHandle> functions;
private final Functions functions;
ExpressionValidator(Collection<String> variableNames,
SetMultimap<String, MethodHandle> functions) {
Functions functions) {
this.variableNames.addAll(variableNames);
this.functions = functions;
}

View File

@ -44,25 +44,10 @@ import static java.lang.invoke.MethodType.methodType;
/**
* Contains all functions that can be used in expressions.
*/
final class Functions {
public final class Functions {
static SetMultimap<String, MethodHandle> getFunctionMap() {
SetMultimap<String, MethodHandle> map = HashMultimap.create();
Functions functions = new Functions();
MethodHandles.Lookup lookup = MethodHandles.lookup();
try {
addMathHandles(map, lookup);
addStaticFunctionHandles(map, lookup);
functions.addInstanceFunctionHandles(map, lookup);
} catch (NoSuchMethodException | IllegalAccessException e) {
throw new IllegalStateException(e);
}
// clean up all the functions
return ImmutableSetMultimap.copyOf(
Multimaps.transformValues(map, Functions::clean)
);
static Functions create() {
return new Functions();
}
private static final MethodHandle DOUBLE_VALUE;
@ -149,15 +134,6 @@ final class Functions {
map.put("ridgedmulti", lookup.findStatic(Functions.class, "ridgedmulti",
methodType(double.class, double.class, double.class, double.class, double.class,
double.class, double.class)));
map.put("query", lookup.findStatic(Functions.class, "query",
methodType(double.class, double.class, double.class, double.class, LocalSlot.class,
LocalSlot.class)));
map.put("queryAbs", lookup.findStatic(Functions.class, "queryAbs",
methodType(double.class, double.class, double.class, double.class, LocalSlot.class,
LocalSlot.class)));
map.put("queryRel", lookup.findStatic(Functions.class, "queryRel",
methodType(double.class, double.class, double.class, double.class, LocalSlot.class,
LocalSlot.class)));
}
private void addInstanceFunctionHandles(
@ -174,6 +150,20 @@ final class Functions {
methodType(double.class, double.class, double.class, double.class, double.class,
double.class, double.class), Functions.class)
.bindTo(this));
// rely on expression field
map.put("query", lookup.findSpecial(Functions.class, "query",
methodType(double.class, double.class, double.class, double.class, LocalSlot.class,
LocalSlot.class), Functions.class)
.bindTo(this));
map.put("queryAbs", lookup.findSpecial(Functions.class, "queryAbs",
methodType(double.class, double.class, double.class, double.class, LocalSlot.class,
LocalSlot.class), Functions.class)
.bindTo(this));
map.put("queryRel", lookup.findSpecial(Functions.class, "queryRel",
methodType(double.class, double.class, double.class, double.class, LocalSlot.class,
LocalSlot.class), Functions.class)
.bindTo(this));
}
private static double rotate(Variable x, Variable y, double angle) {
@ -201,6 +191,35 @@ final class Functions {
private static final Int2ObjectMap<double[]> globalMegaBuffer = new Int2ObjectOpenHashMap<>();
private final Int2ObjectMap<double[]> megaBuffer = new Int2ObjectOpenHashMap<>();
private final SetMultimap<String, MethodHandle> map;
private ExpressionEnvironment environment;
private Functions() {
MethodHandles.Lookup lookup = MethodHandles.lookup();
SetMultimap<String, MethodHandle> map = HashMultimap.create();
try {
addMathHandles(map, lookup);
addStaticFunctionHandles(map, lookup);
addInstanceFunctionHandles(map, lookup);
} catch (NoSuchMethodException | IllegalAccessException e) {
throw new IllegalStateException(e);
}
this.map = ImmutableSetMultimap.copyOf(
Multimaps.transformValues(map, Functions::clean)
);
}
public SetMultimap<String, MethodHandle> getMap() {
return map;
}
public ExpressionEnvironment getEnvironment() {
return environment;
}
public void setEnvironment(ExpressionEnvironment environment) {
this.environment = environment;
}
private static double[] getSubBuffer(Int2ObjectMap<double[]> megabuf, int key) {
return megabuf.computeIfAbsent(key, k -> new double[1024]);
@ -332,9 +351,7 @@ final class Functions {
return ret;
}
private static double query(double x, double y, double z, LocalSlot type, LocalSlot data) {
final ExpressionEnvironment environment = Expression.getInstance().getEnvironment();
private double query(double x, double y, double z, LocalSlot type, LocalSlot data) {
// Read values from world
final double typeId = environment.getBlockType(x, y, z);
final double dataValue = environment.getBlockData(x, y, z);
@ -342,9 +359,7 @@ final class Functions {
return queryInternal(type, data, typeId, dataValue);
}
private static double queryAbs(double x, double y, double z, LocalSlot type, LocalSlot data) {
final ExpressionEnvironment environment = Expression.getInstance().getEnvironment();
private double queryAbs(double x, double y, double z, LocalSlot type, LocalSlot data) {
// Read values from world
final double typeId = environment.getBlockTypeAbs(x, y, z);
final double dataValue = environment.getBlockDataAbs(x, y, z);
@ -352,9 +367,7 @@ final class Functions {
return queryInternal(type, data, typeId, dataValue);
}
private static double queryRel(double x, double y, double z, LocalSlot type, LocalSlot data) {
final ExpressionEnvironment environment = Expression.getInstance().getEnvironment();
private double queryRel(double x, double y, double z, LocalSlot type, LocalSlot data) {
// Read values from world
final double typeId = environment.getBlockTypeRel(x, y, z);
final double dataValue = environment.getBlockDataRel(x, y, z);
@ -362,7 +375,4 @@ final class Functions {
return queryInternal(type, data, typeId, dataValue);
}
private Functions() {
}
}

View File

@ -19,13 +19,13 @@
package com.sk89q.worldedit.internal.expression.invoke;
import com.google.common.collect.SetMultimap;
import com.sk89q.worldedit.antlr.ExpressionBaseVisitor;
import com.sk89q.worldedit.antlr.ExpressionParser;
import com.sk89q.worldedit.internal.expression.BreakException;
import com.sk89q.worldedit.internal.expression.EvaluationException;
import com.sk89q.worldedit.internal.expression.ExecutionData;
import com.sk89q.worldedit.internal.expression.ExpressionHelper;
import com.sk89q.worldedit.internal.expression.Functions;
import com.sk89q.worldedit.internal.expression.LocalSlot;
import it.unimi.dsi.fastutil.doubles.Double2ObjectLinkedOpenHashMap;
import it.unimi.dsi.fastutil.doubles.Double2ObjectMap;
@ -73,6 +73,7 @@ import static com.sk89q.worldedit.internal.expression.invoke.ExpressionHandles.D
import static com.sk89q.worldedit.internal.expression.invoke.ExpressionHandles.IS_NULL;
import static com.sk89q.worldedit.internal.expression.invoke.ExpressionHandles.NEW_LS_CONSTANT;
import static com.sk89q.worldedit.internal.expression.invoke.ExpressionHandles.NULL_DOUBLE;
import static com.sk89q.worldedit.internal.expression.invoke.ExpressionHandles.unboxDoubles;
import static java.lang.invoke.MethodType.methodType;
/**
@ -86,9 +87,9 @@ class CompilingVisitor extends ExpressionBaseVisitor<MethodHandle> {
* (ExecutionData)Double, with a few as (ExecutionData,Double)Double where it needs an existing
* value passed in. EVERY handle returned from an overriden method must be of the first type.
*/
private final SetMultimap<String, MethodHandle> functions;
private final Functions functions;
CompilingVisitor(SetMultimap<String, MethodHandle> functions) {
CompilingVisitor(Functions functions) {
this.functions = functions;
}
@ -136,7 +137,6 @@ class CompilingVisitor extends ExpressionBaseVisitor<MethodHandle> {
private MethodHandle evaluateBoolean(ParserRuleContext boolExpression) {
MethodHandle value = evaluateForNamedValue(boolExpression, "a boolean");
value = value.asType(value.type().unwrap());
// Pass `value` into converter, returns (ExecutionData)boolean;
return MethodHandles.collectArguments(
DOUBLE_TO_BOOL, 0, value
@ -340,13 +340,13 @@ class CompilingVisitor extends ExpressionBaseVisitor<MethodHandle> {
// Inject left as primary condition, on failure take right with data parameter
// logic = (Double,ExecutionData)Double
MethodHandle logic = MethodHandles.guardWithTest(
// data arg dropped implicitly
// data arg dropped implicitly -- (Double)boolean;
DOUBLE_TO_BOOL,
// drop data arg
// drop data arg -- (Double,ExecutionData)Double;
MethodHandles.dropArguments(
MethodHandles.identity(Double.class), 1, ExecutionData.class
),
// drop left arg, call right
// drop left arg, call right -- (Double,ExecutionData)Double;
MethodHandles.dropArguments(
right, 0, Double.class
)
@ -367,9 +367,8 @@ class CompilingVisitor extends ExpressionBaseVisitor<MethodHandle> {
// Map two data args to two double args, then evaluate op
MethodHandle doubleData = MethodHandles.filterArguments(
CALL_BINARY_OP.bindTo(op), 0,
mhLeft.asType(mhLeft.type().unwrap()), mhRight.asType(mhRight.type().unwrap())
unboxDoubles(mhLeft), unboxDoubles(mhRight)
);
doubleData = doubleData.asType(doubleData.type().wrap());
return ExpressionHandles.dedupData(doubleData);
}

View File

@ -19,9 +19,9 @@
package com.sk89q.worldedit.internal.expression.invoke;
import com.google.common.collect.SetMultimap;
import com.sk89q.worldedit.antlr.ExpressionParser;
import com.sk89q.worldedit.internal.expression.CompiledExpression;
import com.sk89q.worldedit.internal.expression.Functions;
import java.lang.invoke.LambdaConversionException;
import java.lang.invoke.LambdaMetafactory;
@ -66,7 +66,7 @@ public class ExpressionCompiler {
}
public CompiledExpression compileExpression(ExpressionParser.AllStatementsContext root,
SetMultimap<String, MethodHandle> functions) {
Functions functions) {
MethodHandle invokable = root.accept(new CompilingVisitor(functions));
return (CompiledExpression) safeInvoke(
HANDLE_TO_CE_CONVERTER, h -> h.invoke(invokable)

View File

@ -38,10 +38,10 @@ import java.lang.invoke.MethodType;
import java.util.Objects;
import java.util.function.DoubleBinaryOperator;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import static com.sk89q.worldedit.internal.expression.ExpressionHelper.check;
import static com.sk89q.worldedit.internal.expression.ExpressionHelper.checkIterations;
import static com.sk89q.worldedit.internal.expression.ExpressionHelper.checkTimeout;
import static com.sk89q.worldedit.internal.expression.ExpressionHelper.getErrorPosition;
import static java.lang.invoke.MethodHandles.collectArguments;
import static java.lang.invoke.MethodHandles.constant;
@ -63,8 +63,11 @@ class ExpressionHandles {
private static final MethodHandle SIMPLE_FOR_LOOP_IMPL;
private static final MethodHandle SWITCH_IMPL;
// (Object)boolean;
static final MethodHandle IS_NULL;
// (Double)boolean;
static final MethodHandle DOUBLE_TO_BOOL;
// (double, double)Double;
static final MethodHandle CALL_BINARY_OP;
static final MethodHandle NEW_LS_CONSTANT;
@ -95,10 +98,11 @@ class ExpressionHandles {
IS_NULL = lookup.findStatic(Objects.class, "isNull",
methodType(boolean.class, Object.class));
DOUBLE_TO_BOOL = lookup.findStatic(ExpressionHandles.class, "doubleToBool",
methodType(boolean.class, double.class));
DOUBLE_TO_BOOL = boxDoubles(lookup.findStatic(ExpressionHandles.class, "doubleToBool",
methodType(boolean.class, double.class)));
CALL_BINARY_OP = lookup.findVirtual(DoubleBinaryOperator.class, "applyAsDouble",
methodType(double.class, double.class, double.class));
methodType(double.class, double.class, double.class))
.asType(methodType(Double.class, DoubleBinaryOperator.class, double.class, double.class));
NEW_LS_CONSTANT = lookup.findConstructor(LocalSlot.Constant.class,
methodType(void.class, double.class));
} catch (NoSuchMethodException | IllegalAccessException e) {
@ -106,6 +110,34 @@ class ExpressionHandles {
}
}
static MethodHandle boxDoubles(MethodHandle handle) {
MethodType type = handle.type();
type = methodType(
boxIfPrimitiveDouble(type.returnType()),
type.parameterList().stream().map(ExpressionHandles::boxIfPrimitiveDouble)
.collect(Collectors.toList())
);
return handle.asType(type);
}
private static Class<?> boxIfPrimitiveDouble(Class<?> clazz) {
return clazz == double.class ? Double.class : clazz;
}
static MethodHandle unboxDoubles(MethodHandle handle) {
MethodType type = handle.type();
type = methodType(
unboxIfDouble(type.returnType()),
type.parameterList().stream().map(ExpressionHandles::unboxIfDouble)
.collect(Collectors.toList())
);
return handle.asType(type);
}
private static Class<?> unboxIfDouble(Class<?> clazz) {
return clazz == Double.class ? double.class : clazz;
}
@FunctionalInterface
interface Invokable {
Object invoke(MethodHandle handle) throws Throwable;
@ -237,7 +269,7 @@ class ExpressionHandles {
}
while ((boolean) standardInvoke(condition, data)) {
checkIterations(iterations, body.ctx);
checkTimeout();
data.checkDeadline();
iterations++;
try {
result = (Double) standardInvoke(body.handle, data);
@ -264,7 +296,7 @@ class ExpressionHandles {
int iterations = 0;
do {
checkIterations(iterations, body.ctx);
checkTimeout();
data.checkDeadline();
iterations++;
try {
result = (Double) standardInvoke(body.handle, data);
@ -297,7 +329,7 @@ class ExpressionHandles {
LocalSlot.Variable variable = initVariable(data, counterToken);
for (double i = first; i <= last; i++) {
checkIterations(iterations, body.ctx);
checkTimeout();
data.checkDeadline();
iterations++;
variable.setValue(i);
try {

View File

@ -47,6 +47,12 @@ class ExpressionTest extends BaseExpressionTest {
// check variables
assertEquals(8, compile("foo+bar", "foo", "bar").evaluate(5D, 3D), 0);
// check conditionals
assertEquals(5, simpleEval("0 || 5"), 0);
assertEquals(2, simpleEval("2 || 5"), 0);
assertEquals(5, simpleEval("2 && 5"), 0);
assertEquals(0, simpleEval("5 && 0"), 0);
}
@Test