Always call TruffleLanguage#initializeThread on the thread being initialized.

Author: jchalou

sulong/projects/com.oracle.truffle.llvm.runtime/src/com/oracle/truffle/llvm/runtime/LLVMContext.java

@@ -29,6 +29,27 @@
  */
 package com.oracle.truffle.llvm.runtime;
 
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.security.SecureRandom;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.logging.Level;
+import java.util.stream.Collectors;
+
+import org.graalvm.collections.EconomicMap;
+import org.graalvm.collections.EconomicSet;
+import org.graalvm.collections.Equivalence;
+import org.graalvm.collections.Pair;
+
 import com.oracle.truffle.api.Assumption;
 import com.oracle.truffle.api.CallTarget;
 import com.oracle.truffle.api.CompilerAsserts;
@@ -45,6 +66,7 @@
 import com.oracle.truffle.api.TruffleSafepoint;
 import com.oracle.truffle.api.nodes.ControlFlowException;
 import com.oracle.truffle.api.nodes.Node;
+import com.oracle.truffle.api.profiles.BranchProfile;
 import com.oracle.truffle.api.source.Source;
 import com.oracle.truffle.llvm.api.Toolchain;
 import com.oracle.truffle.llvm.runtime.IDGenerater.BitcodeID;
@@ -67,29 +89,6 @@
 import com.oracle.truffle.llvm.runtime.pointer.LLVMPointer;
 import com.oracle.truffle.llvm.runtime.pthread.LLVMPThreadContext;
 
-import org.graalvm.collections.EconomicMap;
-import org.graalvm.collections.Pair;
-
-import java.io.IOException;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.security.SecureRandom;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.locks.ReentrantLock;
-import java.util.logging.Level;
-import java.util.stream.Collectors;
-
-import com.oracle.truffle.api.profiles.BranchProfile;
-import org.graalvm.collections.EconomicSet;
-import org.graalvm.collections.Equivalence;
-
 public final class LLVMContext {
     public static final String SULONG_INIT_CONTEXT = "__sulong_init_context";
     public static final String SULONG_DISPOSE_CONTEXT = "__sulong_dispose_context";
@@ -552,7 +551,7 @@ private void cleanUpNoGuestCode() {
         }
 
         Thread[] allThreads;
-        try (TLSInitializerAccess access = getTLSInitializerAccess()) {
+        try (TLSInitializerAccess access = getTLSInitializerAccess(true)) {
             allThreads = access.getAllRunningThreads();
         }
 
@@ -1047,8 +1046,12 @@ public synchronized void unregisterThread(LLVMThread thread) {
     public final class TLSInitializerAccess implements AutoCloseable {
 
         @TruffleBoundary
-        private TLSInitializerAccess() {
-            threadInitLock.lock();
+        private TLSInitializerAccess(boolean interruptible) {
+            if (interruptible) {
+                TruffleSafepoint.setBlockedThreadInterruptible(null, ReentrantLock::lockInterruptibly, threadInitLock);
+            } else {
+                threadInitLock.lock();
+            }
         }
 
         @TruffleBoundary
@@ -1088,8 +1091,8 @@ public List<AggregateTLGlobalInPlaceNode> getThreadLocalGlobalInitializer() {
         }
     }
 
-    public TLSInitializerAccess getTLSInitializerAccess() {
-        return new TLSInitializerAccess();
+    public TLSInitializerAccess getTLSInitializerAccess(boolean interruptible) {
+        return new TLSInitializerAccess(interruptible);
     }
 
     @TruffleBoundary

sulong/projects/com.oracle.truffle.llvm.runtime/src/com/oracle/truffle/llvm/runtime/LLVMLanguage.java

@@ -416,7 +416,7 @@ public static LLVMLanguage get(Node node) {
     @Override
     protected void initializeThread(LLVMContext context, Thread thread) {
         getCapability(PlatformCapability.class).initializeThread(context, thread);
-        try (TLSInitializerAccess access = context.getTLSInitializerAccess()) {
+        try (TLSInitializerAccess access = context.getTLSInitializerAccess(true)) {
             // need to duplicate the thread local globals for this thread.
             for (AggregateTLGlobalInPlaceNode globalInitializer : access.getThreadLocalGlobalInitializer()) {
                 // TODO: use the call target of AggregateTLGlobalInPlaceNode, rather than the node
@@ -869,7 +869,7 @@ protected void disposeThreadNoGuestCode(LLVMContext context, Thread thread) {
             freeThreadLocalGlobal(threadLocalValue);
         }
 
-        try (TLSInitializerAccess access = context.getTLSInitializerAccess()) {
+        try (TLSInitializerAccess access = context.getTLSInitializerAccess(false)) {
             access.unregisterLiveThread(thread);
         }
     }

sulong/projects/com.oracle.truffle.llvm/src/com/oracle/truffle/llvm/initialization/InitializeGlobalNode.java

@@ -34,6 +34,8 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import org.graalvm.collections.Pair;
+
 import com.oracle.truffle.api.Truffle;
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.nodes.DirectCallNode;
@@ -62,8 +64,6 @@
 import com.oracle.truffle.llvm.runtime.types.Type;
 import com.oracle.truffle.llvm.runtime.types.Type.TypeOverflowException;
 
-import org.graalvm.collections.Pair;
-
 /**
  * {@link InitializeGlobalNode} initializes the value of all defined global symbols.
  *
@@ -99,7 +99,7 @@ public void execute(VirtualFrame frame, Pair<LLVMPointer, Long> roDataBase) {
         LLVMContext context = LLVMContext.get(this);
 
         // try-with-resources is not PE-safe (blocklisted method Throwable.addSuppressed)
-        TLSInitializerAccess access = context.getTLSInitializerAccess();
+        TLSInitializerAccess access = context.getTLSInitializerAccess(true);
         try {
             Thread[] threads = access.getAllRunningThreads();
             for (Thread thread : threads) {

tools/src/com.oracle.truffle.tools.profiler.test/src/com/oracle/truffle/tools/profiler/test/NoTagSamplingTest.java

@@ -25,6 +25,7 @@
 package com.oracle.truffle.tools.profiler.test;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import java.util.List;
@@ -54,42 +55,32 @@ public class NoTagSamplingTest {
 
     private static final String TEST_ROOT_NAME = LILRootNode.class.getName();
 
-    private static Semaphore await;
+    private static Semaphore awaitStart;
+    private static Semaphore awaitFinish;
 
     @Test
     public void testNoTagSampling() throws InterruptedException, ExecutionException {
         ExecutorService singleThread = Executors.newSingleThreadExecutor();
-        await = new Semaphore(0);
+        awaitStart = new Semaphore(0);
+        awaitFinish = new Semaphore(0);
         try (Context context = Context.create(NoTagLanguage.ID)) {
             CPUSampler sampler = CPUSampler.find(context.getEngine());
             Source source = Source.newBuilder(NoTagLanguage.ID, "", "").buildLiteral();
-            Future<?> f = singleThread.submit(() -> {
-                return context.eval(source).asInt();
-            });
-
-            Map<Thread, List<StackTraceEntry>> sample = null;
-            for (int i = 0; i < 10000; i++) { // times out after 10s
-                sample = sampler.takeSample();
-                if (!sample.isEmpty()) {
-                    break;
-                }
-                try {
-                    Thread.sleep(1);
-                } catch (InterruptedException e) {
-                    throw new AssertionError(e);
-                }
-            }
+            Future<?> f = singleThread.submit(() -> context.eval(source).asInt());
+
+            awaitStart.acquire();
+            Map<Thread, List<StackTraceEntry>> sample = sampler.takeSample();
 
             // wait for future
-            await.release();
+            awaitFinish.release();
             assertEquals(42, f.get());
-            assertTrue(!sample.isEmpty());
+            assertFalse(sample.isEmpty());
             List<StackTraceEntry> entries = sample.values().iterator().next();
             assertEquals(1, entries.size());
             assertEquals(TEST_ROOT_NAME, entries.get(0).getRootName());
 
             singleThread.shutdown();
-            singleThread.awaitTermination(10, TimeUnit.SECONDS);
+            assertTrue(singleThread.awaitTermination(10, TimeUnit.SECONDS));
         }
 
     }
@@ -107,7 +98,8 @@ public String getName() {
 
         @Override
         public Object execute(VirtualFrame frame) {
-            TruffleSafepoint.setBlockedThreadInterruptible(this, Semaphore::acquire, await);
+            awaitStart.release();
+            TruffleSafepoint.setBlockedThreadInterruptible(this, Semaphore::acquire, awaitFinish);
             return 42;
         }
     }

truffle/CHANGELOG.md

@@ -14,7 +14,9 @@ This changelog summarizes major changes between Truffle versions relevant to lan
 * GR-46293 The deprecated `com.oracle.truffle.api.library.DefaultExportProvider` and `com.oracle.truffle.api.library.EagerExportProvider` interfaces, deprecated since version 23.1, have now been removed. They are replaced by the `com.oracle.truffle.api.library.provider.DefaultExportProvider` and `com.oracle.truffle.api.library.provider.EagerExportProvider` interfaces. The implementations of these interfaces are automatically generated by the annotation processor and this change requires no source code modifications, only recompilation.
 * GR-59640 `InternalResource.versionHash(Env)` may now throw an `IOException` to indicate problems when reading the version from disk.
 * GR-54300 An inner context is now automatically closed when its corresponding creator `TruffleContext` is no longer strongly reachable. A reachable API `TruffleContext` will keep the associated creator `TruffleContext` reachable. However, it is still recommended not to rely on garbage collection for closing. Instead, use the try-with-resources pattern for explicit context management. For more information, refer to the [Automatic Close on GC documentation](https://github.com/oracle/graal/blob/master/truffle/docs/CloseOnGc.md).
-
+* GR-30264 `TruffleLanguage#initializeThread(Object, Thread)` is now guaranteed to be executed on the thread that is being initialized. Additional changes:  
+    * Added `ThreadLocalAction#notifyBlocked(Access)` and `ThreadLocalAction#notifyUnblocked(Access)` to notify thread local actions that their processing has been blocked/unblocked due to a blocked call (see `ThreadLocalAction` documentation).
+    * `TruffleSafepoint#poll(Node)` does not require a non-null location anymore. However, it is still recommended to always pass a location node, if available.  
 
 ## Version 24.1.0
 * GR-43839 Added optional parameter to TruffleString.ByteIndexOfCodePointSetNode to choose whether the node may calculate the input string's precise code range.

truffle/docs/Safepoints.md

@@ -51,6 +51,16 @@ env.submitThreadLocal(null, new ThreadLocalAction(true /*side-effecting*/, true
      protected void perform(Access access) {
          assert access.getThread() == Thread.currentThread();
      }
+
+    @Override
+    protected void notifyBlocked(Access access) {
+        assert access.getThread() == Thread.currentThread();
+    }
+
+    @Override
+    protected void notifyUnblocked(Access access) {
+        assert access.getThread() == Thread.currentThread();
+    }
 });
 
 ```
@@ -86,21 +96,24 @@ graalvm/bin/js --engine.SafepointALot js-benchmarks/harness.js -- octane-deltabl
 Prints the following output to the log on context close:
 
 ```
-DeltaBlue: 540
-[engine] Safepoint Statistics
-  --------------------------------------------------------------------------------------
-   Thread Name         Safepoints | Interval     Avg              Min              Max
-  --------------------------------------------------------------------------------------
-   main                  48384054 |            0.425 us           0.1 us       44281.1 us
-  -------------------------------------------------------------------------------------
-   All threads           48384054 |            0.425 us           0.1 us       42281.1 us
+DeltaBlue: 3037
+[engine] Safepoint Statistics 
+  ------------------------------------------------------------------------------------------------------------------------------------------------------- 
+   Thread Name         Safepoints | Interval     Avg              Min              Max      | Blocked Intervals   Avg              Min              Max
+  ------------------------------------------------------------------------------------------------------------------------------------------------------- 
+   main                  83187332 |            0,452 us           0,2 us      104938,8 us   |      18            6,536 us           0,9 us          36,8 us
+  ------------------------------------------------------------------------------------------------------------------------------------------------------- 
+   All threads           83187332 |            0,452 us           0,2 us      104938,8 us   |      18            6,536 us           0,9 us          36,8 us
 ```
 
 It is recommended for guest language implementations to try to stay below 1ms on average.
 Note that precise timing can depend on CPU and interruptions by the GC.
 Since GC times are included in the safepoint interval times, it is expected that the maximum is close to the maximum GC interruption time.
 Future versions of this tool will be able to exclude GC interruption times from this statistic.
 
+The SafepointALot tool also does not interrupt blocking operations that typically wait for some resource to be available (e.g. IO, thread start).
+These blocked intervals are shown separately in the statistics. Ordinary thread local actions do interrupt blocking operations, and so the blocked intervals do not apply to them.
+
 ### Find missing safepoint polls
 
 The option `TraceMissingSafepointPollInterval` helps to find missing safepoint polls, use it like: