From 9fcaba7d727533158385c255d123076636b83716 Mon Sep 17 00:00:00 2001 From: Nate Bauernfeind Date: Mon, 12 Feb 2024 14:46:11 -0700 Subject: [PATCH] PythonScopeJpyImpl: Disable Conversion Cache --- .../python/PythonDeephavenSession.java | 27 +++-- .../python/PythonObjectWrapper.java | 5 + .../engine/context/EmptyQueryScope.java | 3 +- .../engine/context/PoisonedQueryScope.java | 3 +- .../deephaven/engine/context/QueryScope.java | 35 ++++-- .../engine/context/QueryScopeParam.java | 2 - .../engine/context/StandaloneQueryScope.java | 15 ++- .../table/impl/lang/QueryLanguageParser.java | 110 +++++++++++++----- .../impl/select/AbstractConditionFilter.java | 53 +++++---- .../impl/select/AbstractFormulaColumn.java | 17 +-- .../table/impl/select/DhFormulaColumn.java | 2 +- .../impl/select/codegen/FormulaAnalyzer.java | 49 ++++---- .../select/python/FormulaColumnPython.java | 2 +- .../engine/util/AbstractScriptSession.java | 16 +-- .../engine/util/GroovyDeephavenSession.java | 7 +- .../util/NoLanguageDeephavenSession.java | 13 ++- .../engine/util/PyCallableWrapperJpyImpl.java | 5 + .../io/deephaven/engine/util/PythonScope.java | 30 ----- .../engine/util/PythonScopeJpyImpl.java | 77 ++++++------ .../deephaven/engine/util/ScriptSession.java | 2 +- .../impl/lang/TestQueryLanguageParser.java | 16 ++- .../python/PythonGlobalScopeCopyModule.java | 4 +- .../python/PythonGlobalScopeModule.java | 4 +- .../python/PythonImportInitializer.java | 17 +++ .../io/deephaven/api/util/NameValidator.java | 6 +- 25 files changed, 316 insertions(+), 204 deletions(-) create mode 100644 server/src/main/java/io/deephaven/server/console/python/PythonImportInitializer.java diff --git a/Integrations/src/main/java/io/deephaven/integrations/python/PythonDeephavenSession.java b/Integrations/src/main/java/io/deephaven/integrations/python/PythonDeephavenSession.java index 9ca382093ad..9e607ba4729 100644 --- a/Integrations/src/main/java/io/deephaven/integrations/python/PythonDeephavenSession.java +++ b/Integrations/src/main/java/io/deephaven/integrations/python/PythonDeephavenSession.java @@ -41,9 +41,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Map; -import java.util.Objects; -import java.util.Set; +import java.util.*; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.function.Consumer; @@ -301,10 +299,23 @@ protected synchronized Object setVariable(String name, @Nullable Object newValue } @Override - protected Map getAllValues() { - return PyLib - .ensureGil(() -> scope.getEntries() - .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue))); + protected Map getAllValues(@NotNull final Predicate> predicate) { + final HashMap result = PyLib.ensureGil( + () -> scope.getEntriesRaw().>map( + e -> new AbstractMap.SimpleImmutableEntry<>(scope.convertStringKey(e.getKey()), e.getValue())) + .collect(HashMap::new, (map, entry) -> map.put(entry.getKey(), entry.getValue()), + HashMap::putAll)); + + final Iterator> iter = result.entrySet().iterator(); + while (iter.hasNext()) { + final Map.Entry entry = iter.next(); + entry.setValue(scope.convertValue((PyObject) entry.getValue())); + if (!predicate.test(entry)) { + iter.remove(); + } + } + + return result; } @Override @@ -315,7 +326,7 @@ public String scriptType() { // TODO core#41 move this logic into the python console instance or scope like this - can go further and move // isWidget too @Override - public Object unwrapObject(Object object) { + public Object unwrapObject(@Nullable Object object) { if (object instanceof PyObject) { final PyObject pyObject = (PyObject) object; final Object unwrapped = module.javaify(pyObject); diff --git a/Integrations/src/main/java/io/deephaven/integrations/python/PythonObjectWrapper.java b/Integrations/src/main/java/io/deephaven/integrations/python/PythonObjectWrapper.java index 302f2dce34f..54539ba316e 100644 --- a/Integrations/src/main/java/io/deephaven/integrations/python/PythonObjectWrapper.java +++ b/Integrations/src/main/java/io/deephaven/integrations/python/PythonObjectWrapper.java @@ -13,6 +13,11 @@ public class PythonObjectWrapper { private static final PyModule PY_WRAPPER_MODULE = PyModule.importModule("deephaven._wrapper"); + /** + * Ensure that the class initializer runs. + */ + public static void init() {} + /** * Unwrap a Python object to return the wrapped Java object. * diff --git a/engine/context/src/main/java/io/deephaven/engine/context/EmptyQueryScope.java b/engine/context/src/main/java/io/deephaven/engine/context/EmptyQueryScope.java index d2b3eb7cca2..e465d487578 100644 --- a/engine/context/src/main/java/io/deephaven/engine/context/EmptyQueryScope.java +++ b/engine/context/src/main/java/io/deephaven/engine/context/EmptyQueryScope.java @@ -10,6 +10,7 @@ import java.util.Collections; import java.util.Map; import java.util.Set; +import java.util.function.Predicate; import java.util.stream.Stream; public class EmptyQueryScope implements QueryScope { @@ -48,7 +49,7 @@ public void putParam(String name, T value) { } @Override - public Map toMap() { + public Map toMap(@NotNull Predicate> predicate) { return Collections.emptyMap(); } diff --git a/engine/context/src/main/java/io/deephaven/engine/context/PoisonedQueryScope.java b/engine/context/src/main/java/io/deephaven/engine/context/PoisonedQueryScope.java index 4da6964287c..7acda65ad9e 100644 --- a/engine/context/src/main/java/io/deephaven/engine/context/PoisonedQueryScope.java +++ b/engine/context/src/main/java/io/deephaven/engine/context/PoisonedQueryScope.java @@ -10,6 +10,7 @@ import java.lang.ref.WeakReference; import java.util.Map; import java.util.Set; +import java.util.function.Predicate; import java.util.stream.Stream; public class PoisonedQueryScope implements QueryScope { @@ -53,7 +54,7 @@ public void putParam(String name, T value) { } @Override - public Map toMap() { + public Map toMap(@NotNull Predicate> predicate) { return fail(); } diff --git a/engine/context/src/main/java/io/deephaven/engine/context/QueryScope.java b/engine/context/src/main/java/io/deephaven/engine/context/QueryScope.java index 2ea696b3189..b9df1da15f7 100644 --- a/engine/context/src/main/java/io/deephaven/engine/context/QueryScope.java +++ b/engine/context/src/main/java/io/deephaven/engine/context/QueryScope.java @@ -6,12 +6,15 @@ import io.deephaven.engine.liveness.LivenessNode; import io.deephaven.base.log.LogOutput; import io.deephaven.base.log.LogOutputAppendable; +import io.deephaven.util.annotations.FinalDefault; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.Collection; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.function.Predicate; /** * Variable scope used to resolve parameter values during query execution and to expose named objects to users. Objects @@ -69,8 +72,8 @@ private MissingVariableException(final Throwable cause) { * @return A newly-constructed array of newly-constructed Params. * @throws QueryScope.MissingVariableException If any of the named scope variables does not exist. */ - default QueryScopeParam[] getParams(final Collection names) throws MissingVariableException { - final QueryScopeParam[] result = new QueryScopeParam[names.size()]; + default QueryScopeParam[] getParams(final Collection names) throws MissingVariableException { + final QueryScopeParam[] result = new QueryScopeParam[names.size()]; int pi = 0; for (final String name : names) { result[pi++] = createParam(name); @@ -131,12 +134,24 @@ default QueryScopeParam[] getParams(final Collection names) throws Missi void putParam(final String name, final T value); /** - * Returns an immutable map with all objects in the scope. Callers may want to unwrap language-specific values using - * {@link #unwrapObject(Object)} before using them. + * Returns a mutable map with all objects in the scope. Callers may want to unwrap language-specific values using + * {@link #unwrapObject(Object)} before using them. This map is owned by the caller and may be mutated. + * + * @param predicate a predicate to filter the map entries + * @return a caller-owned mutable map with all known variables and their values. + */ + Map toMap(@NotNull Predicate> predicate); + + /** + * Returns a mutable map with all objects in the scope. Callers may want to unwrap language-specific values using + * {@link #unwrapObject(Object)} before using them. This map is owned by the caller and may be mutated. * - * @return an immutable map with all known variables and their values. + * @return a caller-owned mutable map with all known variables and their values. */ - Map toMap(); + @FinalDefault + default Map toMap() { + return toMap(entry -> true); + } /** * Removes any wrapping that exists on a scope param object so that clients can fetch them. Defaults to returning @@ -145,16 +160,16 @@ default QueryScopeParam[] getParams(final Collection names) throws Missi * @param object the scoped object * @return an obj which can be consumed by a client */ - default Object unwrapObject(Object object) { + default Object unwrapObject(@Nullable Object object) { return object; } @Override default LogOutput append(@NotNull final LogOutput logOutput) { logOutput.append('{'); - for (final String paramName : getParamNames()) { - final Object paramValue = readParamValue(paramName); - logOutput.nl().append(paramName).append("="); + for (final Map.Entry param : toMap().entrySet()) { + logOutput.nl().append(param.getKey()).append("="); + Object paramValue = param.getValue(); if (paramValue == this) { logOutput.append("this QueryScope (" + paramValue.getClass().getName() + ':' + System.identityHashCode(paramValue) + ')'); diff --git a/engine/context/src/main/java/io/deephaven/engine/context/QueryScopeParam.java b/engine/context/src/main/java/io/deephaven/engine/context/QueryScopeParam.java index 55c311a526c..466fdcdcf49 100644 --- a/engine/context/src/main/java/io/deephaven/engine/context/QueryScopeParam.java +++ b/engine/context/src/main/java/io/deephaven/engine/context/QueryScopeParam.java @@ -5,8 +5,6 @@ public class QueryScopeParam { - public static final QueryScopeParam[] ZERO_LENGTH_PARAM_ARRAY = new QueryScopeParam[0]; - private final String name; private final T value; diff --git a/engine/context/src/main/java/io/deephaven/engine/context/StandaloneQueryScope.java b/engine/context/src/main/java/io/deephaven/engine/context/StandaloneQueryScope.java index 3461992aaa0..359b4c4bf33 100644 --- a/engine/context/src/main/java/io/deephaven/engine/context/StandaloneQueryScope.java +++ b/engine/context/src/main/java/io/deephaven/engine/context/StandaloneQueryScope.java @@ -6,10 +6,13 @@ import io.deephaven.engine.updategraph.DynamicNode; import io.deephaven.hash.KeyedObjectHashMap; import io.deephaven.hash.KeyedObjectKey; +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.jetbrains.annotations.NotNull; +import java.util.HashMap; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; +import java.util.function.Predicate; /** * Map-based implementation, extending LivenessArtifact to manage the objects passed into it. @@ -77,9 +80,13 @@ public void putParam(final String name, final T value) { } @Override - public Map toMap() { - return valueRetrievers.entrySet().stream() - .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, e -> e.getValue().value)); + public Map toMap(@NotNull final Predicate> predicate) { + final HashMap result = new HashMap<>(); + valueRetrievers.entrySet().stream() + .map(e -> ImmutablePair.of(e.getKey(), (Object) e.getValue().value)) + .filter(predicate) + .forEach(e -> result.put(e.getKey(), e.getValue())); + return result; } private static class ValueRetriever { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/lang/QueryLanguageParser.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/lang/QueryLanguageParser.java index 71addc2fdaa..705111387d6 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/lang/QueryLanguageParser.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/lang/QueryLanguageParser.java @@ -86,8 +86,6 @@ import io.deephaven.base.Pair; import io.deephaven.base.verify.Assert; import io.deephaven.configuration.Configuration; -import io.deephaven.engine.context.ExecutionContext; -import io.deephaven.engine.context.QueryScope; import io.deephaven.engine.table.impl.MatchPair; import io.deephaven.engine.table.impl.ShiftedColumnsFactory; import io.deephaven.engine.util.PyCallableWrapper; @@ -96,6 +94,7 @@ import io.deephaven.engine.util.PyCallableWrapperJpyImpl; import io.deephaven.internal.log.LoggerFactory; import io.deephaven.io.logger.Logger; +import io.deephaven.util.annotations.TestUseOnly; import io.deephaven.util.type.TypeUtils; import io.deephaven.vector.ByteVector; import io.deephaven.vector.CharVector; @@ -144,6 +143,8 @@ public final class QueryLanguageParser extends GenericVisitorAdapter, Q private final Map> variables; private final Map[]> variableTypeArguments; + private final Map queryScopeVariables; + private final Set columnVariables; private final HashSet variablesUsed = new HashSet<>(); @@ -189,14 +190,45 @@ public final class QueryLanguageParser extends GenericVisitorAdapter, Q * @param variableTypeArguments A map of the names of scope variables to their type arguments * @throws QueryLanguageParseException If any exception or error is encountered */ - public QueryLanguageParser(String expression, + @TestUseOnly + QueryLanguageParser( + String expression, Collection packageImports, Collection> classImports, Collection> staticImports, Map> variables, Map[]> variableTypeArguments) throws QueryLanguageParseException { this(expression, packageImports, classImports, staticImports, variables, - variableTypeArguments, true); + variableTypeArguments, null, null, true); + } + + /** + * Create a QueryLanguageParser and parse the given {@code expression}. After construction, the + * {@link QueryLanguageParser.Result result} of parsing the {@code expression} is available with the + * {@link #getResult()}} method. + * + * @param expression The query language expression to parse + * @param packageImports Wildcard package imports + * @param classImports Individual class imports + * @param staticImports Wildcard static imports. All static variables and methods for the given classes are + * imported. + * @param variables A map of the names of scope variables to their types + * @param variableTypeArguments A map of the names of scope variables to their type arguments + * @param queryScopeVariables A mutable map of the names of query scope variables to their values + * @param columnVariables A set of column variable names + * @throws QueryLanguageParseException If any exception or error is encountered + */ + public QueryLanguageParser( + String expression, + Collection packageImports, + Collection> classImports, + Collection> staticImports, + Map> variables, + Map[]> variableTypeArguments, + @Nullable Map queryScopeVariables, + @Nullable Set columnVariables) throws QueryLanguageParseException { + this(expression, packageImports, classImports, staticImports, variables, + variableTypeArguments, queryScopeVariables, columnVariables, true); } /** @@ -212,14 +244,20 @@ public QueryLanguageParser(String expression, * @param variables A map of the names of scope variables to their types * @param variableTypeArguments A map of the names of scope variables to their type arguments * @param unboxArguments If true it will unbox the query scope arguments + * @param queryScopeVariables A mutable map of the names of query scope variables to their values + * @param columnVariables A set of column variable names * @throws QueryLanguageParseException If any exception or error is encountered */ - public QueryLanguageParser(String expression, + public QueryLanguageParser( + String expression, Collection packageImports, Collection> classImports, Collection> staticImports, Map> variables, - Map[]> variableTypeArguments, boolean unboxArguments) + Map[]> variableTypeArguments, + @Nullable Map queryScopeVariables, + @Nullable Set columnVariables, + boolean unboxArguments) throws QueryLanguageParseException { this( expression, @@ -228,6 +266,8 @@ public QueryLanguageParser(String expression, staticImports, variables, variableTypeArguments, + queryScopeVariables, + columnVariables, unboxArguments, false, PyCallableWrapperJpyImpl.class.getName()); @@ -240,15 +280,19 @@ public QueryLanguageParser(String expression, final Collection> staticImports, final Map> variables, final Map[]> variableTypeArguments, + @Nullable final Map queryScopeVariables, + @Nullable final Set columnVariables, final boolean unboxArguments, final boolean verifyIdempotence, - final @NotNull String pyCallableWrapperImplName) throws QueryLanguageParseException { + @NotNull final String pyCallableWrapperImplName) throws QueryLanguageParseException { this.packageImports = packageImports == null ? Collections.emptySet() : Set.copyOf(packageImports); this.classImports = classImports == null ? Collections.emptySet() : Set.copyOf(classImports); this.staticImports = staticImports == null ? Collections.emptySet() : Set.copyOf(staticImports); this.variables = variables == null ? Collections.emptyMap() : Map.copyOf(variables); this.variableTypeArguments = variableTypeArguments == null ? Collections.emptyMap() : Map.copyOf(variableTypeArguments); + this.queryScopeVariables = queryScopeVariables == null ? new HashMap<>() : queryScopeVariables; + this.columnVariables = columnVariables == null ? Collections.emptySet() : columnVariables; this.unboxArguments = unboxArguments; Assert.nonempty(pyCallableWrapperImplName, "pyCallableWrapperImplName"); @@ -299,9 +343,10 @@ public QueryLanguageParser(String expression, if (verifyIdempotence) { try { // make sure the parser has no problem reparsing its own output and makes no changes to it. - final QueryLanguageParser validationQueryLanguageParser = new QueryLanguageParser(printedSource, - packageImports, classImports, staticImports, variables, - variableTypeArguments, false, false, pyCallableWrapperImplName); + final QueryLanguageParser validationQueryLanguageParser = new QueryLanguageParser( + printedSource, packageImports, classImports, staticImports, variables, + variableTypeArguments, queryScopeVariables, columnVariables, false, false, + pyCallableWrapperImplName); final String reparsedSource = validationQueryLanguageParser.result.source; Assert.equals( @@ -318,8 +363,8 @@ public QueryLanguageParser(String expression, } } - result = new Result(type, printer.builder.toString(), variablesUsed, isConstantValueExpression, - formulaShiftColPair); + result = new Result(type, printer.builder.toString(), variablesUsed, this.queryScopeVariables, + isConstantValueExpression, formulaShiftColPair); } catch (Throwable e) { // need to catch it and make a new one because it contains unserializable variables... final StringBuilder exceptionMessageBuilder = new StringBuilder(1024) @@ -2501,11 +2546,7 @@ private Optional makeCastExpressionForPyCallable(Class retType, Met private Optional> pyCallableReturnType(@NotNull MethodCallExpr n) { final PyCallableDetails pyCallableDetails = n.getData(QueryLanguageParserDataKeys.PY_CALLABLE_DETAILS); final String pyMethodName = pyCallableDetails.pythonMethodName; - final QueryScope queryScope = ExecutionContext.getContext().getQueryScope(); - final Object paramValueRaw = queryScope.readParamValue(pyMethodName, null); - if (paramValueRaw == null) { - return Optional.empty(); - } + final Object paramValueRaw = queryScopeVariables.get(pyMethodName); if (!(paramValueRaw instanceof PyCallableWrapper)) { return Optional.empty(); } @@ -2594,15 +2635,13 @@ private void tryVectorizePythonCallable(@NotNull MethodCallExpr n, final String pyMethodName = pyCallableDetails.pythonMethodName; Assert.nonempty(pyMethodName, "pyMethodName"); - final QueryScope queryScope = ExecutionContext.getContext().getQueryScope(); - // Note: "paramValueRaw" needs to be the *actual PyCallableWrapper* corresponding to the method. // TODO: Support vectorization of instance methods of constant objects // ^^ i.e., create a constant for `new PyCallableWrapperImpl(pyScopeObj.getAttribute("pyMethodName"))` - final Object paramValueRaw = queryScope.readParamValue(pyMethodName, null); - if (paramValueRaw == null) { + if (!queryScopeVariables.containsKey(pyMethodName)) { throw new IllegalStateException("Resolved Python function name " + pyMethodName + " not found"); } + final Object paramValueRaw = queryScopeVariables.get(pyMethodName); if (!(paramValueRaw instanceof PyCallableWrapper)) { throw new IllegalStateException("Resolved Python function name " + pyMethodName + " not callable"); } @@ -2610,7 +2649,7 @@ private void tryVectorizePythonCallable(@NotNull MethodCallExpr n, prepareVectorization(n, argExpressions, pyCallableWrapper); if (pyCallableWrapper.isVectorizable()) { - prepareVectorizationArgs(n, queryScope, argExpressions, argTypes, pyCallableWrapper); + prepareVectorizationArgs(n, argExpressions, argTypes, pyCallableWrapper); } } @@ -2695,7 +2734,9 @@ private void checkVectorizability(@NotNull final MethodCallExpr n, } } - private void prepareVectorizationArgs(MethodCallExpr n, QueryScope queryScope, Expression[] expressions, + private void prepareVectorizationArgs( + MethodCallExpr n, + Expression[] expressions, Class[] argTypes, PyCallableWrapper pyCallableWrapper) { List> paramTypes = pyCallableWrapper.getParamTypes(); @@ -2710,12 +2751,14 @@ private void prepareVectorizationArgs(MethodCallExpr n, QueryScope queryScope, E addLiteralArg(expressions[i], argTypes[i], pyCallableWrapper); } else if (expressions[i] instanceof NameExpr) { String name = expressions[i].asNameExpr().getNameAsString(); - try { - Object param = queryScope.readParamValue(name); - pyCallableWrapper.addChunkArgument(new ConstantChunkArgument(param, argTypes[i])); - } catch (QueryScope.MissingVariableException ex) { + if (columnVariables.contains(name)) { // A column name or one of the special variables pyCallableWrapper.addChunkArgument(new ColumnChunkArgument(name, argTypes[i])); + } else if (queryScopeVariables.containsKey(name)) { + pyCallableWrapper.addChunkArgument( + new ConstantChunkArgument(queryScopeVariables.get(name), argTypes[i])); + } else { + throw new IllegalStateException("Vectorizability check could not find variable by name: " + name); } } else { throw new IllegalStateException("Vectorizability check failed: " + n); @@ -3180,14 +3223,21 @@ public static class Result { private final Class type; private final String source; private final HashSet variablesUsed; + private final Map possibleParams; private final boolean isConstantValueExpression; private final Pair>> formulaShiftColPair; - Result(Class type, String source, HashSet variablesUsed, boolean isConstantValueExpression, + Result( + Class type, + String source, + HashSet variablesUsed, + Map possibleParams, + boolean isConstantValueExpression, Pair>> formulaShiftColPair) { this.type = Objects.requireNonNull(type, "type"); this.source = source; this.variablesUsed = variablesUsed; + this.possibleParams = possibleParams; this.isConstantValueExpression = isConstantValueExpression; this.formulaShiftColPair = formulaShiftColPair; } @@ -3208,6 +3258,10 @@ public HashSet getVariablesUsed() { return variablesUsed; } + public Map getPossibleParams() { + return possibleParams; + } + public Pair>> getFormulaShiftColPair() { return formulaShiftColPair; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/AbstractConditionFilter.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/AbstractConditionFilter.java index fc9011748b5..1c7ede9c690 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/AbstractConditionFilter.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/AbstractConditionFilter.java @@ -3,6 +3,7 @@ */ package io.deephaven.engine.table.impl.select; +import io.deephaven.api.util.NameValidator; import io.deephaven.base.Pair; import io.deephaven.engine.context.ExecutionContext; import io.deephaven.engine.context.QueryScope; @@ -29,12 +30,7 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.net.MalformedURLException; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.function.BiConsumer; import java.util.stream.Collectors; @@ -97,26 +93,37 @@ public synchronized void init(TableDefinition tableDefinition) { final Map[]> possibleVariableParameterizedTypes = new HashMap<>(); try { - final Map> possibleParams = new HashMap<>(); final QueryScope queryScope = ExecutionContext.getContext().getQueryScope(); - for (QueryScopeParam param : queryScope.getParams(queryScope.getParamNames())) { - possibleParams.put(param.getName(), param); - possibleVariables.put(param.getName(), QueryScopeParamTypeUtil.getDeclaredClass(param.getValue())); + final Map queryScopeVariables = queryScope.toMap( + NameValidator.VALID_QUERY_PARAMETER_MAP_ENTRY_PREDICATE); + for (Map.Entry param : queryScopeVariables.entrySet()) { + possibleVariables.put(param.getKey(), QueryScopeParamTypeUtil.getDeclaredClass(param.getValue())); Type declaredType = QueryScopeParamTypeUtil.getDeclaredType(param.getValue()); if (declaredType instanceof ParameterizedType) { ParameterizedType pt = (ParameterizedType) declaredType; Class[] paramTypes = Arrays.stream(pt.getActualTypeArguments()) .map(QueryScopeParamTypeUtil::classFromType) .toArray(Class[]::new); - possibleVariableParameterizedTypes.put(param.getName(), paramTypes); + possibleVariableParameterizedTypes.put(param.getKey(), paramTypes); } } + final Set columnVariables = new HashSet<>(); + columnVariables.add("i"); + columnVariables.add("ii"); + columnVariables.add("k"); + final BiConsumer> createColumnMappings = (columnName, column) -> { final Class vectorType = DhFormulaColumn.getVectorType(column.getDataType()); - possibleVariables.put(columnName, column.getDataType()); - possibleVariables.put(columnName + COLUMN_SUFFIX, vectorType); + columnVariables.add(columnName); + if (possibleVariables.put(columnName, column.getDataType()) != null) { + possibleVariableParameterizedTypes.remove(columnName); + } + columnVariables.add(columnName + COLUMN_SUFFIX); + if (possibleVariables.put(columnName + COLUMN_SUFFIX, vectorType) != null) { + possibleVariableParameterizedTypes.remove(columnName + COLUMN_SUFFIX); + } final Class compType = column.getComponentType(); if (compType != null && !compType.isPrimitive()) { @@ -148,12 +155,14 @@ public synchronized void init(TableDefinition tableDefinition) { possibleVariables.putAll(timeConversionResult.getNewVariables()); - final QueryLanguageParser.Result result = - new QueryLanguageParser(timeConversionResult.getConvertedFormula(), - ExecutionContext.getContext().getQueryLibrary().getPackageImports(), - ExecutionContext.getContext().getQueryLibrary().getClassImports(), - ExecutionContext.getContext().getQueryLibrary().getStaticImports(), - possibleVariables, possibleVariableParameterizedTypes, unboxArguments).getResult(); + final QueryLanguageParser.Result result = new QueryLanguageParser( + timeConversionResult.getConvertedFormula(), + ExecutionContext.getContext().getQueryLibrary().getPackageImports(), + ExecutionContext.getContext().getQueryLibrary().getClassImports(), + ExecutionContext.getContext().getQueryLibrary().getStaticImports(), + possibleVariables, possibleVariableParameterizedTypes, queryScopeVariables, columnVariables, + unboxArguments) + .getResult(); formulaShiftColPair = result.getFormulaShiftColPair(); if (formulaShiftColPair != null) { log.debug("Formula (after shift conversion) : " + formulaShiftColPair.getFirst()); @@ -205,11 +214,11 @@ public synchronized void init(TableDefinition tableDefinition) { usedColumns.add(variable); } else if (arrayColumnToFind != null && tableDefinition.getColumn(arrayColumnToFind) != null) { usedColumnArrays.add(arrayColumnOuterName); - } else if (possibleParams.containsKey(variable)) { - paramsList.add(possibleParams.get(variable)); + } else if (result.getPossibleParams().containsKey(variable)) { + paramsList.add(new QueryScopeParam<>(variable, result.getPossibleParams().get(variable))); } } - params = paramsList.toArray(QueryScopeParam.ZERO_LENGTH_PARAM_ARRAY); + params = paramsList.toArray(QueryScopeParam[]::new); checkAndInitializeVectorization(result, paramsList); if (!initialized) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/AbstractFormulaColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/AbstractFormulaColumn.java index 14c7ea9e6e4..791be4beaf9 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/AbstractFormulaColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/AbstractFormulaColumn.java @@ -6,8 +6,6 @@ import io.deephaven.base.verify.Require; import io.deephaven.configuration.Configuration; import io.deephaven.engine.table.*; -import io.deephaven.engine.context.ExecutionContext; -import io.deephaven.engine.context.QueryScope; import io.deephaven.engine.context.QueryScopeParam; import io.deephaven.engine.table.impl.BaseTable; import io.deephaven.engine.table.impl.MatchPair; @@ -104,16 +102,13 @@ public void validateSafeForRefresh(BaseTable sourceTable) { } } - protected void applyUsedVariables(Map> columnDefinitionMap, Set variablesUsed) { + protected void applyUsedVariables( + @NotNull final Map> columnDefinitionMap, + @NotNull final Set variablesUsed, + @NotNull final Map possibleParams) { // the column definition map passed in is being mutated by the caller, so we need to make a copy columnDefinitions = Map.copyOf(columnDefinitionMap); - final Map> possibleParams = new HashMap<>(); - final QueryScope queryScope = ExecutionContext.getContext().getQueryScope(); - for (QueryScopeParam param : queryScope.getParams(queryScope.getParamNames())) { - possibleParams.put(param.getName(), param); - } - final List> paramsList = new ArrayList<>(); usedColumns = new ArrayList<>(); usedColumnArrays = new ArrayList<>(); @@ -132,12 +127,12 @@ protected void applyUsedVariables(Map> columnDefinit if (variable.endsWith(COLUMN_SUFFIX) && columnDefinitions.get(strippedColumnName) != null) { usedColumnArrays.add(strippedColumnName); } else if (possibleParams.containsKey(variable)) { - paramsList.add(possibleParams.get(variable)); + paramsList.add(new QueryScopeParam<>(variable, possibleParams.get(variable))); } } } - params = paramsList.toArray(QueryScopeParam.ZERO_LENGTH_PARAM_ARRAY); + params = paramsList.toArray(QueryScopeParam[]::new); } protected void onCopy(final AbstractFormulaColumn copy) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/DhFormulaColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/DhFormulaColumn.java index 08ae9eaed62..4a807eb2c79 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/DhFormulaColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/DhFormulaColumn.java @@ -201,7 +201,7 @@ public List initDef(Map> columnDefinitionMap log.debug().append("Expression (after language conversion) : ").append(analyzedFormula.cookedFormulaString) .endl(); - applyUsedVariables(columnDefinitionMap, result.getVariablesUsed()); + applyUsedVariables(columnDefinitionMap, result.getVariablesUsed(), result.getPossibleParams()); returnedType = result.getType(); if (returnedType == boolean.class) { returnedType = Boolean.class; diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/codegen/FormulaAnalyzer.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/codegen/FormulaAnalyzer.java index d32841e8afe..cc9dceedf58 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/codegen/FormulaAnalyzer.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/codegen/FormulaAnalyzer.java @@ -3,15 +3,14 @@ */ package io.deephaven.engine.table.impl.select.codegen; +import io.deephaven.api.util.NameValidator; import io.deephaven.datastructures.util.CollectionUtil; import io.deephaven.engine.context.ExecutionContext; import io.deephaven.engine.table.ColumnDefinition; import io.deephaven.engine.table.impl.lang.QueryLanguageParser; import io.deephaven.engine.table.impl.select.QueryScopeParamTypeUtil; -import io.deephaven.engine.context.QueryScopeParam; import io.deephaven.time.TimeLiteralReplacedExpression; import io.deephaven.vector.ObjectVector; -import io.deephaven.engine.context.QueryScope; import io.deephaven.engine.table.impl.select.DhFormulaColumn; import io.deephaven.engine.table.impl.select.formula.FormulaSourceDescriptor; import io.deephaven.engine.table.WritableColumnSource; @@ -30,11 +29,6 @@ public static Result analyze(final String rawFormulaString, final Map> columnDefinitionMap, final TimeLiteralReplacedExpression timeConversionResult, final QueryLanguageParser.Result queryLanguageResult) throws Exception { - final Map> possibleParams = new HashMap<>(); - final QueryScope queryScope = ExecutionContext.getContext().getQueryScope(); - for (QueryScopeParam param : queryScope.getParams(queryScope.getParamNames())) { - possibleParams.put(param.getName(), param); - } log.debug().append("Expression (after language conversion) : ") .append(queryLanguageResult.getConvertedExpression()) @@ -55,7 +49,7 @@ public static Result analyze(final String rawFormulaString, null != columnDefinitionMap .get(bareName = variable.substring(0, variable.length() - colSuffix.length()))) { usedColumnArrays.add(bareName); - } else if (possibleParams.containsKey(variable)) { + } else if (queryLanguageResult.getPossibleParams().containsKey(variable)) { userParams.add(variable); } } @@ -80,24 +74,45 @@ public static QueryLanguageParser.Result getCompiledFormula(Map columnVariables = new HashSet<>(); + columnVariables.add("i"); + columnVariables.add("ii"); + columnVariables.add("k"); + final Map[]> possibleVariableParameterizedTypes = new HashMap<>(); for (ColumnDefinition columnDefinition : availableColumns.values()) { + // add column-vectors final String columnSuffix = DhFormulaColumn.COLUMN_SUFFIX; final Class vectorType = DhFormulaColumn.getVectorType(columnDefinition.getDataType()); possibleVariables.put(columnDefinition.getName() + columnSuffix, vectorType); + columnVariables.add(columnDefinition.getName() + columnSuffix); if (vectorType == ObjectVector.class) { possibleVariableParameterizedTypes.put(columnDefinition.getName() + columnSuffix, new Class[] {columnDefinition.getDataType()}); } + + // add columns + columnVariables.add(columnDefinition.getName()); + possibleVariables.put(columnDefinition.getName(), columnDefinition.getDataType()); + final Class compType = columnDefinition.getComponentType(); + if (compType != null && !compType.isPrimitive()) { + possibleVariableParameterizedTypes.put(columnDefinition.getName(), new Class[] {compType}); + } } final ExecutionContext context = ExecutionContext.getContext(); - final QueryScope queryScope = context.getQueryScope(); - for (QueryScopeParam param : queryScope.getParams(queryScope.getParamNames())) { - possibleVariables.put(param.getName(), QueryScopeParamTypeUtil.getDeclaredClass(param.getValue())); + final Map queryScopeVariables = context.getQueryScope().toMap( + NameValidator.VALID_QUERY_PARAMETER_MAP_ENTRY_PREDICATE); + for (Map.Entry param : queryScopeVariables.entrySet()) { + if (possibleVariables.containsKey(param.getKey())) { + // skip any existing matches + continue; + } + + possibleVariables.put(param.getKey(), QueryScopeParamTypeUtil.getDeclaredClass(param.getValue())); Type declaredType = QueryScopeParamTypeUtil.getDeclaredType(param.getValue()); if (declaredType instanceof ParameterizedType) { @@ -105,15 +120,7 @@ public static QueryLanguageParser.Result getCompiledFormula(Map[] paramTypes = Arrays.stream(pt.getActualTypeArguments()) .map(QueryScopeParamTypeUtil::classFromType) .toArray(Class[]::new); - possibleVariableParameterizedTypes.put(param.getName(), paramTypes); - } - } - - for (ColumnDefinition columnDefinition : availableColumns.values()) { - possibleVariables.put(columnDefinition.getName(), columnDefinition.getDataType()); - final Class compType = columnDefinition.getComponentType(); - if (compType != null && !compType.isPrimitive()) { - possibleVariableParameterizedTypes.put(columnDefinition.getName(), new Class[] {compType}); + possibleVariableParameterizedTypes.put(param.getKey(), paramTypes); } } @@ -131,7 +138,7 @@ public static QueryLanguageParser.Result getCompiledFormula(Map initDef(Map> columnDefinit validateColumnDefinition(columnDefinitionMap); } else { returnedType = dcf.getReturnedType(); - applyUsedVariables(columnDefinitionMap, new LinkedHashSet<>(dcf.getColumnNames())); + applyUsedVariables(columnDefinitionMap, new LinkedHashSet<>(dcf.getColumnNames()), Map.of()); formulaFactory = createKernelFormulaFactory(this); } diff --git a/engine/table/src/main/java/io/deephaven/engine/util/AbstractScriptSession.java b/engine/table/src/main/java/io/deephaven/engine/util/AbstractScriptSession.java index bce3c2f8236..5cf6677e3b2 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/AbstractScriptSession.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/AbstractScriptSession.java @@ -25,6 +25,7 @@ import io.deephaven.plugin.type.ObjectType; import io.deephaven.plugin.type.ObjectTypeLookup; import io.deephaven.util.SafeCloseable; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.io.File; @@ -291,12 +292,13 @@ public QueryScope getQueryScope() { protected abstract Object setVariable(String name, @Nullable Object value); /** - * Returns an immutable map with all known variables and their values. + * Returns a mutable map with all known variables and their values. It is owned by the caller. * - * @return an immutable map with all known variables and their values. As with {@link #getVariable(String)}, values - * may need to be unwrapped. + * @param predicate a predicate to decide if an entry should be included in the returned map + * @return a mutable map with all known variables and their values. As with {@link #getVariable(String)}, values may + * need to be unwrapped. */ - protected abstract Map getAllValues(); + protected abstract Map getAllValues(@NotNull Predicate> predicate); // ----------------------------------------------------------------------------------------------------------------- // ScriptSession-based QueryScope implementation, with no remote scope or object reflection support @@ -370,12 +372,12 @@ public void putParam(final String name, final T value) { } @Override - public Map toMap() { - return AbstractScriptSession.this.getAllValues(); + public Map toMap(@NotNull final Predicate> predicate) { + return AbstractScriptSession.this.getAllValues(predicate); } @Override - public Object unwrapObject(Object object) { + public Object unwrapObject(@Nullable Object object) { return AbstractScriptSession.this.unwrapObject(object); } } diff --git a/engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java b/engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java index 88c8a51f3a8..41af97b5a1c 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java @@ -52,6 +52,7 @@ import org.codehaus.groovy.control.Phases; import org.codehaus.groovy.control.customizers.ImportCustomizer; import org.codehaus.groovy.tools.GroovyClass; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import javax.tools.JavaFileObject; @@ -769,9 +770,11 @@ protected Object setVariable(String name, @Nullable Object newValue) { } @Override - protected Map getAllValues() { + protected Map getAllValues(@NotNull final Predicate> predicate) { synchronized (bindingBackingMap) { - return Map.copyOf(bindingBackingMap); + return bindingBackingMap.entrySet().stream() + .filter(predicate) + .collect(HashMap::new, (map, entry) -> map.put(entry.getKey(), entry.getValue()), HashMap::putAll); } } diff --git a/engine/table/src/main/java/io/deephaven/engine/util/NoLanguageDeephavenSession.java b/engine/table/src/main/java/io/deephaven/engine/util/NoLanguageDeephavenSession.java index 56abfb0756e..9ffac0e5c59 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/NoLanguageDeephavenSession.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/NoLanguageDeephavenSession.java @@ -3,15 +3,14 @@ */ package io.deephaven.engine.util; +import io.deephaven.api.util.NameValidator; import io.deephaven.engine.context.QueryScope; import io.deephaven.engine.updategraph.OperationInitializer; import io.deephaven.engine.updategraph.UpdateGraph; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.Collections; -import java.util.LinkedHashMap; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -95,9 +94,11 @@ protected Object setVariable(String name, @Nullable Object newValue) { } @Override - protected Map getAllValues() { + protected Map getAllValues(@NotNull final Predicate> predicate) { synchronized (variables) { - return Map.copyOf(variables); + return variables.entrySet().stream() + .filter(predicate) + .collect(HashMap::new, (map, entry) -> map.put(entry.getKey(), entry.getValue()), HashMap::putAll); } } diff --git a/engine/table/src/main/java/io/deephaven/engine/util/PyCallableWrapperJpyImpl.java b/engine/table/src/main/java/io/deephaven/engine/util/PyCallableWrapperJpyImpl.java index c4b05c25493..53aaf158498 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/PyCallableWrapperJpyImpl.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/PyCallableWrapperJpyImpl.java @@ -42,6 +42,11 @@ public class PyCallableWrapperJpyImpl implements PyCallableWrapper { numpyType2JavaClass.put('O', Object.class); } + /** + * Ensure that the class initializer runs. + */ + public static void init() {} + // TODO: support for vectorizing functions that return arrays // https://github.com/deephaven/deephaven-core/issues/4649 private static final Set> vectorizableReturnTypes = Set.of( diff --git a/engine/table/src/main/java/io/deephaven/engine/util/PythonScope.java b/engine/table/src/main/java/io/deephaven/engine/util/PythonScope.java index 8368a8a3aef..ba12ecb22aa 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/PythonScope.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/PythonScope.java @@ -6,9 +6,7 @@ import org.jpy.PyDictWrapper; import org.jpy.PyObject; -import java.util.AbstractMap.SimpleImmutableEntry; import java.util.Collection; -import java.util.Map; import java.util.Map.Entry; import java.util.Optional; import java.util.stream.Collectors; @@ -132,17 +130,6 @@ default Stream getKeys() { .map(this::convertStringKey); } - /** - * Equivalent to {@link #getEntriesRaw()}, where the keys have been converted via {@link #convertStringKey(Object)} - * and the values via {@link #convertValue(Object)}. - * - * @return the string keys and converted values - */ - default Stream> getEntries() { - return getEntriesRaw() - .map(e -> new SimpleImmutableEntry<>(convertStringKey(e.getKey()), convertValue(e.getValue()))); - } - /** * Equivalent to {@link #getKeys()}.collect(someCollector) * @@ -153,23 +140,6 @@ default Collection getKeysCollection() { .collect(Collectors.toList()); } - /** - * Equivalent to {@link #getEntries()}.collect(someMapCollector) - * - * @return the string keys and converted values, as a map - */ - default Map getEntriesMap() { - return getEntries() - .collect(Collectors.toMap(Entry::getKey, Entry::getValue)); - - // we're currently making sure that we don't convert None to null... - /* - * // workaround since the collector doesn't work w/ null values // - * https://bugs.openjdk.java.net/browse/JDK-8148463 return getEntries() .collect( HashMap::new, (map, entry) -> - * map.put(entry.getKey(), entry.getValue()), HashMap::putAll); - */ - } - /** * @return the Python's __main__ module namespace */ diff --git a/engine/table/src/main/java/io/deephaven/engine/util/PythonScopeJpyImpl.java b/engine/table/src/main/java/io/deephaven/engine/util/PythonScopeJpyImpl.java index 46caa9b3777..c2258bc5d99 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/PythonScopeJpyImpl.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/PythonScopeJpyImpl.java @@ -3,23 +3,33 @@ */ package io.deephaven.engine.util; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; +import io.deephaven.UncheckedDeephavenException; +import io.deephaven.configuration.Configuration; import org.jpy.PyDictWrapper; import org.jpy.PyLib; import org.jpy.PyObject; import java.util.ArrayDeque; import java.util.Deque; -import java.util.HashMap; -import java.util.Map; import java.util.Map.Entry; import java.util.Optional; +import java.util.concurrent.ExecutionException; import java.util.stream.Stream; public class PythonScopeJpyImpl implements PythonScope { + private static volatile boolean cacheEnabled = + Configuration.getInstance().getBooleanForClassWithDefault(PythonScopeJpyImpl.class, "cacheEnabled", false); + + public static void setCacheEnabled(boolean enabled) { + cacheEnabled = enabled; + } + private final PyDictWrapper dict; private static final ThreadLocal> threadScopeStack = new ThreadLocal<>(); - private static final ThreadLocal>> threadConvertedMapStack = new ThreadLocal<>(); + private static final Cache conversionCache = CacheBuilder.newBuilder().weakValues().build(); public static PythonScopeJpyImpl ofMainGlobals() { return new PythonScopeJpyImpl(PyLib.getMainGlobals().asDict()); @@ -77,25 +87,6 @@ public Object convertValue(PyObject value) { return convert(value); } - private static Deque> ensureConvertedMap() { - Deque> convertedMapStack = threadConvertedMapStack.get(); - if (convertedMapStack == null) { - convertedMapStack = new ArrayDeque<>(); - threadConvertedMapStack.set(convertedMapStack); - } - // the current thread doesn't have a default map for the default main scope yet - if (convertedMapStack.isEmpty()) { - HashMap convertedMap = new HashMap<>(); - convertedMapStack.push(convertedMap); - } - return convertedMapStack; - } - - private static Map currentConvertedMap() { - Deque> convertedMapStack = ensureConvertedMap(); - return convertedMapStack.peek(); - } - /** * Converts a pyObject into an appropriate Java object for use outside of JPy. *

@@ -109,22 +100,31 @@ private static Map currentConvertedMap() { * @return a Java object representing the underlying JPy object. */ public static Object convert(PyObject pyObject) { - Map convertedMap = currentConvertedMap(); - return convertedMap.computeIfAbsent(pyObject, PythonScopeJpyImpl::convertInternal); + if (!cacheEnabled) { + return convertInternal(pyObject, false); + } + + try { + final Object cached = conversionCache.get(pyObject, () -> convertInternal(pyObject, true)); + return cached instanceof NULL_TOKEN ? null : cached; + } catch (ExecutionException err) { + throw new UncheckedDeephavenException("Error converting PyObject to Java object", err); + } } - private static Object convertInternal(PyObject pyObject) { + private static Object convertInternal(PyObject pyObject, boolean fromCache) { + Object ret = pyObject; if (pyObject.isList()) { - return pyObject.asList(); + ret = pyObject.asList(); } else if (pyObject.isDict()) { - return pyObject.asDict(); + ret = pyObject.asDict(); } else if (pyObject.isCallable()) { - return new PyCallableWrapperJpyImpl(pyObject); + ret = new PyCallableWrapperJpyImpl(pyObject); } else if (pyObject.isConvertible()) { - return pyObject.getObjectValue(); - } else { - return pyObject; + ret = pyObject.getObjectValue(); } + + return ret == null && fromCache ? new NULL_TOKEN() : ret; } public PyDictWrapper mainGlobals() { @@ -139,10 +139,6 @@ public void pushScope(PyObject pydict) { threadScopeStack.set(scopeStack); } scopeStack.push(pydict.asDict()); - - Deque> convertedMapStack = ensureConvertedMap(); - HashMap convertedMap = new HashMap<>(); - convertedMapStack.push(convertedMap); } @Override @@ -153,11 +149,12 @@ public void popScope() { } PyDictWrapper pydict = scopeStack.pop(); pydict.close(); + } - Deque> convertedMapStack = threadConvertedMapStack.get(); - if (convertedMapStack == null) { - throw new IllegalStateException("The thread converted-map stack is empty."); - } - convertedMapStack.pop(); + /** + * Guava caches are not allowed to hold on to null values. Additionally, we can't use a singleton pattern or else + * the weak-value map will never release null values. + */ + private static class NULL_TOKEN { } } diff --git a/engine/table/src/main/java/io/deephaven/engine/util/ScriptSession.java b/engine/table/src/main/java/io/deephaven/engine/util/ScriptSession.java index 44280df2aa0..e22b0bd03b9 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/ScriptSession.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/ScriptSession.java @@ -118,7 +118,7 @@ default Throwable sanitizeThrowable(Throwable e) { * @param object the scoped object * @return an obj which can be consumed by a client */ - default Object unwrapObject(Object object) { + default Object unwrapObject(@Nullable Object object) { return object; } } diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/lang/TestQueryLanguageParser.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/lang/TestQueryLanguageParser.java index 34ac7284985..3eba42253f5 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/lang/TestQueryLanguageParser.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/lang/TestQueryLanguageParser.java @@ -4,12 +4,12 @@ package io.deephaven.engine.table.impl.lang; import groovy.lang.Closure; +import io.deephaven.api.util.NameValidator; import io.deephaven.base.Pair; import io.deephaven.base.testing.BaseArrayTestCase; import io.deephaven.base.verify.Assert; import io.deephaven.base.verify.Require; -import io.deephaven.engine.context.ExecutionContext; -import io.deephaven.engine.context.TestExecutionContext; +import io.deephaven.engine.context.*; import io.deephaven.engine.table.Table; import io.deephaven.engine.table.impl.lang.QueryLanguageParser.QueryLanguageParseException; import io.deephaven.engine.testutil.ControlledUpdateGraph; @@ -3175,9 +3175,17 @@ private void check(String expression, String resultExpression, Class resultTy private void check(String expression, String resultExpression, Class resultType, String[] resultVarsUsed, boolean verifyIdempotence) throws Exception { - QueryLanguageParser.Result result = + final Map possibleParams; + final QueryScope queryScope = ExecutionContext.getContext().getQueryScope(); + if (!(queryScope instanceof PoisonedQueryScope)) { + possibleParams = queryScope.toMap(NameValidator.VALID_QUERY_PARAMETER_MAP_ENTRY_PREDICATE); + } else { + possibleParams = null; + } + + final QueryLanguageParser.Result result = new QueryLanguageParser(expression, packageImports, classImports, staticImports, - variables, variableParameterizedTypes, + variables, variableParameterizedTypes, possibleParams, null, true, verifyIdempotence, PyCallableWrapperDummyImpl.class.getName()).getResult(); diff --git a/server/src/main/java/io/deephaven/server/console/python/PythonGlobalScopeCopyModule.java b/server/src/main/java/io/deephaven/server/console/python/PythonGlobalScopeCopyModule.java index 40104dbd117..1e091c35ffd 100644 --- a/server/src/main/java/io/deephaven/server/console/python/PythonGlobalScopeCopyModule.java +++ b/server/src/main/java/io/deephaven/server/console/python/PythonGlobalScopeCopyModule.java @@ -16,7 +16,9 @@ public interface PythonGlobalScopeCopyModule { @Provides static PythonEvaluatorJpy providePythonEvaluatorJpy() { try { - return PythonEvaluatorJpy.withGlobalCopy(); + PythonEvaluatorJpy jpy = PythonEvaluatorJpy.withGlobalCopy(); + PythonImportInitializer.init(); + return jpy; } catch (IOException | InterruptedException | TimeoutException e) { throw new IllegalStateException("Unable to start a python session: ", e); } diff --git a/server/src/main/java/io/deephaven/server/console/python/PythonGlobalScopeModule.java b/server/src/main/java/io/deephaven/server/console/python/PythonGlobalScopeModule.java index be29e87006c..4b260e6d302 100644 --- a/server/src/main/java/io/deephaven/server/console/python/PythonGlobalScopeModule.java +++ b/server/src/main/java/io/deephaven/server/console/python/PythonGlobalScopeModule.java @@ -16,7 +16,9 @@ public interface PythonGlobalScopeModule { @Provides static PythonEvaluatorJpy providePythonEvaluatorJpy() { try { - return PythonEvaluatorJpy.withGlobals(); + PythonEvaluatorJpy jpy = PythonEvaluatorJpy.withGlobals(); + PythonImportInitializer.init(); + return jpy; } catch (IOException | InterruptedException | TimeoutException e) { throw new IllegalStateException("Unable to start a python session: ", e); } diff --git a/server/src/main/java/io/deephaven/server/console/python/PythonImportInitializer.java b/server/src/main/java/io/deephaven/server/console/python/PythonImportInitializer.java new file mode 100644 index 00000000000..521845b552e --- /dev/null +++ b/server/src/main/java/io/deephaven/server/console/python/PythonImportInitializer.java @@ -0,0 +1,17 @@ +package io.deephaven.server.console.python; + +import io.deephaven.engine.util.PyCallableWrapperJpyImpl; +import io.deephaven.integrations.python.PythonObjectWrapper; + +public abstract class PythonImportInitializer { + public static void init() { + // ensured that these classes are initialized during Jpy initialization as they import python modules and we'd + // like to avoid deadlocking the GIL during class initialization + PythonObjectWrapper.init(); + PyCallableWrapperJpyImpl.init(); + } + + private PythonImportInitializer() { + // no instances should be created + } +} diff --git a/table-api/src/main/java/io/deephaven/api/util/NameValidator.java b/table-api/src/main/java/io/deephaven/api/util/NameValidator.java index 21890a16ad8..a37e6a8ee17 100644 --- a/table-api/src/main/java/io/deephaven/api/util/NameValidator.java +++ b/table-api/src/main/java/io/deephaven/api/util/NameValidator.java @@ -5,9 +5,8 @@ import javax.lang.model.SourceVersion; import java.util.*; -import java.util.function.Consumer; import java.util.function.Function; -import java.util.function.Supplier; +import java.util.function.Predicate; import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -98,6 +97,9 @@ private boolean isValid(String name) { Stream.of("in", "not", "i", "ii", "k").collect( Collectors.collectingAndThen(Collectors.toSet(), Collections::unmodifiableSet)); + public static final Predicate> VALID_QUERY_PARAMETER_MAP_ENTRY_PREDICATE = + e -> isValidQueryParameterName(e.getKey()); + public static String validateTableName(String name) { return Type.TABLE.validate(name); }