Removing need for asynchronous thread to execute ResourceManager finalizers #6335
Conversation
| @@ -9,7 +9,7 @@ | |||
| final class InliningBuiltinsOutNode extends Node { | |||
|
|
|||
| long execute(VirtualFrame frame, long a, long b) { | |||
| Assert.assertNotNull("VirtualFrame is always provided " + frame); | |||
| Assert.assertNotNull("VirtualFrame is always provided", frame); | |||
There was a problem hiding this comment.
Unreleated. Addresses #6304 (comment)
| @@ -124,7 +129,7 @@ public ManagedResource register(Object object, Object function) { | |||
| } | |||
| if (workerThread == null || !workerThread.isAlive()) { | |||
| worker.setKilled(false); | |||
| workerThread = context.getEnvironment().createThread(worker); | |||
| workerThread = context.getEnvironment().createSystemThread(worker); | |||
There was a problem hiding this comment.
We need thread to read "finalizable" references from ReferenceQueue - however it can be "system thread" - a thread that is not allowed to execute any guest language code.
| @Override | ||
| protected void perform(ThreadLocalAction.Access access) { | ||
| var tmp = futureToCancel.getAndSet(null); | ||
| if (tmp == null) { |
There was a problem hiding this comment.
Rather than directly executing the finalizer we schedule a "safe point action". As soon as it is picked up by some Enso executing thread, we cancel any further executions of the action and execute the "finalizer".
| @@ -113,6 +113,7 @@ class InterpreterContext( | |||
| .newBuilder(LanguageInfo.ID) | |||
| .allowExperimentalOptions(true) | |||
| .allowAllAccess(true) | |||
| .allowCreateThread(false) | |||
There was a problem hiding this comment.
The changes in this PR remove the need for multiple thread. All runtime project tests can now run with single thread access only.
| if (round % 10 == 0) { | ||
| forceGC(); | ||
| val res = eval("main a b = a * b").execute(7, 6) | ||
| assertResult(42)(res.asInt) |
There was a problem hiding this comment.
However, to execute the "finalizer" some Truffle code must be executing. It wasn't in case of RuntimeManagementTest. To mitigate that, I am submitting some simple computation here. As soon as the computation reaches TruffleSafepoint, the "finalizers" are executed and the test can finish.
engine/runtime/src/main/java/org/enso/interpreter/runtime/ResourceManager.java
Outdated
Show resolved
Hide resolved
engine/runtime/src/main/java/org/enso/interpreter/runtime/ResourceManager.java
Outdated
Show resolved
Hide resolved
JaroslavTulach
force-pushed
the
wip/jtulach/SynchronousResourceManager
branch
from
378a5f5
to
4128784
Compare
engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/RuntimeManagementTest.scala
Show resolved
Hide resolved
engine/runtime/src/main/java/org/enso/interpreter/runtime/ResourceManager.java
Outdated
Show resolved
Hide resolved
…renceQueue.remove()
JaroslavTulach
added
the
CI: Ready to merge
|
Jaroslav Tulach reports a new STANDUP for yesterday (2023-04-20): Progress: - finalizers: #6335
Next Day: <| functions |
Pull Request Description
While Enso runs single-threaded, its
ResourceManagerrequired additional asynchronous thread to execute its "finalizers". What has been necessary back then is no longer needed since GraalVM 21.1. GraalVM now provides support for submittingThreadLocalActionthat gets then picked and executed viaTruffleSafepointlocations. This PR uses such mechanism to "inject" finalizer execution into already running Enso evaluation thread.Requiring more than one thread has complicated Enso's co-existence with other Truffle language. For example Graal.js is strictly singlethreaded and used to refuse (simple) co-existence with Enso. By allowing Enso to perform all its actions in a single thread, the synergy with Graal.js becomes better.
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
style guides
Contextclosing