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

Run Enso tests with assertions enabled #7882

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
a8d28a4
Run Enso tests with assertions enabled
Akirathan Sep 25, 2023
219ff91
Avoid assert when using ArrayLikeHelpers.allocate
JaroslavTulach Oct 2, 2023
429c9de
Fix BenchProcessor locating root directory
Akirathan Nov 17, 2023
dc16932
Throw AbstractTruffleException from ArrayLikeAtNode
Akirathan Nov 17, 2023
29d7963
ForeignEvalNode does not enter inner TruffleContext
Akirathan Nov 17, 2023
f4e5a82
Remove epb PolyglotProxy and co.
Akirathan Nov 17, 2023
f23fef3
Merge branch 'develop' into wip/akirathan/5585-Enable-Assertions-in-E…
Akirathan Nov 23, 2023
5bea01a
Merge branch 'develop' into wip/akirathan/5585-Enable-Assertions-in-E…
Akirathan Nov 27, 2023
09fa463
[WIP] Try to do global locking on EpbContext
Akirathan Dec 4, 2023
5177e0b
Merge with develop
JaroslavTulach Dec 12, 2023
189b335
Call super.init before accessing executionContext.context
JaroslavTulach Dec 12, 2023
2302fc8
Formatting
JaroslavTulach Dec 12, 2023
6ce3a1b
Still require inner context for JavaScript
JaroslavTulach Dec 12, 2023
c32d980
Just three failures in runtime-with-polyglot
JaroslavTulach Dec 12, 2023
9d96365
Don't modify InstrumentTestContext to avoid failures in runtime-with-…
JaroslavTulach Dec 12, 2023
88d8db2
Introducing EpbProxyValue to switch to the right context
JaroslavTulach Dec 12, 2023
630bf2d
Log conditionally
JaroslavTulach Dec 12, 2023
f5ba767
Only dispatch the InteropLibrary messages
JaroslavTulach Dec 12, 2023
97f4d76
Enabling asserts in runtime-with-polyglot project
JaroslavTulach Dec 13, 2023
216fc33
Verify JavaScript interop
JaroslavTulach Dec 13, 2023
cc8df8b
Better to leave switching between TruffleContexts to Truffle
JaroslavTulach Dec 13, 2023
39d28cd
Consistency between isMemberReadable and readMember as enforced by ru…
JaroslavTulach Dec 13, 2023
5b45c58
If there is encapsulating source section we have to find it when offe…
JaroslavTulach Dec 13, 2023
328ca8d
Make sure native image can be built
JaroslavTulach Dec 13, 2023
a9a57d8
Allow invocation of JavaScript methods from multiple threads. One by …
JaroslavTulach Dec 13, 2023
30de2ef
No longer supporting JavaScript invocation in parallel
JaroslavTulach Dec 13, 2023
b8d53e5
Merge remote-tracking branch 'origin/develop' into wip/akirathan/5585…
JaroslavTulach Dec 13, 2023
4eff8b4
Hide the EpbLanguage implementation into a single package
JaroslavTulach Dec 13, 2023
1876d8b
Use TruffleContext directly
JaroslavTulach Dec 13, 2023
985ad86
Supporting parseGeneric
JaroslavTulach Dec 13, 2023
1d4107a
Disable pre-initialization of JavaScript for now
JaroslavTulach Dec 13, 2023
acd896e
Formatting
JaroslavTulach Dec 13, 2023
f87c1f6
Verify libraries that polyglot-api project depends on aren't present …
JaroslavTulach Dec 14, 2023
d6c8398
Using PyForeignNode uses parseGeneric
JaroslavTulach Dec 14, 2023
3c2df09
Consolidate JsForeignNode
JaroslavTulach Dec 14, 2023
4a46c23
Formatting build.sbt
JaroslavTulach Dec 14, 2023
3fa992a
Avoid using Meta.is_same_object in Sieve implementations
JaroslavTulach Dec 14, 2023
0e0dc6e
Don't forget to import site to initialize Python's venv
JaroslavTulach Dec 14, 2023
0e8dc8e
Avoid NPE in assert
JaroslavTulach Dec 14, 2023
3f63d9e
No need for arity
JaroslavTulach Dec 14, 2023
ef1c798
Disable pre-initialization of JavaScript for now
JaroslavTulach Dec 14, 2023
8fda9f5
Reverting debug only comment
JaroslavTulach Dec 14, 2023
8f08e61
No special test options are needed
JaroslavTulach Dec 14, 2023
78d603b
Concentrate parsing into ForeingEvalNode
JaroslavTulach Dec 14, 2023
faaffc1
Merging with develop branch
JaroslavTulach Dec 14, 2023
b019877
Don't convert long to EnsoBigInteger as that asserts
JaroslavTulach Dec 15, 2023
61422e0
Associate InstrumentorEvalNode with instance of EnsoLanguage
JaroslavTulach Dec 15, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 12 additions & 8 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -1178,11 +1178,6 @@ val truffleRunOpts = Seq(
"-Dpolyglot.compiler.BackgroundCompilation=false"
)

val truffleRunOptionsNoAssertSettings = Seq(
Copy link
Member

Choose a reason for hiding this comment

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

Removed as we always run with -ea.

fork := true,
javaOptions ++= benchOnlyOptions
)

val truffleRunOptionsSettings = Seq(
fork := true,
javaOptions ++= "-ea" +: benchOnlyOptions
Expand Down Expand Up @@ -1375,11 +1370,20 @@ lazy val instrumentationSettings = frgaalJavaCompilerSetting ++ Seq(
lazy val `runtime-language-epb` =
(project in file("engine/runtime-language-epb"))
.settings(
frgaalJavaCompilerSetting,
inConfig(Compile)(truffleRunOptionsSettings),
truffleDslSuppressWarnsSetting,
instrumentationSettings
commands += WithDebugCommand.withDebug,
fork := true,
Test / javaOptions ++= Seq(),
instrumentationSettings,
libraryDependencies ++= Seq(
"junit" % "junit" % junitVersion % Test,
"com.github.sbt" % "junit-interface" % junitIfVersion % Test,
"org.graalvm.truffle" % "truffle-api" % graalMavenPackagesVersion % "provided",
"org.graalvm.truffle" % "truffle-dsl-processor" % graalMavenPackagesVersion % "provided"
)
)
.dependsOn(`polyglot-api`)
Copy link
Member

Choose a reason for hiding this comment

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

There is no need why EpbLanguage shall depend on Scala & other heavyweight dependencies in the polyglot-api project. Test written in f87c1f6 shall verify some of the enormous set of dependencies of polyglot-api aren't present.


lazy val runtime = (project in file("engine/runtime"))
.configs(Benchmark)
Expand Down Expand Up @@ -1727,7 +1731,7 @@ lazy val `runtime-with-polyglot` =
.configs(Benchmark)
.settings(
frgaalJavaCompilerSetting,
inConfig(Compile)(truffleRunOptionsNoAssertSettings),
inConfig(Compile)(truffleRunOptionsSettings),
inConfig(Benchmark)(Defaults.testSettings),
commands += WithDebugCommand.withDebug,
Benchmark / javacOptions --= Seq(
Expand Down
4 changes: 2 additions & 2 deletions build/build/src/enso.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ impl From<bool> for Boolean {
}

ide_ci::define_env_var! {
ENSO_JVM_OPTS, String;
JaroslavTulach marked this conversation as resolved.
Show resolved Hide resolved
JAVA_OPTS, String;
ENSO_BENCHMARK_TEST_DRY_RUN, Boolean;
}

Expand Down Expand Up @@ -88,7 +88,7 @@ impl BuiltEnso {
.arg(test_path)
// This flag enables assertions in the JVM. Some of our stdlib tests had in the past
// failed on Graal/Truffle assertions, so we want to have them triggered.
.set_env(ENSO_JVM_OPTS, &ide_ci::programs::java::Option::EnableAssertions.as_ref())?;
.set_env(JAVA_OPTS, &ide_ci::programs::java::Option::EnableAssertions.as_ref())?;
Ok(command)
}

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import org.enso.compiler.pass.analyse.{
}
import org.enso.compiler.pass.lint.UnusedBindings
import org.enso.compiler.pass.optimise.LambdaConsolidate
import org.enso.polyglot.ForeignLanguage

import scala.annotation.tailrec

Expand Down Expand Up @@ -161,7 +160,7 @@ case object GenerateMethodBodies extends IRPass {
case lam @ Function.Lambda(_, body, _, _, _, _)
if findForeignDefinition(
body,
lang = Some(ForeignLanguage.JS)
lang = Some("js")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we keep that as a constant somewhere, please?

Copy link
Member

Choose a reason for hiding this comment

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

Too much overhead to do so. We would need a new project sharable between all that were using ForeginLanguage.JS. But I want to get rid of the heavyweight polyglot-api dependency (in EpbLanguage at least). I don't want to create new project just to host js constant.

Btw. the real constant is defined in graal-js and we don't compile against such module. E.g. there was never any guarantee (except tests) that we are using the right constant.

).isDefined =>
val thisArgs = chainedFunctionArgs.collect {
case (arg, idx) if arg.name.name == "this" =>
Expand Down Expand Up @@ -308,7 +307,7 @@ case object GenerateMethodBodies extends IRPass {
@tailrec
private def findForeignDefinition(
body: Expression,
lang: Option[ForeignLanguage]
lang: Option[String]
): Option[Foreign.Definition] = {
body match {
case foreignDef: Foreign.Definition =>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package org.enso.interpreter.epb.node;
package org.enso.interpreter.epb;

import com.oracle.truffle.api.dsl.Cached;
import com.oracle.truffle.api.dsl.GenerateUncached;
Expand All @@ -10,7 +10,7 @@

@GenerateUncached
@ReportPolymorphism
public abstract class CoercePrimitiveNode extends Node {
abstract class CoercePrimitiveNode extends Node {

/**
* Create a new node responsible for coercing primitive values to Enso primitives at the polyglot
Expand Down
Original file line number Diff line number Diff line change
@@ -1,38 +1,38 @@
package org.enso.interpreter.epb;

import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
import com.oracle.truffle.api.TruffleContext;
import com.oracle.truffle.api.TruffleLanguage;
import com.oracle.truffle.api.TruffleLogger;
import com.oracle.truffle.api.nodes.Node;
import java.util.concurrent.CountDownLatch;
import java.util.function.Consumer;
import java.util.concurrent.locks.ReentrantLock;
import java.util.logging.Level;
import org.enso.interpreter.epb.runtime.GuardedTruffleContext;

/**
* A context for {@link EpbLanguage}. Provides access to both isolated Truffle contexts used in
* polyglot execution.
*/
public class EpbContext {
final class EpbContext {

private static final TruffleLanguage.ContextReference<EpbContext> REFERENCE =
TruffleLanguage.ContextReference.create(EpbLanguage.class);

private static final String INNER_OPTION = "isEpbInner";
private final boolean isInner;
private final TruffleLanguage.Env env;
private @CompilerDirectives.CompilationFinal GuardedTruffleContext innerContext;
private final GuardedTruffleContext currentContext;
private @CompilationFinal TruffleContext innerContext;
private final ReentrantLock lock = new ReentrantLock();
private final TruffleLogger log;

/**
* Creates a new instance of this context.
*
* @param env the current language environment.
*/
public EpbContext(TruffleLanguage.Env env) {
EpbContext(TruffleLanguage.Env env) {
this.env = env;
isInner = env.getConfig().get(INNER_OPTION) != null;
currentContext = new GuardedTruffleContext(env.getContext(), isInner);
this.log = env.getLogger(EpbContext.class);
}

/**
Expand All @@ -45,58 +45,15 @@ public void initialize(String preInitializeLanguages) {
if (!isInner) {
if (innerContext == null) {
innerContext =
new GuardedTruffleContext(
env.newInnerContextBuilder()
.initializeCreatorContext(true)
.inheritAllAccess(true)
.config(INNER_OPTION, "yes")
.build(),
true);
env.newInnerContextBuilder()
.initializeCreatorContext(true)
.inheritAllAccess(true)
.config(INNER_OPTION, "yes")
.build();
}
initializeLanguages(env, innerContext, preInitializeLanguages);
}
}

private static void initializeLanguages(
TruffleLanguage.Env environment, GuardedTruffleContext innerContext, String langs) {
if (langs == null || langs.isEmpty()) {
return;
}
var log = environment.getLogger(EpbContext.class);
log.log(Level.FINE, "Initializing languages {0}", langs);
var cdl = new CountDownLatch(1);
var run =
(Consumer<TruffleContext>)
(context) -> {
var lock = innerContext.enter(null);
try {
log.log(Level.FINEST, "Entering initialization thread");
cdl.countDown();
for (var l : langs.split(",")) {
log.log(Level.FINEST, "Initializing language {0}", l);
long then = System.currentTimeMillis();
var res = context.initializeInternal(null, l);
long took = System.currentTimeMillis() - then;
log.log(
Level.FINE,
"Done initializing language {0} with {1} in {2} ms",
new Object[] {l, res, took});
}
} finally {
innerContext.leave(null, lock);
}
};
var init = innerContext.createThread(environment, run);
log.log(Level.FINEST, "Starting initialization thread");
init.start();
try {
cdl.await();
} catch (InterruptedException ex) {
Thread.currentThread().interrupt();
}
log.log(Level.FINEST, "Initializing on background");
}

/**
* @param node the location of context access. Pass {@code null} if not in a node.
* @return the proper context instance for the current {@link
Expand All @@ -106,30 +63,16 @@ public static EpbContext get(Node node) {
return REFERENCE.get(node);
}

/**
* Checks if this context corresponds to the inner Truffle context.
*
* @return true if run in the inner Truffle context, false otherwise.
*/
public boolean isInner() {
return isInner;
/** @return the language environment associated with this context. */
public TruffleLanguage.Env getEnv() {
return env;
}

/**
* @return the inner Truffle context handle if called from the outer context, or null if called in
* the inner context.
*/
public GuardedTruffleContext getInnerContext() {
public TruffleContext getInnerContext() {
return innerContext;
}

/** @return returns the currently entered Truffle context handle. */
public GuardedTruffleContext getCurrentContext() {
return currentContext;
}

/** @return the language environment associated with this context. */
public TruffleLanguage.Env getEnv() {
return env;
public void log(Level level, String msg, Object... args) {
this.log.log(level, msg, args);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,41 +3,17 @@
import com.oracle.truffle.api.CallTarget;
import com.oracle.truffle.api.TruffleLanguage;
import java.util.function.Consumer;
import org.enso.interpreter.epb.node.ContextRewrapNode;
import org.enso.interpreter.epb.node.ForeignEvalNode;
import org.enso.polyglot.ForeignLanguage;

/**
* An internal language that serves as a bridge between Enso and other supported languages.
*
* <p>Truffle places a lot of emphasis on safety guarantees, which also means that single-threaded
* languages cannot easily be called from multiple threads. We circumvent this by using two separate
* {@link com.oracle.truffle.api.TruffleContext}s, one (often referred to as "outer") is allowed to
* run Enso, Host Java, and possibly other thread-ready languages. Languages that cannot safely run
* in a multithreaded environment are relegated to the other context (referred to as "inner"). The
* inner context provides a GIL capability, ensuring that access to the single-threaded languages is
* serialized.
*
* <p>This imposes certain limitations on data interchange between the contexts. In particular, it
* is impossible to execute origin language's code when executing in the other context. Therefore
* outer context values need to be specially wrapped before being passed (e.g. as arguments) to the
* inner context, and inner context values need rewrapping for use in the outer context. See {@link
* org.enso.interpreter.epb.runtime.PolyglotProxy} and {@link ContextRewrapNode} for details of how
* and when this wrapping is done.
*
* <p>With the structure outlined above, EPB is the only language that is initialized in both inner
* and outer contexts and thus it is very minimal. Its only role is to manage both contexts and
* provide context-switching facilities.
*/
/** An internal language that serves as a bridge between Enso and other supported languages. */
@TruffleLanguage.Registration(
Comment on lines -10 to 8
Copy link
Member

Choose a reason for hiding this comment

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

Why is this whole block of documentation removed?

Copy link
Member

Choose a reason for hiding this comment

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

Done by @Akirathan, but I'd say it no longer reflects reality. Times changed and we no longer have to implement this ourselves, but rather rely on Truffle to do it for us.

Copy link
Member

Choose a reason for hiding this comment

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

Btw. @kustosz asked why do we need this EpbLanguage anymore? True, possibly we could do without it.

id = ForeignLanguage.ID,
id = "epb",
Copy link
Contributor

Choose a reason for hiding this comment

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

EpbLanguage constant?

Copy link
Member

Choose a reason for hiding this comment

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

Needless. There is only one use of such a constant. Others are in modules that don't compile against this module:

image

They wouldn't be able to use the constant anyway. The fact that this all holds together is ensured by tests.

name = "Enso Polyglot Bridge",
characterMimeTypes = {EpbLanguage.MIME},
internal = true,
defaultMimeType = EpbLanguage.MIME,
contextPolicy = TruffleLanguage.ContextPolicy.SHARED,
services = Consumer.class)
public class EpbLanguage extends TruffleLanguage<EpbContext> {
public final class EpbLanguage extends TruffleLanguage<EpbContext> {
public static final String MIME = "application/epb";

@Override
Expand All @@ -55,9 +31,8 @@ protected void initializeContext(EpbContext context) {

@Override
protected CallTarget parse(ParsingRequest request) {
EpbParser.Result code = EpbParser.parse(request.getSource());
ForeignEvalNode foreignEvalNode = ForeignEvalNode.build(this, code, request.getArgumentNames());
return foreignEvalNode.getCallTarget();
var node = ForeignEvalNode.parse(this, request.getSource(), request.getArgumentNames());
return node.getCallTarget();
}

@Override
Expand Down

This file was deleted.