Skip to content

Commit

Permalink
Removing need for asynchronous thread to execute ResourceManager fina…
Browse files Browse the repository at this point in the history
…lizers (#6335)

While Enso runs single-threaded, its `ResourceManager` required 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 submitting `ThreadLocalAction` that gets then picked and executed via `TruffleSafepoint` locations. 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.
  • Loading branch information
JaroslavTulach authored Apr 20, 2023
1 parent 4076a64 commit e47eb49
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 15 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,8 @@
- [One can define lazy atom fields][6151]
- [Replace IOContexts with Execution Environment and generic Context][6171]
- [Vector.sort handles incomparable types][5998]
- [Removing need for asynchronous thread to execute ResourceManager
finalizers][6335]

[3227]: https://github.com/enso-org/enso/pull/3227
[3248]: https://github.com/enso-org/enso/pull/3248
Expand Down Expand Up @@ -794,6 +796,7 @@
[6151]: https://github.com/enso-org/enso/pull/6151
[6171]: https://github.com/enso-org/enso/pull/6171
[5998]: https://github.com/enso-org/enso/pull/5998
[6335]: https://github.com/enso-org/enso/pull/6335

# Enso 2.0.0-alpha.18 (2021-10-12)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
return a + b;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package org.enso.interpreter.runtime;

import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.ThreadLocalAction;
import com.oracle.truffle.api.TruffleSafepoint;
import com.oracle.truffle.api.interop.InteropLibrary;
import org.enso.interpreter.runtime.data.ManagedResource;

Expand All @@ -9,8 +11,10 @@
import java.lang.ref.ReferenceQueue;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.Future;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;

/** Allows the context to attach garbage collection hooks on the removal of certain objects. */
public class ResourceManager {
Expand Down Expand Up @@ -59,7 +63,7 @@ public void unpark(ManagedResource resource) {
return;
}
it.getParkedCount().decrementAndGet();
tryFinalize(it);
scheduleFinalizationAtSafepoint(it);
}

/**
Expand All @@ -75,7 +79,7 @@ public void close(ManagedResource resource) {
return;
}
// Unconditional finalization – user controls the resource manually.
it.doFinalize(context);
it.finalizeNow(context);
}

/**
Expand All @@ -89,7 +93,7 @@ public void take(ManagedResource resource) {
items.remove(resource.getPhantomReference());
}

private void tryFinalize(Item it) {
private void scheduleFinalizationAtSafepoint(Item it) {
if (it.isFlaggedForFinalization().get()) {
if (it.getParkedCount().get() == 0) {
// We already know that isFlaggedForFinalization was true at some
Expand All @@ -101,8 +105,21 @@ private void tryFinalize(Item it) {
// no further attempts are made.
boolean continueFinalizing = it.isFlaggedForFinalization().compareAndSet(true, false);
if (continueFinalizing) {
it.doFinalize(context);
items.remove(it.reference);
var futureToCancel = new AtomicReference<Future<Void>>(null);
var performFinalizeNow =
new ThreadLocalAction(false, false, true) {
@Override
protected void perform(ThreadLocalAction.Access access) {
var tmp = futureToCancel.getAndSet(null);
if (tmp == null) {
return;
}
tmp.cancel(false);
it.finalizeNow(context);
items.remove(it.reference);
}
};
futureToCancel.set(context.getEnvironment().submitThreadLocal(null, performFinalizeNow));
}
}
}
Expand All @@ -124,7 +141,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);
workerThread.start();
}
ManagedResource resource = new ManagedResource(object);
Expand Down Expand Up @@ -158,7 +175,7 @@ public void shutdown() {
Item it = items.remove(key);
if (it != null) {
// Finalize unconditionally – all other threads are dead by now.
it.doFinalize(context);
it.finalizeNow(context);
}
}
}
Expand All @@ -181,7 +198,7 @@ public void run() {
continue;
}
it.isFlaggedForFinalization().set(true);
tryFinalize(it);
scheduleFinalizationAtSafepoint(it);
}
if (killed) {
return;
Expand Down Expand Up @@ -229,18 +246,16 @@ public Item(Object underlying, Object finalizer, PhantomReference<ManagedResourc
}

/**
* Unconditionally performs the finalization action of this resource.
* Performs the finalization action of this resource right now. The thread must be inside of a
* context.
*
* @param context current execution context
*/
public void doFinalize(EnsoContext context) {
Object p = context.getThreadManager().enter();
public void finalizeNow(EnsoContext context) {
try {
InteropLibrary.getUncached(finalizer).execute(finalizer, underlying);
} catch (Exception e) {
context.getErr().println("Exception in finalizer: " + e.getMessage());
} finally {
context.getThreadManager().leave(p);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ class InterpreterContext(
.newBuilder(LanguageInfo.ID)
.allowExperimentalOptions(true)
.allowAllAccess(true)
.allowCreateThread(false)
.out(output)
.err(err)
.option(RuntimeOptions.LOG_LEVEL, "WARNING")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,11 @@ class RuntimeManagementTest extends InterpreterTest {
totalOut = consumeOut
while (totalOut.length < expect && round < 500) {
round = round + 1
if (round % 10 == 0) forceGC();
if (round % 10 == 0) {
forceGC();
}
val res = eval("main a b = a * b").execute(7, 6)
assertResult(42)(res.asInt)
Thread.sleep(100)
totalOut ++= consumeOut
}
Expand Down

0 comments on commit e47eb49

Please sign in to comment.