From fc5279d53f8c36d8ed5d1365c406da4e5bf514f9 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Fri, 18 Jun 2021 15:58:59 +0200 Subject: [PATCH 1/7] [GR-32011] Disallow side-effecting safepoints for killOtherFibers() * Needs GR-32204 and GR-32205 Truffle fixes. (cherry picked from commit abc994a3bcc7d1691c0f0dcd7824ed0306ca3dc7) --- .../truffleruby/core/fiber/FiberManager.java | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/truffleruby/core/fiber/FiberManager.java b/src/main/java/org/truffleruby/core/fiber/FiberManager.java index 704a8fe8a1bc..1b3cf1e295da 100644 --- a/src/main/java/org/truffleruby/core/fiber/FiberManager.java +++ b/src/main/java/org/truffleruby/core/fiber/FiberManager.java @@ -15,6 +15,7 @@ import java.util.concurrent.CountDownLatch; import com.oracle.truffle.api.TruffleContext; +import com.oracle.truffle.api.TruffleSafepoint; import org.truffleruby.RubyContext; import org.truffleruby.RubyLanguage; import org.truffleruby.core.DummyNode; @@ -357,11 +358,19 @@ public void killOtherFibers() { // This method might not be executed on the rootFiber Java Thread but possibly on another Java Thread. - final TruffleContext truffleContext = context.getEnv().getContext(); - context.getThreadManager().leaveAndEnter(truffleContext, DummyNode.INSTANCE, () -> { - doKillOtherFibers(); - return BlockingAction.SUCCESS; - }); + // Disallow side-effecting safepoints, the current thread is cleaning up and terminating. + // It can no longer process any exception or guest code. + final TruffleSafepoint safepoint = TruffleSafepoint.getCurrent(); + boolean allowSideEffects = safepoint.setAllowSideEffects(false); + try { + final TruffleContext truffleContext = context.getEnv().getContext(); + context.getThreadManager().leaveAndEnter(truffleContext, DummyNode.INSTANCE, () -> { + doKillOtherFibers(); + return BlockingAction.SUCCESS; + }); + } finally { + safepoint.setAllowSideEffects(allowSideEffects); + } } private void doKillOtherFibers() { From 375db73b0e4659fec527c2ea47efa42eaa907d03 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Fri, 25 Jun 2021 17:07:31 +0200 Subject: [PATCH 2/7] [GR-32154] Add test for --cpusampler + Thread#kill (cherry picked from commit ed1b470f2faa8b98ba23ebc901283d8739aaff69) --- spec/tags/truffle/tools_tags.txt | 1 + spec/truffle/tools_spec.rb | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/spec/tags/truffle/tools_tags.txt b/spec/tags/truffle/tools_tags.txt index 3ec4b54ae9f1..cfc77413abd3 100644 --- a/spec/tags/truffle/tools_tags.txt +++ b/spec/tags/truffle/tools_tags.txt @@ -1,3 +1,4 @@ slow:Tools --coverage is available and works slow:Tools --coverage works for internal sources slow:Tools --engine.TraceCompilation works and outputs to STDERR +slow:Tools --cpusampler works if Thread#kill is used diff --git a/spec/truffle/tools_spec.rb b/spec/truffle/tools_spec.rb index 65e7f4b9eedc..185987cff090 100644 --- a/spec/truffle/tools_spec.rb +++ b/spec/truffle/tools_spec.rb @@ -53,4 +53,26 @@ $?.should.success? end end + + describe "--cpusampler" do + it "works if Thread#kill is used" do + code = <<~RUBY + def foo + n = 0 + loop { itself { n += 1 } } + n + end + t = Thread.new { foo } + sleep 1 + p :kill + t.kill + t.join + RUBY + out = ruby_exe(code, options: "--cpusampler --cpusampler.Mode=roots") + out.should.include?(":kill") + out.should.include?("block in Object#foo") + out.should_not.include?('KillException') + $?.should.success? + end + end end From 0d0b6121cee2d17427d6dce9bba9dbbcbe5f1373 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Fri, 2 Jul 2021 18:32:08 +0200 Subject: [PATCH 3/7] Update graal to the release branch to get fixes --- common.json | 18 +++++++++--------- mx.truffleruby/suite.py | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/common.json b/common.json index 30269f61440c..e338e2047543 100644 --- a/common.json +++ b/common.json @@ -2,20 +2,20 @@ "README": "This file contains definitions that are useful for the hocon and jsonnet CI files of multiple repositories.", "jdks": { - "openjdk8": {"name": "openjdk", "version": "8u302+02-jvmci-21.2-b02", "platformspecific": true }, - "oraclejdk8": {"name": "oraclejdk", "version": "8u301+05-jvmci-21.2-b02", "platformspecific": true }, - "oraclejdk8Debug": {"name": "oraclejdk", "version": "8u301+05-jvmci-21.2-b02-fastdebug", "platformspecific": true }, + "openjdk8": {"name": "openjdk", "version": "8u302+06-jvmci-21.2-b05", "platformspecific": true }, + "oraclejdk8": {"name": "oraclejdk", "version": "8u301+09-jvmci-21.2-b05", "platformspecific": true }, + "oraclejdk8Debug": {"name": "oraclejdk", "version": "8u301+09-jvmci-21.2-b05-fastdebug", "platformspecific": true }, "openjdk11": {"name": "openjdk", "version": "11.0.11+9", "platformspecific": true }, "oraclejdk11": {"name": "oraclejdk", "version": "11.0.11+9", "platformspecific": true }, - "labsjdk-ce-11": {"name": "labsjdk", "version": "ce-11.0.11+8-jvmci-21.2-b02", "platformspecific": true }, - "labsjdk-ee-11": {"name": "labsjdk", "version": "ee-11.0.11+9-jvmci-21.2-b02", "platformspecific": true }, + "labsjdk-ce-11": {"name": "labsjdk", "version": "ce-11.0.12+5-jvmci-21.2-b05", "platformspecific": true }, + "labsjdk-ee-11": {"name": "labsjdk", "version": "ee-11.0.12+8-jvmci-21.2-b05", "platformspecific": true }, "oraclejdk16": {"name": "oraclejdk", "version": "16.0.1+4", "platformspecific": true }, - "labsjdk-ce-16": {"name": "labsjdk", "version": "ce-16.0.2+4-jvmci-21.2-b02", "platformspecific": true }, - "labsjdk-ce-16Debug": {"name": "labsjdk", "version": "ce-16.0.2+4-jvmci-21.2-b02-debug", "platformspecific": true }, - "labsjdk-ee-16": {"name": "labsjdk", "version": "ee-16.0.2+4-jvmci-21.2-b02", "platformspecific": true }, - "labsjdk-ee-16Debug": {"name": "labsjdk", "version": "ee-16.0.2+4-jvmci-21.2-b02-debug", "platformspecific": true } + "labsjdk-ce-16": {"name": "labsjdk", "version": "ce-16.0.2+7-jvmci-21.2-b05", "platformspecific": true }, + "labsjdk-ce-16Debug": {"name": "labsjdk", "version": "ce-16.0.2+7-jvmci-21.2-b05-debug", "platformspecific": true }, + "labsjdk-ee-16": {"name": "labsjdk", "version": "ee-16.0.2+7-jvmci-21.2-b05", "platformspecific": true }, + "labsjdk-ee-16Debug": {"name": "labsjdk", "version": "ee-16.0.2+7-jvmci-21.2-b05-debug", "platformspecific": true } }, "COMMENT" : "The devkits versions reflect those used to build the JVMCI JDKs (e.g., see devkit_platform_revisions in /make/conf/jib-profiles.js)", diff --git a/mx.truffleruby/suite.py b/mx.truffleruby/suite.py index b7376c36aedd..940f11a637e5 100644 --- a/mx.truffleruby/suite.py +++ b/mx.truffleruby/suite.py @@ -7,7 +7,7 @@ { "name": "regex", "subdir": True, - "version": "dbe78abd4c604ca50f639d6a4c1eba0411166229", + "version": "371edf7da6456f063bfe2a6b35b4335364d95d7e", "urls": [ {"url": "https://github.com/oracle/graal.git", "kind": "git"}, {"url": "https://curio.ssw.jku.at/nexus/content/repositories/snapshots", "kind": "binary"}, @@ -16,7 +16,7 @@ { "name": "sulong", "subdir": True, - "version": "dbe78abd4c604ca50f639d6a4c1eba0411166229", + "version": "371edf7da6456f063bfe2a6b35b4335364d95d7e", "urls": [ {"url": "https://github.com/oracle/graal.git", "kind": "git"}, {"url": "https://curio.ssw.jku.at/nexus/content/repositories/snapshots", "kind": "binary"}, From 5f78ee1cac93feaef8747375cbfa60dab85fe203 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 29 Jun 2021 17:17:21 +0200 Subject: [PATCH 4/7] System.identityHashCode() no longer needs a TruffleBoundary * It used to be needed on SubstrateVM, but not anymore. (cherry picked from commit 55171c9f8ed5988c41e6049720b5d219d6bf9263) --- src/main/java/org/truffleruby/core/kernel/KernelNodes.java | 3 --- src/main/java/org/truffleruby/debug/TruffleDebugNodes.java | 3 --- 2 files changed, 6 deletions(-) diff --git a/src/main/java/org/truffleruby/core/kernel/KernelNodes.java b/src/main/java/org/truffleruby/core/kernel/KernelNodes.java index d8eaebac5b32..7fcbad5f4afa 100644 --- a/src/main/java/org/truffleruby/core/kernel/KernelNodes.java +++ b/src/main/java/org/truffleruby/core/kernel/KernelNodes.java @@ -965,19 +965,16 @@ protected long hashBignum(RubyBignum value) { return HashOperations.hashBignum(value, getContext(), this); } - @TruffleBoundary @Specialization protected int hash(Nil self) { return System.identityHashCode(self); } - @TruffleBoundary @Specialization protected int hashEncoding(RubyEncoding self) { return System.identityHashCode(self); } - @TruffleBoundary @Specialization(guards = "!isRubyBignum(self)") protected int hash(RubyDynamicObject self) { return System.identityHashCode(self); diff --git a/src/main/java/org/truffleruby/debug/TruffleDebugNodes.java b/src/main/java/org/truffleruby/debug/TruffleDebugNodes.java index 27728a0619fc..7dfbd85774f6 100644 --- a/src/main/java/org/truffleruby/debug/TruffleDebugNodes.java +++ b/src/main/java/org/truffleruby/debug/TruffleDebugNodes.java @@ -549,13 +549,11 @@ public abstract static class ForeignObjectNode extends CoreMethodArrayArgumentsN @ExportLibrary(InteropLibrary.class) public static class ForeignObject implements TruffleObject { - @TruffleBoundary @ExportMessage protected TriState isIdenticalOrUndefined(Object other) { return other instanceof ForeignObject ? TriState.valueOf(this == other) : TriState.UNDEFINED; } - @TruffleBoundary @ExportMessage protected int identityHashCode() { return System.identityHashCode(this); @@ -567,7 +565,6 @@ protected String toDisplayString(boolean allowSideEffects) { } } - @TruffleBoundary @Specialization protected Object foreignObject() { return new ForeignObject(); From 309b411150a74b0d0f1440dda0eff6197566dded Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 29 Jun 2021 17:19:13 +0200 Subject: [PATCH 5/7] Fix specializations in Kernel#hash to cover all Ruby values (cherry picked from commit ef7c3dc057c056364ffb2a7a79ed373d5eb953f3) --- .../java/org/truffleruby/core/kernel/KernelNodes.java | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/truffleruby/core/kernel/KernelNodes.java b/src/main/java/org/truffleruby/core/kernel/KernelNodes.java index 7fcbad5f4afa..19de631c5029 100644 --- a/src/main/java/org/truffleruby/core/kernel/KernelNodes.java +++ b/src/main/java/org/truffleruby/core/kernel/KernelNodes.java @@ -933,7 +933,6 @@ protected boolean isFrozen(Object self, @CoreMethod(names = "hash") public abstract static class HashNode extends CoreMethodArrayArgumentsNode { - public static HashNode create() { return KernelNodesFactory.HashNodeFactory.create(null); } @@ -965,17 +964,12 @@ protected long hashBignum(RubyBignum value) { return HashOperations.hashBignum(value, getContext(), this); } - @Specialization - protected int hash(Nil self) { + @Specialization(guards = "!isRubyBignum(self)") + protected int hash(ImmutableRubyObject self) { return System.identityHashCode(self); } @Specialization - protected int hashEncoding(RubyEncoding self) { - return System.identityHashCode(self); - } - - @Specialization(guards = "!isRubyBignum(self)") protected int hash(RubyDynamicObject self) { return System.identityHashCode(self); } From e7b1cbac35b1089261823ffbd67993578657d20f Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 29 Jun 2021 17:22:12 +0200 Subject: [PATCH 6/7] Simplify ToHashByHashCode (cherry picked from commit 9ed5948f4a46ccbb30c0d9cb2ee1ff6bfb22ffae) --- .../java/org/truffleruby/core/hash/HashingNodes.java | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/truffleruby/core/hash/HashingNodes.java b/src/main/java/org/truffleruby/core/hash/HashingNodes.java index a2a532a59e07..ea5f211f064b 100644 --- a/src/main/java/org/truffleruby/core/hash/HashingNodes.java +++ b/src/main/java/org/truffleruby/core/hash/HashingNodes.java @@ -11,6 +11,7 @@ import com.oracle.truffle.api.dsl.Cached; import com.oracle.truffle.api.dsl.CachedContext; +import com.oracle.truffle.api.dsl.Fallback; import com.oracle.truffle.api.dsl.GenerateUncached; import com.oracle.truffle.api.dsl.Specialization; import org.truffleruby.RubyContext; @@ -25,7 +26,6 @@ import org.truffleruby.core.symbol.SymbolNodes; import org.truffleruby.core.string.ImmutableRubyString; import org.truffleruby.language.RubyBaseNode; -import org.truffleruby.language.RubyGuards; import org.truffleruby.language.dispatch.DispatchNode; public abstract class HashingNodes { @@ -109,24 +109,18 @@ protected int hashImmutableString(ImmutableRubyString value, return (int) stringHashNode.execute(value); } - @Specialization protected int hashSymbol(RubySymbol value, @Cached SymbolNodes.HashSymbolNode symbolHashNode) { return (int) symbolHashNode.execute(value); } - @Specialization(guards = "!isSpecialized(value)") + @Fallback protected int hash(Object value, @Cached DispatchNode callHash, @Cached HashCastResultNode cast) { return cast.execute(callHash.call(value, "hash")); } - - protected static boolean isSpecialized(Object value) { - return RubyGuards.isPrimitive(value) || value instanceof RubyBignum || value instanceof RubyString || - value instanceof ImmutableRubyString || value instanceof RubySymbol; - } } @GenerateUncached From c6acb0f6e8d7385ab599128c055dff66e1655e75 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 29 Jun 2021 17:29:06 +0200 Subject: [PATCH 7/7] Make KernelNodes.HashNode and HashingNodes.ToHashByHashCode consistent (cherry picked from commit 423aa6ebf0afa65bb86e1bd99314dfb67a58558d) --- .../truffleruby/core/hash/HashingNodes.java | 3 +- .../truffleruby/core/kernel/KernelNodes.java | 38 +++++++++++++------ 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/truffleruby/core/hash/HashingNodes.java b/src/main/java/org/truffleruby/core/hash/HashingNodes.java index ea5f211f064b..bfe05b8ed2d5 100644 --- a/src/main/java/org/truffleruby/core/hash/HashingNodes.java +++ b/src/main/java/org/truffleruby/core/hash/HashingNodes.java @@ -58,6 +58,7 @@ protected int hashCompareByIdentity(Object key, boolean compareByIdentity, } // MRI: any_hash + /** Keep consistent with {@link org.truffleruby.core.kernel.KernelNodes.HashNode} */ @GenerateUncached public abstract static class ToHashByHashCode extends RubyBaseNode { @@ -116,7 +117,7 @@ protected int hashSymbol(RubySymbol value, } @Fallback - protected int hash(Object value, + protected int hashOther(Object value, @Cached DispatchNode callHash, @Cached HashCastResultNode cast) { return cast.execute(callHash.call(value, "hash")); diff --git a/src/main/java/org/truffleruby/core/kernel/KernelNodes.java b/src/main/java/org/truffleruby/core/kernel/KernelNodes.java index 19de631c5029..a7f9dc71c025 100644 --- a/src/main/java/org/truffleruby/core/kernel/KernelNodes.java +++ b/src/main/java/org/truffleruby/core/kernel/KernelNodes.java @@ -17,6 +17,7 @@ import java.util.concurrent.TimeUnit; import com.oracle.truffle.api.dsl.CachedContext; +import com.oracle.truffle.api.dsl.Fallback; import com.oracle.truffle.api.frame.Frame; import com.oracle.truffle.api.utilities.AssumedValue; import org.jcodings.specific.UTF8Encoding; @@ -83,6 +84,7 @@ import org.truffleruby.core.support.TypeNodes.ObjectInstanceVariablesNode; import org.truffleruby.core.support.TypeNodesFactory.ObjectInstanceVariablesNodeFactory; import org.truffleruby.core.symbol.RubySymbol; +import org.truffleruby.core.symbol.SymbolNodes; import org.truffleruby.core.symbol.SymbolTable; import org.truffleruby.core.thread.GetCurrentRubyThreadNode; import org.truffleruby.core.thread.RubyThread; @@ -930,6 +932,7 @@ protected boolean isFrozen(Object self, } + /** Keep consistent with {@link org.truffleruby.core.hash.HashingNodes.ToHashByHashCode} */ @CoreMethod(names = "hash") public abstract static class HashNode extends CoreMethodArrayArgumentsNode { @@ -940,23 +943,23 @@ public static HashNode create() { public abstract Object execute(Object value); @Specialization - protected long hash(int value) { - return HashOperations.hashLong(value, getContext(), this); + protected long hashBoolean(boolean value) { + return HashOperations.hashBoolean(value, getContext(), this); } @Specialization - protected long hash(long value) { + protected long hashInt(int value) { return HashOperations.hashLong(value, getContext(), this); } @Specialization - protected long hash(double value) { - return HashOperations.hashDouble(value, getContext(), this); + protected long hashLong(long value) { + return HashOperations.hashLong(value, getContext(), this); } @Specialization - protected long hash(boolean value) { - return HashOperations.hashBoolean(value, getContext(), this); + protected long hashDouble(double value) { + return HashOperations.hashDouble(value, getContext(), this); } @Specialization @@ -964,13 +967,26 @@ protected long hashBignum(RubyBignum value) { return HashOperations.hashBignum(value, getContext(), this); } - @Specialization(guards = "!isRubyBignum(self)") - protected int hash(ImmutableRubyObject self) { - return System.identityHashCode(self); + @Specialization + protected long hashString(RubyString value, + @Cached StringNodes.HashStringNode stringHashNode) { + return stringHashNode.execute(value); + } + + @Specialization + protected long hashImmutableString(ImmutableRubyString value, + @Cached StringNodes.HashStringNode stringHashNode) { + return stringHashNode.execute(value); } @Specialization - protected int hash(RubyDynamicObject self) { + protected long hashSymbol(RubySymbol value, + @Cached SymbolNodes.HashSymbolNode symbolHashNode) { + return symbolHashNode.execute(value); + } + + @Fallback + protected int hashOtherUsingIdentity(Object self) { return System.identityHashCode(self); } }