SharedObjects.isShared() does not need the context

eregon · eregon · commit 7c411d9106cc · 2020-09-28T17:40:16.000+02:00
* If an object has a shared Shape then the option must be enabled.
diff --git a/src/main/java/org/truffleruby/core/array/ArrayOperations.java b/src/main/java/org/truffleruby/core/array/ArrayOperations.java
@@ -11,7 +11,6 @@
 
 import java.lang.reflect.Array;
 
-import org.truffleruby.RubyContext;
 import org.truffleruby.core.array.library.ArrayStoreLibrary;
 import org.truffleruby.core.array.library.DelegatedArrayStorage;
 import org.truffleruby.core.array.library.NativeArrayStorage;
@@ -35,16 +34,14 @@ public static boolean verifyStore(RubyArray array) {
                 backingStore instanceof int[] || backingStore instanceof long[] || backingStore instanceof double[] ||
                 backingStore.getClass() == Object[].class : backingStore;
 
-        final RubyContext context = array.getLogicalClass().fields.getContext();
-        if (SharedObjects.isShared(context, array)) {
+        if (SharedObjects.isShared(array)) {
             final Object store = array.store;
 
             if (store.getClass() == Object[].class) {
                 final Object[] objectArray = (Object[]) store;
 
                 for (Object element : objectArray) {
                     assert SharedObjects.assertPropagateSharing(
-                            context,
                             array,
                             element) : "unshared element in shared Array: " + element;
                 }
@@ -56,7 +53,6 @@ public static boolean verifyStore(RubyArray array) {
                 for (int i = delegated.offset; i < delegated.offset + delegated.length; i++) {
                     final Object element = objectArray[i];
                     assert SharedObjects.assertPropagateSharing(
-                            context,
                             array,
                             element) : "unshared element in shared copy-on-write Array: " + element;
                 }
diff --git a/src/main/java/org/truffleruby/core/hash/HashOperations.java b/src/main/java/org/truffleruby/core/hash/HashOperations.java
@@ -59,11 +59,9 @@ public static boolean verifyStore(RubyContext context, RubyHash hash) {
 
                 while (entry != null) {
                     assert SharedObjects.assertPropagateSharing(
-                            context,
                             hash,
                             entry.getKey()) : "unshared key in shared Hash: " + entry.getKey();
                     assert SharedObjects.assertPropagateSharing(
-                            context,
                             hash,
                             entry.getValue()) : "unshared value in shared Hash: " + entry.getValue();
 
@@ -119,8 +117,8 @@ public static boolean verifyStore(RubyContext context, RubyHash hash) {
                 final Object key = PackedArrayStrategy.getKey(packedStore, n);
                 final Object value = PackedArrayStrategy.getValue(packedStore, n);
 
-                assert SharedObjects.assertPropagateSharing(context, hash, key) : "unshared key in shared Hash: " + key;
-                assert SharedObjects.assertPropagateSharing(context, hash, value) : "unshared value in shared Hash: " +
+                assert SharedObjects.assertPropagateSharing(hash, key) : "unshared key in shared Hash: " + key;
+                assert SharedObjects.assertPropagateSharing(hash, value) : "unshared value in shared Hash: " +
                         value;
             }
 
diff --git a/src/main/java/org/truffleruby/core/kernel/KernelNodes.java b/src/main/java/org/truffleruby/core/kernel/KernelNodes.java
@@ -1102,7 +1102,7 @@ protected Object removeInstanceVariable(RubyDynamicObject object, String name) {
             final String ivar = SymbolTable.checkInstanceVariableName(getContext(), name, object, this);
             final Object value = DynamicObjectLibrary.getUncached().getOrDefault(object, ivar, nil);
 
-            if (SharedObjects.isShared(getContext(), object)) {
+            if (SharedObjects.isShared(object)) {
                 synchronized (object) {
                     removeField(object, name);
                 }
diff --git a/src/main/java/org/truffleruby/core/module/ModuleFields.java b/src/main/java/org/truffleruby/core/module/ModuleFields.java
@@ -405,7 +405,7 @@ public void addMethod(RubyContext context, Node currentNode, InternalMethod meth
 
         checkFrozen(context, currentNode);
 
-        if (SharedObjects.isShared(context, rubyModule)) {
+        if (SharedObjects.isShared(rubyModule)) {
             Set<Object> adjacent = ObjectGraph.newObjectSet();
             ObjectGraph.addProperty(adjacent, method);
             for (Object object : adjacent) {
diff --git a/src/main/java/org/truffleruby/debug/TruffleDebugNodes.java b/src/main/java/org/truffleruby/debug/TruffleDebugNodes.java
@@ -363,13 +363,13 @@ public abstract static class IsSharedNode extends CoreMethodArrayArgumentsNode {
                 limit = "getCacheLimit()")
         protected boolean isSharedCached(RubyDynamicObject object,
                 @Cached("object.getShape()") Shape cachedShape,
-                @Cached("isShared(getContext(), cachedShape)") boolean shared) {
+                @Cached("cachedShape.isShared()") boolean shared) {
             return shared;
         }
 
         @Specialization(replaces = "isSharedCached")
         protected boolean isShared(RubyDynamicObject object) {
-            return SharedObjects.isShared(getContext(), object);
+            return SharedObjects.isShared(object);
         }
 
         @Specialization
diff --git a/src/main/java/org/truffleruby/language/objects/ShapeCachingGuards.java b/src/main/java/org/truffleruby/language/objects/ShapeCachingGuards.java
@@ -10,7 +10,6 @@
 
 package org.truffleruby.language.objects;
 
-import org.truffleruby.RubyLanguage;
 import org.truffleruby.language.RubyDynamicObject;
 import org.truffleruby.language.objects.shared.SharedObjects;
 
@@ -24,7 +23,7 @@ public static boolean updateShape(RubyDynamicObject object) {
         // TODO (eregon, 14 July 2020): review callers, once they use the library they should not need to update the Shape manually anymore
         boolean updated = DynamicObjectLibrary.getUncached().updateShape(object);
         if (updated) {
-            assert !SharedObjects.isShared(RubyLanguage.getCurrentContext(), object);
+            assert !SharedObjects.isShared(object);
         }
         return updated;
     }
diff --git a/src/main/java/org/truffleruby/language/objects/shared/IsSharedNode.java b/src/main/java/org/truffleruby/language/objects/shared/IsSharedNode.java
@@ -43,7 +43,7 @@ protected boolean isShareCached(RubyDynamicObject object,
             @Cached("object.getShape()") Shape cachedShape,
             @CachedContext(RubyLanguage.class) TruffleLanguage.ContextReference<RubyContext> contextReference,
             @Cached("contextReference.get()") RubyContext cachedContext,
-            @Cached("isShared(cachedContext, cachedShape)") boolean shared) {
+            @Cached("cachedShape.isShared()") boolean shared) {
         return shared;
     }
 
@@ -55,11 +55,7 @@ protected boolean updateShapeAndIsShared(RubyDynamicObject object) {
     @Specialization(replaces = { "isShareCached", "updateShapeAndIsShared" })
     protected boolean isSharedUncached(RubyDynamicObject object,
             @CachedContext(RubyLanguage.class) RubyContext context) {
-        return SharedObjects.isShared(context, object);
-    }
-
-    protected boolean isShared(RubyContext context, Shape shape) {
-        return SharedObjects.isShared(context, shape);
+        return context.getOptions().SHARED_OBJECTS_ENABLED && SharedObjects.isShared(object);
     }
 
 }
diff --git a/src/main/java/org/truffleruby/language/objects/shared/ShareObjectNode.java b/src/main/java/org/truffleruby/language/objects/shared/ShareObjectNode.java
@@ -59,15 +59,15 @@ protected void shareCached(RubyDynamicObject object,
         assert object.getShape() == sharedShape;
 
         // Share the logical class
-        if (!SharedObjects.isShared(getContext(), object.getLogicalClass().getShape())) {
+        if (!object.getLogicalClass().getShape().isShared()) {
             // The logical class is fixed for a given Shape and only needs to be shared once
             CompilerDirectives.transferToInterpreterAndInvalidate();
             SharedObjects.writeBarrier(getContext(), object.getLogicalClass());
         }
 
         // Share the metaclass. Note that the metaclass might refer to `object` via `attached`,
         // so it is important to share the object first.
-        if (!SharedObjects.isShared(getContext(), object.getMetaClass().getShape())) {
+        if (!object.getMetaClass().getShape().isShared()) {
             // The metaclass is fixed for a given Shape and only needs to be shared once
             CompilerDirectives.transferToInterpreterAndInvalidate();
             SharedObjects.writeBarrier(getContext(), object.getMetaClass());
@@ -84,7 +84,7 @@ protected void shareCached(RubyDynamicObject object,
 
     private boolean allFieldsAreShared(RubyDynamicObject object) {
         for (Object value : ObjectGraph.getAdjacentObjects(object)) {
-            assert SharedObjects.isShared(getContext(), value) : "unshared field in shared object: " + value;
+            assert SharedObjects.isShared(value) : "unshared field in shared object: " + value;
         }
 
         return true;
@@ -126,7 +126,7 @@ protected ReadAndShareFieldNode[] createReadAndShareFieldNodes(List<Property> pr
     }
 
     protected Shape createSharedShape(Shape cachedShape) {
-        if (SharedObjects.isShared(getContext(), cachedShape)) {
+        if (cachedShape.isShared()) {
             throw new UnsupportedOperationException(
                     "Thread-safety bug: the object is already shared. This means another thread marked the object as shared concurrently.");
         } else {
diff --git a/src/main/java/org/truffleruby/language/objects/shared/SharedObjects.java b/src/main/java/org/truffleruby/language/objects/shared/SharedObjects.java
@@ -24,7 +24,6 @@
 
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.object.DynamicObjectLibrary;
-import com.oracle.truffle.api.object.Shape;
 import com.oracle.truffle.api.source.SourceSection;
 
 public class SharedObjects {
@@ -112,38 +111,39 @@ private static void shareObject(RubyContext context, RubyDynamicObject value) {
         shareObjects(context, stack);
     }
 
-    public static boolean isShared(RubyContext context, Object object) {
+    /** Callers of this should be careful, this method will return true for RubySymbol even if the
+     * SHARED_OBJECTS_ENABLED option is false. */
+    public static boolean isShared(Object object) {
         return object instanceof RubySymbol ||
-                (object instanceof RubyDynamicObject && isShared(context, ((RubyDynamicObject) object).getShape()));
+                (object instanceof RubyDynamicObject && isShared((RubyDynamicObject) object));
     }
 
-    public static boolean isShared(RubyContext context, Shape shape) {
-        return context.getOptions().SHARED_OBJECTS_ENABLED && shape.isShared();
+    public static boolean isShared(RubyDynamicObject object) {
+        return object.getShape().isShared();
     }
 
-    public static boolean assertPropagateSharing(RubyContext context, RubyDynamicObject source, Object value) {
-        if (isShared(context, source) && value instanceof RubyDynamicObject) {
-            return isShared(context, value);
+    public static boolean assertPropagateSharing(RubyDynamicObject source, Object value) {
+        if (isShared(source) && value instanceof RubyDynamicObject) {
+            return isShared(value);
         } else {
             return true;
         }
     }
 
     public static void writeBarrier(RubyContext context, Object value) {
-        if (context.getOptions().SHARED_OBJECTS_ENABLED && value instanceof RubyDynamicObject &&
-                !isShared(context, value)) {
+        if (context.getOptions().SHARED_OBJECTS_ENABLED && value instanceof RubyDynamicObject && !isShared(value)) {
             shareObject(context, (RubyDynamicObject) value);
         }
     }
 
     public static void propagate(RubyContext context, RubyDynamicObject source, Object value) {
-        if (isShared(context, source)) {
+        if (isShared(source)) {
             writeBarrier(context, value);
         }
     }
 
     private static boolean share(RubyContext context, RubyDynamicObject object) {
-        if (isShared(context, object)) {
+        if (isShared(object)) {
             return false;
         }
 
diff --git a/src/main/java/org/truffleruby/language/objects/shared/WriteBarrierNode.java b/src/main/java/org/truffleruby/language/objects/shared/WriteBarrierNode.java
@@ -51,7 +51,7 @@ protected void writeBarrierCached(RubyDynamicObject value,
             @Cached("value.getShape()") Shape cachedShape,
             @CachedContext(RubyLanguage.class) ContextReference<RubyContext> contextReference,
             @Cached("contextReference.get()") RubyContext cachedContext,
-            @Cached("isShared(cachedContext, cachedShape)") boolean alreadyShared,
+            @Cached("cachedShape.isShared()") boolean alreadyShared,
             @Cached("createShareObjectNode(alreadyShared)") ShareObjectNode shareObjectNode) {
         if (!alreadyShared) {
             shareObjectNode.executeShare(value);
@@ -73,10 +73,6 @@ protected void writeBarrierUncached(RubyDynamicObject value,
     protected void noWriteBarrier(Object value) {
     }
 
-    protected boolean isShared(RubyContext context, Shape shape) {
-        return SharedObjects.isShared(context, shape);
-    }
-
     protected ShareObjectNode createShareObjectNode(boolean alreadyShared) {
         if (!alreadyShared) {
             return ShareObjectNodeGen.create(getDepth() + 1);

Original file line number	Diff line number	Diff line change
`@@ -1102,7 +1102,7 @@ protected Object removeInstanceVariable(RubyDynamicObject object, String name) {`
`1102`	`1102`	`final String ivar = SymbolTable.checkInstanceVariableName(getContext(), name, object, this);`
`1103`	`1103`	`final Object value = DynamicObjectLibrary.getUncached().getOrDefault(object, ivar, nil);`
`1104`	`1104`
`1105`		`- if (SharedObjects.isShared(getContext(), object)) {`
	`1105`	`+ if (SharedObjects.isShared(object)) {`
`1106`	`1106`	`synchronized (object) {`
`1107`	`1107`	`removeField(object, name);`
`1108`	`1108`	`}`
Original file line number	Diff line number	Diff line change
`@@ -363,13 +363,13 @@ public abstract static class IsSharedNode extends CoreMethodArrayArgumentsNode {`
`363`	`363`	`limit = "getCacheLimit()")`
`364`	`364`	`protected boolean isSharedCached(RubyDynamicObject object,`
`365`	`365`	`@Cached("object.getShape()") Shape cachedShape,`
`366`		`- @Cached("isShared(getContext(), cachedShape)") boolean shared) {`
	`366`	`+ @Cached("cachedShape.isShared()") boolean shared) {`
`367`	`367`	`return shared;`
`368`	`368`	`}`
`369`	`369`
`370`	`370`	`@Specialization(replaces = "isSharedCached")`
`371`	`371`	`protected boolean isShared(RubyDynamicObject object) {`
`372`		`- return SharedObjects.isShared(getContext(), object);`
	`372`	`+ return SharedObjects.isShared(object);`
`373`	`373`	`}`
`374`	`374`
`375`	`375`	`@Specialization`
Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,6 @@`
`10`	`10`
`11`	`11`	`package org.truffleruby.language.objects;`
`12`	`12`
`13`		`-import org.truffleruby.RubyLanguage;`
`14`	`13`	`import org.truffleruby.language.RubyDynamicObject;`
`15`	`14`	`import org.truffleruby.language.objects.shared.SharedObjects;`
`16`	`15`
`@@ -24,7 +23,7 @@ public static boolean updateShape(RubyDynamicObject object) {`
`24`	`23`	`// TODO (eregon, 14 July 2020): review callers, once they use the library they should not need to update the Shape manually anymore`
`25`	`24`	`boolean updated = DynamicObjectLibrary.getUncached().updateShape(object);`
`26`	`25`	`if (updated) {`
`27`		`- assert !SharedObjects.isShared(RubyLanguage.getCurrentContext(), object);`
	`26`	`+ assert !SharedObjects.isShared(object);`
`28`	`27`	`}`
`29`	`28`	`return updated;`
`30`	`29`	`}`
Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ protected boolean isShareCached(RubyDynamicObject object,`
`43`	`43`	`@Cached("object.getShape()") Shape cachedShape,`
`44`	`44`	`@CachedContext(RubyLanguage.class) TruffleLanguage.ContextReference<RubyContext> contextReference,`
`45`	`45`	`@Cached("contextReference.get()") RubyContext cachedContext,`
`46`		`- @Cached("isShared(cachedContext, cachedShape)") boolean shared) {`
	`46`	`+ @Cached("cachedShape.isShared()") boolean shared) {`
`47`	`47`	`return shared;`
`48`	`48`	`}`
`49`	`49`
`@@ -55,11 +55,7 @@ protected boolean updateShapeAndIsShared(RubyDynamicObject object) {`
`55`	`55`	`@Specialization(replaces = { "isShareCached", "updateShapeAndIsShared" })`
`56`	`56`	`protected boolean isSharedUncached(RubyDynamicObject object,`
`57`	`57`	`@CachedContext(RubyLanguage.class) RubyContext context) {`
`58`		`- return SharedObjects.isShared(context, object);`
`59`		`- }`
`60`		`-`
`61`		`- protected boolean isShared(RubyContext context, Shape shape) {`
`62`		`- return SharedObjects.isShared(context, shape);`
	`58`	`+ return context.getOptions().SHARED_OBJECTS_ENABLED && SharedObjects.isShared(object);`
`63`	`59`	`}`
`64`	`60`
`65`	`61`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,6 @@`
`24`	`24`
`25`	`25`	`import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;`
`26`	`26`	`import com.oracle.truffle.api.object.DynamicObjectLibrary;`
`27`		`-import com.oracle.truffle.api.object.Shape;`
`28`	`27`	`import com.oracle.truffle.api.source.SourceSection;`
`29`	`28`
`30`	`29`	`public class SharedObjects {`
`@@ -112,38 +111,39 @@ private static void shareObject(RubyContext context, RubyDynamicObject value) {`
`112`	`111`	`shareObjects(context, stack);`
`113`	`112`	`}`
`114`	`113`
`115`		`- public static boolean isShared(RubyContext context, Object object) {`
	`114`	`+ /** Callers of this should be careful, this method will return true for RubySymbol even if the`
	`115`	`+ * SHARED_OBJECTS_ENABLED option is false. */`
	`116`	`+ public static boolean isShared(Object object) {`
`116`	`117`	`return object instanceof RubySymbol \|\|`
`117`		`- (object instanceof RubyDynamicObject && isShared(context, ((RubyDynamicObject) object).getShape()));`
	`118`	`+ (object instanceof RubyDynamicObject && isShared((RubyDynamicObject) object));`
`118`	`119`	`}`
`119`	`120`
`120`		`- public static boolean isShared(RubyContext context, Shape shape) {`
`121`		`- return context.getOptions().SHARED_OBJECTS_ENABLED && shape.isShared();`
	`121`	`+ public static boolean isShared(RubyDynamicObject object) {`
	`122`	`+ return object.getShape().isShared();`
`122`	`123`	`}`
`123`	`124`
`124`		`- public static boolean assertPropagateSharing(RubyContext context, RubyDynamicObject source, Object value) {`
`125`		`- if (isShared(context, source) && value instanceof RubyDynamicObject) {`
`126`		`- return isShared(context, value);`
	`125`	`+ public static boolean assertPropagateSharing(RubyDynamicObject source, Object value) {`
	`126`	`+ if (isShared(source) && value instanceof RubyDynamicObject) {`
	`127`	`+ return isShared(value);`
`127`	`128`	`} else {`
`128`	`129`	`return true;`
`129`	`130`	`}`
`130`	`131`	`}`
`131`	`132`
`132`	`133`	`public static void writeBarrier(RubyContext context, Object value) {`
`133`		`- if (context.getOptions().SHARED_OBJECTS_ENABLED && value instanceof RubyDynamicObject &&`
`134`		`- !isShared(context, value)) {`
	`134`	`+ if (context.getOptions().SHARED_OBJECTS_ENABLED && value instanceof RubyDynamicObject && !isShared(value)) {`
`135`	`135`	`shareObject(context, (RubyDynamicObject) value);`
`136`	`136`	`}`
`137`	`137`	`}`
`138`	`138`
`139`	`139`	`public static void propagate(RubyContext context, RubyDynamicObject source, Object value) {`
`140`		`- if (isShared(context, source)) {`
	`140`	`+ if (isShared(source)) {`
`141`	`141`	`writeBarrier(context, value);`
`142`	`142`	`}`
`143`	`143`	`}`
`144`	`144`
`145`	`145`	`private static boolean share(RubyContext context, RubyDynamicObject object) {`
`146`		`- if (isShared(context, object)) {`
	`146`	`+ if (isShared(object)) {`
`147`	`147`	`return false;`
`148`	`148`	`}`
`149`	`149`