Revert "Marking changes"

PullRequest: truffleruby/560

Author: aardvark179

lib/cext/include/ruby/ruby.h

@@ -498,11 +498,10 @@ MUST_INLINE int RB_FLOAT_TYPE_P(VALUE obj);
 
 bool RB_TYPE_P(VALUE value, int type);
 
-#define RB_GC_GUARD(v) \
-    (*__extension__ ({ \
-        volatile VALUE *rb_gc_guarded_ptr = rb_tr_gc_guard(&v);   \
-        rb_gc_guarded_ptr;                              \
-    }))
+#define HAVE_RB_GC_GUARDED_PTR_VAL 1
+#define rb_gc_guarded_ptr_val(ptr, val) (ptr)
+#define RB_GC_GUARD(v) (v)
+#define RB_GC_GUARD_PTR(ptr) (ptr)
 
 #ifdef __GNUC__
 #define RB_UNUSED_VAR(x) x __attribute__ ((unused))

lib/cext/include/truffleruby/truffleruby-pre.h

@@ -48,13 +48,6 @@ MUST_INLINE VALUE rb_tr_unwrap(VALUE object) {
   return polyglot_invoke(RUBY_CEXT, "rb_tr_unwrap", object);
 }
 
-// Needed for GC guarding
-
-MUST_INLINE VALUE *rb_tr_gc_guard(VALUE *ptr) {
-  polyglot_invoke(RUBY_CEXT, "rb_tr_gc_guard", *ptr);
-  return ptr;
-}
-
 #include <ruby/thread_native.h>
 
 // Helpers

lib/truffle/truffle/cext.rb

@@ -1451,7 +1451,7 @@ def data_finalizer(free, data_holder)
     # In a separate method to avoid capturing the object
     raise unless free.respond_to?(:call)
     proc {
-      execute_with_mutex(free, data_holder.data) unless data_holder.data.nil?
+      execute_with_mutex(free, data_holder.data)
     }
   end
 
@@ -1460,7 +1460,7 @@ def data_marker(mark, data_holder)
     raise unless mark.respond_to?(:call)
     proc { |obj|
       create_mark_list
-      execute_with_mutex(mark, data_holder.data) unless data_holder.data.nil?
+      execute_with_mutex(mark, data_holder.data)
       set_mark_list_on_object(obj)
     }
   end

src/main/java/org/truffleruby/RubyContext.java

@@ -108,9 +108,9 @@ public class RubyContext {
     private final CodeLoader codeLoader = new CodeLoader(this);
     private final FeatureLoader featureLoader = new FeatureLoader(this);
     private final TraceManager traceManager;
-    private final ReferenceProcessor referenceProcessor;
-    private final FinalizationService finalizationService;
-    private final MarkingService markingService;
+    private final ReferenceProcessor referenceProcessor = new ReferenceProcessor(this);
+    private final FinalizationService finalizationService = new FinalizationService(this, referenceProcessor);
+    private final MarkingService markingService = new MarkingService(this, referenceProcessor);
     private final ObjectSpaceManager objectSpaceManager = new ObjectSpaceManager(this);
     private final SharedObjects sharedObjects = new SharedObjects(this);
     private final AtExitManager atExitManager = new AtExitManager(this);
@@ -174,10 +174,6 @@ public RubyContext(RubyLanguage language, TruffleLanguage.Env env) {
 
         options = createOptions(env);
 
-        referenceProcessor = new ReferenceProcessor(this);
-        finalizationService = new FinalizationService(this, referenceProcessor);
-        markingService = new MarkingService(this, referenceProcessor);
-
         // We need to construct this at runtime
         random = createRandomInstance();
 

src/main/java/org/truffleruby/cext/CExtNodes.java

@@ -14,7 +14,6 @@
 import com.oracle.truffle.api.Truffle;
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.CreateCast;
-import com.oracle.truffle.api.dsl.Fallback;
 import com.oracle.truffle.api.dsl.NodeChild;
 import com.oracle.truffle.api.dsl.NodeChildren;
 import com.oracle.truffle.api.dsl.Specialization;
@@ -1317,29 +1316,24 @@ protected WrapNode createWrapNode() {
     public abstract static class UnwrapValueNode extends CoreMethodArrayArgumentsNode {
 
         @Specialization
-        public Object unwrap(TruffleObject value,
+        public Object unwrap(Object value,
                 @Cached("create()") BranchProfile exceptionProfile,
                 @Cached("createUnwrapNode()") UnwrapNode unwrapNode) {
             Object object = unwrapNode.execute(value);
             if (object == null) {
                 exceptionProfile.enter();
-                throw new RaiseException(getContext(), coreExceptions().runtimeError(exceptionMessage(value), this));
+                throw new RaiseException(getContext(), coreExceptions().runtimeError("native handle not found", this));
             } else {
                 return object;
             }
         }
 
-        @TruffleBoundary
-        private String exceptionMessage(Object value) {
-            return String.format("native handle not found (%s)", value);
-        }
-
         protected UnwrapNode createUnwrapNode() {
             return UnwrapNodeGen.create();
         }
     }
 
-    private static final ThreadLocal<ArrayList<Object>> markList = new ThreadLocal<>();
+    private static ThreadLocal<ArrayList<Object>> markList = new ThreadLocal<>();
 
     @CoreMethod(names = "create_mark_list", onSingleton = true, required = 0)
     public abstract static class NewMarkerList extends CoreMethodArrayArgumentsNode {
@@ -1360,14 +1354,14 @@ protected void setThreadLocal() {
     public abstract static class AddToMarkList extends CoreMethodArrayArgumentsNode {
 
         @Specialization
-        public DynamicObject addToMarkList(VirtualFrame frmae, TruffleObject markedObject,
+        public DynamicObject addToMarkList(VirtualFrame frmae, Object markedObject,
                 @Cached("create()") BranchProfile exceptionProfile,
                 @Cached("create()") BranchProfile noExceptionProfile,
-                @Cached("create()") UnwrapNode.ToWrapperNode toWrapperNode) {
-            ValueWrapper wrappedValue = toWrapperNode.execute(markedObject);
-            if (wrappedValue != null) {
+                @Cached("createUnwrapNode()") UnwrapNode unwrapNode) {
+            Object unwrappedValue = unwrapNode.execute(markedObject);
+            if (unwrappedValue != null) {
                 noExceptionProfile.enter();
-                getList().add(wrappedValue);
+                getList().add(unwrappedValue);
             }
             // We do nothing here if the handle cannot be resolved. If we are marking an object
             // which is only reachable via weak refs then the handles of objects it is iteself
@@ -1385,33 +1379,6 @@ protected UnwrapNode createUnwrapNode() {
         }
     }
 
-    @CoreMethod(names = "rb_tr_gc_guard", onSingleton = true, required = 1)
-    public abstract static class GCGuardNode extends CoreMethodArrayArgumentsNode {
-
-        @Specialization
-        public DynamicObject addToMarkList(VirtualFrame frmae, TruffleObject guardedObject,
-                @Cached("create()") BranchProfile noExceptionProfile,
-                @Cached("create()") UnwrapNode.ToWrapperNode toWrapperNode) {
-            ValueWrapper wrappedValue = toWrapperNode.execute(guardedObject);
-            if (wrappedValue != null) {
-                noExceptionProfile.enter();
-                getContext().getMarkingService().keepObject(guardedObject);
-            }
-            return nil();
-        }
-
-        @Fallback
-        public DynamicObject addToMarkList(VirtualFrame frmae, Object guardedObject) {
-            // Do nothing for unexpected objects, no matter how unexpected. This can occur inside
-            // macros that guard a variable which may not have been initialized.
-            return nil();
-        }
-
-        protected UnwrapNode createUnwrapNode() {
-            return UnwrapNodeGen.create();
-        }
-    }
-
     @CoreMethod(names = "set_mark_list_on_object", onSingleton = true, required = 1)
     public abstract static class SetMarkList extends CoreMethodArrayArgumentsNode {
 

src/main/java/org/truffleruby/cext/UnwrapNode.java

@@ -9,20 +9,17 @@
  */
 package org.truffleruby.cext;
 
-import static org.truffleruby.cext.ValueWrapperManager.TRUE_HANDLE;
-import static org.truffleruby.cext.ValueWrapperManager.UNDEF_HANDLE;
-import static org.truffleruby.cext.ValueWrapperManager.NIL_HANDLE;
+import static org.truffleruby.cext.ValueWrapperManager.TAG_MASK;
+import static org.truffleruby.cext.ValueWrapperManager.LONG_TAG;
+import static org.truffleruby.cext.ValueWrapperManager.OBJECT_TAG;
 import static org.truffleruby.cext.ValueWrapperManager.FALSE_HANDLE;
 
-import org.truffleruby.cext.UnwrapNodeGen.NativeToWrapperNodeGen;
-import org.truffleruby.cext.UnwrapNodeGen.ToWrapperNodeGen;
 import org.truffleruby.cext.UnwrapNodeGen.UnwrapNativeNodeGen;
 import org.truffleruby.language.NotProvided;
 import org.truffleruby.language.RubyBaseNode;
 import org.truffleruby.language.control.RaiseException;
 
 import com.oracle.truffle.api.dsl.Cached;
-import com.oracle.truffle.api.dsl.Fallback;
 import com.oracle.truffle.api.dsl.ImportStatic;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.interop.ForeignAccess;
@@ -33,7 +30,7 @@
 import com.oracle.truffle.api.object.DynamicObject;
 import com.oracle.truffle.api.profiles.BranchProfile;
 
-@ImportStatic({ Message.class, ValueWrapperManager.class })
+@ImportStatic(Message.class)
 public abstract class UnwrapNode extends RubyBaseNode {
 
     @ImportStatic(ValueWrapperManager.class)
@@ -71,103 +68,20 @@ public Object unwrapTaggedObject(long handle) {
             return getContext().getValueWrapperManager().getFromHandleMap(handle);
         }
 
-        @Fallback
-        public ValueWrapper unWrapUnexpectedHandle(long handle) {
-            // Avoid throwing a specialization exception when given an uninitialized or corrupt
-            // handle.
-            return null;
+        public boolean isTaggedLong(long handle) {
+            return (handle & LONG_TAG) == LONG_TAG;
         }
 
-        public static UnwrapNativeNode create() {
-            return UnwrapNativeNodeGen.create();
-        }
-    }
-
-    @ImportStatic(ValueWrapperManager.class)
-    public static abstract class NativeToWrapperNode extends RubyBaseNode {
-
-        public abstract ValueWrapper execute(long handle);
-
-        @Specialization(guards = "handle == FALSE_HANDLE")
-        public ValueWrapper unwrapFalse(long handle) {
-            return new ValueWrapper(false, FALSE_HANDLE);
-        }
-
-        @Specialization(guards = "handle == TRUE_HANDLE")
-        public ValueWrapper unwrapTrue(long handle) {
-            return new ValueWrapper(true, TRUE_HANDLE);
-        }
-
-        @Specialization(guards = "handle == UNDEF_HANDLE")
-        public ValueWrapper unwrapUndef(long handle) {
-            return new ValueWrapper(NotProvided.INSTANCE, UNDEF_HANDLE);
-        }
-
-        @Specialization(guards = "handle == NIL_HANDLE")
-        public ValueWrapper unwrapNil(long handle) {
-            return new ValueWrapper(nil(), NIL_HANDLE);
-        }
-
-        @Specialization(guards = "isTaggedLong(handle)")
-        public ValueWrapper unwrapTaggedLong(long handle) {
-            return new ValueWrapper(handle >> 1, handle);
-        }
-
-        @Specialization(guards = "isTaggedObject(handle)")
-        public ValueWrapper unwrapTaggedObject(long handle) {
-            return getContext().getValueWrapperManager().getWrapperFromHandleMap(handle);
-        }
-
-        @Fallback
-        public ValueWrapper unWrapUnexpectedHandle(long handle) {
-            // Avoid throwing a specialization exception when given an uninitialized or corrupt
-            // handle.
-            return null;
-        }
-
-        public static NativeToWrapperNode create() {
-            return NativeToWrapperNodeGen.create();
-        }
-    }
-
-    @ImportStatic({ Message.class, ValueWrapperManager.class })
-    public static abstract class ToWrapperNode extends RubyBaseNode {
-
-        public abstract ValueWrapper execute(TruffleObject value);
-
-        @Specialization
-        public ValueWrapper wrappedValueWrapper(ValueWrapper value) {
-            return value;
-        }
-
-        @Specialization(guards = "!isWrapper(value)")
-        public ValueWrapper unwrapTypeCastObject(TruffleObject value,
-                @Cached("IS_POINTER.createNode()") Node isPointerNode,
-                @Cached("AS_POINTER.createNode()") Node asPointerNode,
-                @Cached("create()") NativeToWrapperNode nativeToWrapperNode,
-                @Cached("create()") BranchProfile unsupportedProfile,
-                @Cached("create()") BranchProfile nonPointerProfile) {
-            if (ForeignAccess.sendIsPointer(isPointerNode, value)) {
-                long handle = 0;
-                try {
-                    handle = ForeignAccess.sendAsPointer(asPointerNode, value);
-                } catch (UnsupportedMessageException e) {
-                    unsupportedProfile.enter();
-                    throw new RaiseException(getContext(), coreExceptions().argumentError(e.getMessage(), this, e));
-                }
-                return nativeToWrapperNode.execute(handle);
-            } else {
-                nonPointerProfile.enter();
-                return null;
-            }
+        public boolean isTaggedObject(long handle) {
+            return handle != FALSE_HANDLE && (handle & TAG_MASK) == OBJECT_TAG;
         }
 
-        public static ToWrapperNode create() {
-            return ToWrapperNodeGen.create();
+        public static UnwrapNativeNode create() {
+            return UnwrapNativeNodeGen.create();
         }
     }
 
-    public abstract Object execute(TruffleObject value);
+    public abstract Object execute(Object value);
 
     @Specialization
     public Object unwrapValue(ValueWrapper value) {
@@ -195,4 +109,8 @@ public Object unwrapTypeCastObject(TruffleObject value,
             throw new RaiseException(getContext(), coreExceptions().argumentError("Not a handle or a pointer", this));
         }
     }
+
+    public static boolean isWrapper(TruffleObject value) {
+        return value instanceof ValueWrapper;
+    }
 }

src/main/java/org/truffleruby/cext/ValueWrapperManager.java

@@ -14,9 +14,7 @@
 import org.truffleruby.RubyContext;
 import org.truffleruby.collections.LongHashMap;
 import org.truffleruby.extra.ffi.Pointer;
-
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
-import com.oracle.truffle.api.interop.TruffleObject;
 
 public class ValueWrapperManager {
 
@@ -51,7 +49,8 @@ public ValueWrapperManager(RubyContext context) {
      * We keep a map of long wrappers that have been generated because various C extensions assume
      * that any given fixnum will translate to a given VALUE.
      */
-    public ValueWrapper longWrapper(long value) {
+    @TruffleBoundary
+    public synchronized ValueWrapper longWrapper(long value) {
         return new ValueWrapper(value, UNSET_HANDLE);
     }
 
@@ -77,19 +76,6 @@ public synchronized Object getFromHandleMap(long handle) {
         return wrapper.getObject();
     }
 
-    @TruffleBoundary
-    public synchronized ValueWrapper getWrapperFromHandleMap(long handle) {
-        WeakReference<ValueWrapper> ref = handleMap.get(handle);
-        ValueWrapper wrapper;
-        if (ref == null) {
-            return null;
-        }
-        if ((wrapper = ref.get()) == null) {
-            return null;
-        }
-        return wrapper;
-    }
-
     @TruffleBoundary
     public synchronized void removeFromHandleMap(long handle) {
         handleMap.remove(handle);
@@ -104,6 +90,7 @@ public synchronized long createNativeHandle(ValueWrapper wrapper) {
         }
         wrapper.setHandle(handleAddress);
         addToHandleMap(handleAddress, wrapper);
+        context.getMarkingService().keepObject(wrapper);
         addFinalizer(wrapper, handlePointer);
         return handleAddress;
     }
@@ -121,16 +108,4 @@ private Runnable createFinalizer(Pointer handle) {
         };
 
     }
-
-    public static boolean isTaggedLong(long handle) {
-        return (handle & LONG_TAG) == LONG_TAG;
-    }
-
-    public static boolean isTaggedObject(long handle) {
-        return handle != FALSE_HANDLE && (handle & TAG_MASK) == OBJECT_TAG;
-    }
-
-    public static boolean isWrapper(TruffleObject value) {
-        return value instanceof ValueWrapper;
-    }
 }