Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Benchmark and speed processing of polyglot java imports up #10899

Merged
merged 12 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -408,16 +408,15 @@ public void testDoubledRandom() throws Exception {
polyglot java import java.util.Random

run seed =
operator1 = Random.new_generator seed
Random.new_generator seed
""");
var run = module.invokeMember("eval_expression", "run");
try {
var err = run.execute(1L);
fail("Not expecting any result: " + err);
assertTrue("Returned value represents exception: " + err, err.isException());
throw err.throwException();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolving polyglot java imports lazily prevents us to emit error during compilation. Instead we get a (dataflow) Error when we execute the run function and try to resolve Random function. Such an error is then returned as a value and needs to be throwException manually.

} catch (PolyglotException ex) {
assertEquals(
"Compile error: Compiler Internal Error: No polyglot symbol for Random.",
ex.getMessage());
assertEquals("Compile error: No polyglot symbol for Random.", ex.getMessage());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,16 +148,27 @@ private void assertWarningsForAType(Value v) {
assertEquals("Types without and with warnings are the same", type, warningType);
assertTrue("It is an exception. Type: " + type, warning2.isException());
try {
warning2.throwException();
throw warning2.throwException();
} catch (PolyglotException ex) {
if (ex.getMessage() == null) {
assertEquals(generator.typeError(), type);
assertEquals(generator.typeError(), warningType);
} else {
assertThat(
"Warning found for " + type,
ex.getMessage(),
AllOf.allOf(containsString("warn:once"), containsString("warn:twice")));
try {
assertThat(
"Warning found for " + type,
ex.getMessage(),
AllOf.allOf(containsString("warn:once"), containsString("warn:twice")));
} catch (AssertionError err) {
if (type != null && v.equals(warning1) && v.equals(warning2)) {
assertEquals(
"Cannot attach warnings to Error - check it is an error",
"Standard.Base.Error.Error",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type.getMetaQualifiedName());
return;
}
throw err;
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import org.enso.interpreter.runtime.callable.function.Function;
import org.enso.interpreter.runtime.data.Type;
import org.enso.interpreter.runtime.data.atom.AtomConstructor;
import org.enso.interpreter.runtime.data.text.Text;
import org.enso.interpreter.runtime.error.PanicException;
import org.enso.interpreter.runtime.scope.ModuleScope;

Expand Down Expand Up @@ -214,7 +213,7 @@ final ExpressionNode replaceItself() {
return newNode;
} catch (CompilerError abnormalException) {
var ctx = EnsoContext.get(this);
var msg = Text.create(abnormalException.getMessage());
var msg = abnormalException.getMessage();
var load = ctx.getBuiltins().error().makeCompileError(msg);
throw new PanicException(load, this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import com.oracle.truffle.api.nodes.RootNode;
import java.util.Arrays;
import java.util.List;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.enso.interpreter.EnsoLanguage;
Expand Down Expand Up @@ -45,6 +46,7 @@
import org.enso.interpreter.runtime.error.PanicSentinel;
import org.enso.interpreter.runtime.library.dispatch.TypeOfNode;
import org.enso.interpreter.runtime.library.dispatch.TypesLibrary;
import org.enso.interpreter.runtime.util.CachingSupplier;
import org.graalvm.collections.Pair;

public abstract class ReadArgumentCheckNode extends Node {
Expand Down Expand Up @@ -162,8 +164,10 @@ public static ReadArgumentCheckNode build(EnsoContext ctx, String comment, Type
return ReadArgumentCheckNodeFactory.TypeCheckNodeGen.create(comment, expectedType);
}

public static ReadArgumentCheckNode meta(String comment, Object metaObject) {
return ReadArgumentCheckNodeFactory.MetaCheckNodeGen.create(comment, metaObject);
public static ReadArgumentCheckNode meta(
String comment, Supplier<? extends Object> metaObjectSupplier) {
var cachingSupplier = CachingSupplier.wrap(metaObjectSupplier);
return ReadArgumentCheckNodeFactory.MetaCheckNodeGen.create(comment, cachingSupplier);
}

public static boolean isWrappedThunk(Function fn) {
Expand Down Expand Up @@ -475,12 +479,12 @@ String expectedTypeMessage() {
}

abstract static class MetaCheckNode extends ReadArgumentCheckNode {
private final Object expectedMeta;
private final CachingSupplier<? extends Object> expectedSupplier;
@CompilerDirectives.CompilationFinal private String expectedTypeMessage;

MetaCheckNode(String name, Object expectedMeta) {
MetaCheckNode(String name, CachingSupplier<? extends Object> expectedMetaSupplier) {
super(name);
this.expectedMeta = expectedMeta;
this.expectedSupplier = expectedMetaSupplier;
}

@Override
Expand All @@ -493,7 +497,7 @@ Object verifyMetaObject(VirtualFrame frame, Object v, @Cached IsValueOfTypeNode
if (isAllFitValue(v)) {
return v;
}
if (isA.execute(expectedMeta, v)) {
if (isA.execute(expectedSupplier.get(), v)) {
return v;
} else {
return null;
Expand All @@ -508,9 +512,9 @@ String expectedTypeMessage() {
CompilerDirectives.transferToInterpreterAndInvalidate();
var iop = InteropLibrary.getUncached();
try {
expectedTypeMessage = iop.asString(iop.getMetaQualifiedName(expectedMeta));
expectedTypeMessage = iop.asString(iop.getMetaQualifiedName(expectedSupplier.get()));
} catch (UnsupportedMessageException ex) {
expectedTypeMessage = expectedMeta.toString();
expectedTypeMessage = expectedSupplier.get().toString();
}
return expectedTypeMessage;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,29 @@
import com.oracle.truffle.api.RootCallTarget;
import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.profiles.CountingConditionProfile;
import org.enso.interpreter.node.ExpressionNode;
import org.enso.interpreter.node.expression.builtin.meta.IsSameObjectNode;

public class ObjectEqualityBranchNode extends BranchNode {
private final Object expected;
private @Child ExpressionNode expected;
private @Child IsSameObjectNode isSameObject = IsSameObjectNode.build();
private final CountingConditionProfile profile = CountingConditionProfile.create();

private ObjectEqualityBranchNode(RootCallTarget branch, Object expected, boolean terminalBranch) {
private ObjectEqualityBranchNode(
RootCallTarget branch, ExpressionNode expected, boolean terminalBranch) {
super(branch, terminalBranch);
this.expected = expected;
}

public static BranchNode build(RootCallTarget branch, Object expected, boolean terminalBranch) {
public static BranchNode build(
RootCallTarget branch, ExpressionNode expected, boolean terminalBranch) {
return new ObjectEqualityBranchNode(branch, expected, terminalBranch);
}

@Override
public void execute(VirtualFrame frame, Object state, Object target) {
if (profile.profile(isSameObject.execute(target, expected))) {
var exp = expected.executeGeneric(frame);
if (profile.profile(isSameObject.execute(target, exp))) {
accept(frame, state, new Object[0]);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

/** Represents a compile-time constant. */
@NodeInfo(shortName = "const", description = "Represents an arbitrary compile-time constant.")
public class ConstantObjectNode extends ExpressionNode {
public final class ConstantObjectNode extends ExpressionNode {
private final Object object;

private ConstantObjectNode(Object object) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package org.enso.interpreter.node.expression.constant;

import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.interop.TruffleObject;
import com.oracle.truffle.api.nodes.NodeInfo;
import java.util.function.Supplier;
import org.enso.interpreter.node.ExpressionNode;
import org.enso.interpreter.runtime.data.text.Text;
import org.enso.interpreter.runtime.error.DataflowError;
import org.enso.interpreter.runtime.util.CachingSupplier;

@NodeInfo(
shortName = "lazy",
description = "Represents an arbitrary compile-time constant computed lazily.")
public final class LazyObjectNode extends ExpressionNode {

private final String error;
private final CachingSupplier<? extends Object> supply;

private LazyObjectNode(String error, Supplier<? extends Object> supply) {
this.error = error;
this.supply = CachingSupplier.wrap(supply);
}

/**
* Creates a node that returns lazily computed value.
*
* @param errorMessage the error message to show when the value is {@code null}
* @param supplier computes the value lazily. Can return {@code null} and then the {@code
* errorMessage} error is created
*/
public static ExpressionNode build(String errorMessage, Supplier<TruffleObject> supplier) {
return new LazyObjectNode(errorMessage, supplier);
}

@Override
public Object executeGeneric(VirtualFrame frame) {
var result = supply.get();
if (result instanceof TruffleObject) {
return result;
}
return DataflowError.withDefaultTrace(Text.create(error), this);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You only check if it is a TruffleObject not if it is null which seems to be what javadoc suggests?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean the Javadoc of build method that takes Supplier<TruffleObject> argument?

If typing is on, then check for null is the same as instanceof TruffleObject. But I decided to go for the latter as someone could sneak in raw Supplier and return anything. I would need to perform some complicated checks to verify the value is proper Enso interpreter value. That can be added, but for now instanceof TruffleObject is sufficient.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import com.oracle.truffle.api.TruffleLogger;
import com.oracle.truffle.api.interop.InteropException;
import com.oracle.truffle.api.interop.InteropLibrary;
import com.oracle.truffle.api.interop.TruffleObject;
import com.oracle.truffle.api.interop.UnknownIdentifierException;
import com.oracle.truffle.api.interop.UnsupportedMessageException;
import com.oracle.truffle.api.io.TruffleProcessBuilder;
Expand Down Expand Up @@ -51,7 +52,7 @@
import org.enso.interpreter.instrument.NotificationHandler;
import org.enso.interpreter.runtime.builtin.Builtins;
import org.enso.interpreter.runtime.data.Type;
import org.enso.interpreter.runtime.data.text.Text;
import org.enso.interpreter.runtime.error.DataflowError;
import org.enso.interpreter.runtime.error.PanicException;
import org.enso.interpreter.runtime.scope.TopLevelScope;
import org.enso.interpreter.runtime.state.ExecutionEnvironment;
Expand Down Expand Up @@ -553,18 +554,18 @@ public boolean isColorTerminalOutput() {
* is looked up by iterating the members of the outer class via Truffle's interop protocol.
*
* @param className Fully qualified class name, can also be nested static inner class.
* @return If the java class is found, return it, otherwise return null.
* @return If the java class is found, return it, otherwise return {@link DataflowError}.
*/
@TruffleBoundary
public Object lookupJavaClass(String className) {
public TruffleObject lookupJavaClass(String className) {
var binaryName = new StringBuilder(className);
var collectedExceptions = new ArrayList<Exception>();
for (; ; ) {
var fqn = binaryName.toString();
try {
var hostSymbol = lookupHostSymbol(fqn);
if (hostSymbol != null) {
return hostSymbol;
return (TruffleObject) hostSymbol;
}
} catch (ClassNotFoundException | RuntimeException | InteropException ex) {
collectedExceptions.add(ex);
Expand All @@ -581,7 +582,7 @@ public Object lookupJavaClass(String className) {
level = Level.FINE;
logger.log(Level.FINE, null, ex);
}
return null;
return getBuiltins().error().makeMissingPolyglotImportError(className);
}

private Object lookupHostSymbol(String fqn)
Expand Down Expand Up @@ -971,8 +972,7 @@ public PanicException raiseAssertionPanic(Node node, String message, Throwable e
if (message != null) {
msg = msg + sep + message;
}
var txt = Text.create(msg);
var err = getBuiltins().error().makeAssertionError(txt);
var err = getBuiltins().error().makeAssertionError(msg);
throw new PanicException(err, e, node);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ private Map<String, Map<String, Supplier<LoadedBuiltinMethod>>> registerBuiltinM
constr -> {
Map<String, Supplier<LoadedBuiltinMethod>> atomNodes =
getOrUpdate(builtinMethodNodes, constr.getName());
atomNodes.put(builtinMethodName, new CachingSupplier<>(() -> meta.toMethod()));
atomNodes.put(builtinMethodName, CachingSupplier.wrap(() -> meta.toMethod()));

Map<String, LoadedBuiltinMetaMethod> atomNodesMeta =
getOrUpdate(builtinMetaMethods, constr.getName());
Expand All @@ -253,7 +253,7 @@ private Map<String, Map<String, Supplier<LoadedBuiltinMethod>>> registerBuiltinM
() -> {
Map<String, Supplier<LoadedBuiltinMethod>> atomNodes =
getOrUpdate(builtinMethodNodes, builtinMethodOwner);
atomNodes.put(builtinMethodName, new CachingSupplier<>(() -> meta.toMethod()));
atomNodes.put(builtinMethodName, CachingSupplier.wrap(() -> meta.toMethod()));

Map<String, LoadedBuiltinMetaMethod> atomNodesMeta =
getOrUpdate(builtinMetaMethods, builtinMethodOwner);
Expand Down Expand Up @@ -420,12 +420,12 @@ private Map<String, Map<String, Supplier<LoadedBuiltinMethod>>> readBuiltinMetho
constr -> {
Map<String, Supplier<LoadedBuiltinMethod>> atomNodes =
getOrUpdate(methodNodes, constr.getName());
atomNodes.put(builtinMethodName, new CachingSupplier<>(builtin));
atomNodes.put(builtinMethodName, CachingSupplier.forValue(builtin));
},
() -> {
Map<String, Supplier<LoadedBuiltinMethod>> atomNodes =
getOrUpdate(methodNodes, builtinMethodOwner);
atomNodes.put(builtinMethodName, new CachingSupplier<>(builtin));
atomNodes.put(builtinMethodName, CachingSupplier.forValue(builtin));
});
});
return methodNodes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.enso.interpreter.runtime.data.atom.Atom;
import org.enso.interpreter.runtime.data.text.Text;
import org.enso.interpreter.runtime.data.vector.ArrayLikeHelpers;
import org.enso.interpreter.runtime.error.DataflowError;

/** Container for builtin Error types */
public final class Error {
Expand Down Expand Up @@ -110,16 +111,16 @@ public Error(Builtins builtins, EnsoContext context) {
mapError = builtins.getBuiltinType(MapError.class);
}

public Atom makeSyntaxError(Object message) {
return syntaxError.newInstance(message);
public Atom makeSyntaxError(String message) {
return syntaxError.newInstance(Text.create(message));
}

public Atom makeCompileError(Object message) {
return compileError.newInstance(message);
public Atom makeCompileError(String message) {
return compileError.newInstance(Text.create(message));
}

public Atom makeAssertionError(Text text) {
return assertionError.newInstance(text);
public Atom makeAssertionError(String text) {
return assertionError.newInstance(Text.create(text));
}

public Atom makeIndexOutOfBounds(long index, long length) {
Expand Down Expand Up @@ -350,4 +351,16 @@ public Atom makeNumberParseError(String message) {
public Atom makeMapError(long index, Object innerError) {
return mapError.newInstance(index, innerError);
}

/**
* Creates error on missing polyglot java import class.
*
* @param className the name of the class that is missing
* @return data flow error representing the missing value
*/
public DataflowError makeMissingPolyglotImportError(String className) {
var msg = "No polyglot symbol for " + className;
var err = makeCompileError(msg);
return DataflowError.withDefaultTrace(err, null);
}
}
Loading
Loading