[GR-64208] Reduce virtual calls to Object and reference visitors.

PullRequest: graal/20648

Author: christianhaeubl

substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/AlignedHeapChunk.java

@@ -144,14 +144,14 @@ public static UnsignedWord getObjectOffset(AlignedHeader that, Pointer objectPoi
         return objectPointer.subtract(objectsStart);
     }
 
-    static boolean walkObjects(AlignedHeader that, ObjectVisitor visitor) {
-        return HeapChunk.walkObjectsFrom(that, getObjectsStart(that), visitor);
+    static void walkObjects(AlignedHeader that, ObjectVisitor visitor) {
+        HeapChunk.walkObjectsFrom(that, getObjectsStart(that), visitor);
     }
 
     @AlwaysInline("GC performance")
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-    static boolean walkObjectsFromInline(AlignedHeader that, Pointer start, ObjectVisitor visitor) {
-        return HeapChunk.walkObjectsFromInline(that, start, visitor);
+    static void walkObjectsFromInline(AlignedHeader that, Pointer start, GreyToBlackObjectVisitor visitor) {
+        HeapChunk.walkObjectsFromInline(that, start, visitor);
     }
 
     @Fold

substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/AuxiliaryImageHeap.java

@@ -24,14 +24,15 @@
  */
 package com.oracle.svm.core.genscavenge;
 
-import jdk.graal.compiler.api.replacements.Fold;
 import org.graalvm.nativeimage.ImageSingletons;
 import org.graalvm.word.Pointer;
 
 import com.oracle.svm.core.MemoryWalker;
 import com.oracle.svm.core.Uninterruptible;
 import com.oracle.svm.core.heap.ObjectVisitor;
 
+import jdk.graal.compiler.api.replacements.Fold;
+
 public interface AuxiliaryImageHeap {
     @Fold
     static boolean isPresent() {
@@ -46,10 +47,10 @@ static AuxiliaryImageHeap singleton() {
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     boolean containsObject(Pointer address);
 
-    boolean walkObjects(ObjectVisitor visitor);
+    void walkObjects(ObjectVisitor visitor);
 
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-    boolean walkRegions(MemoryWalker.ImageHeapRegionVisitor visitor);
+    void walkRegions(MemoryWalker.ImageHeapRegionVisitor visitor);
 
     ImageHeapInfo getImageHeapInfo();
 }

substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/CompactingOldGeneration.java

@@ -166,7 +166,7 @@ boolean scanGreyObjects(boolean incrementalGc) {
             }
             GreyToBlackObjectVisitor visitor = GCImpl.getGCImpl().getGreyToBlackObjectVisitor();
             do {
-                visitor.visitObjectInline(markStack.pop());
+                visitor.visitObject(markStack.pop());
             } while (!markStack.isEmpty());
         }
         return true;
@@ -362,7 +362,7 @@ private void fixupUnalignedChunkReferences(ChunkReleaser chunkReleaser) {
         while (uChunk.isNonNull()) {
             UnalignedHeapChunk.UnalignedHeader next = HeapChunk.getNext(uChunk);
             Pointer objPointer = UnalignedHeapChunk.getObjectStart(uChunk);
-            Object obj = objPointer.toObject();
+            Object obj = objPointer.toObjectNonNull();
             if (ObjectHeaderImpl.isMarked(obj)) {
                 ObjectHeaderImpl.unsetMarkedAndKeepRememberedSetBit(obj);
                 RememberedSet.get().clearRememberedSet(uChunk);
@@ -447,8 +447,8 @@ boolean printLocationInfo(Log log, Pointer ptr) {
     }
 
     @Override
-    public boolean walkObjects(ObjectVisitor visitor) {
-        return space.walkObjects(visitor);
+    public void walkObjects(ObjectVisitor visitor) {
+        space.walkObjects(visitor);
     }
 
     @Override

substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/CopyingOldGeneration.java

@@ -65,8 +65,9 @@ void tearDown() {
     }
 
     @Override
-    public boolean walkObjects(ObjectVisitor visitor) {
-        return getFromSpace().walkObjects(visitor) && getToSpace().walkObjects(visitor);
+    public void walkObjects(ObjectVisitor visitor) {
+        getFromSpace().walkObjects(visitor);
+        getToSpace().walkObjects(visitor);
     }
 
     /** Promote an Object to ToSpace if it is not already in ToSpace. */

substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/Generation.java

@@ -41,13 +41,7 @@ abstract class Generation {
         this.name = name;
     }
 
-    /**
-     * Walk the Objects in this Space, passing each to a Visitor.
-     *
-     * @param visitor An ObjectVisitor.
-     * @return True if all visits returned true, false otherwise.
-     */
-    public abstract boolean walkObjects(ObjectVisitor visitor);
+    public abstract void walkObjects(ObjectVisitor visitor);
 
     public String getName() {
         return name;

substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/GreyObjectsWalker.java

@@ -24,15 +24,15 @@
  */
 package com.oracle.svm.core.genscavenge;
 
-import jdk.graal.compiler.word.Word;
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
 import org.graalvm.word.Pointer;
 
 import com.oracle.svm.core.AlwaysInline;
 import com.oracle.svm.core.NeverInline;
 import com.oracle.svm.core.Uninterruptible;
-import com.oracle.svm.core.util.VMError;
+
+import jdk.graal.compiler.word.Word;
 
 /**
  * Apply an ObjectVisitor to all the new Objects in a Space since a snapshot.
@@ -103,9 +103,7 @@ private void walkAlignedGreyObjects() {
             AlignedHeapChunk.AlignedHeader lastChunk;
             do {
                 lastChunk = aChunk;
-                if (!AlignedHeapChunk.walkObjectsFromInline(aChunk, aStart, visitor)) {
-                    throw VMError.shouldNotReachHereAtRuntime();
-                }
+                AlignedHeapChunk.walkObjectsFromInline(aChunk, aStart, visitor);
                 aChunk = HeapChunk.getNext(aChunk);
                 aStart = (aChunk.isNonNull() ? AlignedHeapChunk.getObjectsStart(aChunk) : Word.nullPointer());
             } while (aChunk.isNonNull());
@@ -131,9 +129,7 @@ private void walkUnalignedGreyObjects() {
             UnalignedHeapChunk.UnalignedHeader lastChunk;
             do {
                 lastChunk = uChunk;
-                if (!UnalignedHeapChunk.walkObjectsInline(uChunk, visitor)) {
-                    throw VMError.shouldNotReachHereAtRuntime();
-                }
+                UnalignedHeapChunk.walkObjectsInline(uChunk, visitor);
                 uChunk = HeapChunk.getNext(uChunk);
             } while (uChunk.isNonNull());
 

substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/GreyToBlackObjRefVisitor.java

@@ -33,7 +33,6 @@
 import com.oracle.svm.core.genscavenge.remset.RememberedSet;
 import com.oracle.svm.core.heap.ObjectReferenceVisitor;
 import com.oracle.svm.core.heap.ReferenceAccess;
-import com.oracle.svm.core.hub.LayoutEncoding;
 import com.oracle.svm.core.log.Log;
 
 import jdk.graal.compiler.word.Word;
@@ -60,31 +59,32 @@ final class GreyToBlackObjRefVisitor implements ObjectReferenceVisitor {
     }
 
     @Override
+    @AlwaysInline("GC performance")
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-    public boolean visitObjectReference(Pointer objRef, boolean compressed, Object holderObject) {
-        return visitObjectReferenceInline(objRef, 0, compressed, holderObject);
+    public void visitObjectReferences(Pointer firstObjRef, boolean compressed, int referenceSize, Object holderObject, int count) {
+        Pointer pos = firstObjRef;
+        Pointer end = firstObjRef.add(Word.unsigned(count).multiply(referenceSize));
+        while (pos.belowThan(end)) {
+            visitObjectReference(pos, compressed, holderObject);
+            pos = pos.add(referenceSize);
+        }
     }
 
-    @Override
     @AlwaysInline("GC performance")
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-    public boolean visitObjectReferenceInline(Pointer objRef, int innerOffset, boolean compressed, Object holderObject) {
-        assert innerOffset >= 0;
+    private void visitObjectReference(Pointer objRef, boolean compressed, Object holderObject) {
         assert !objRef.isNull();
         counters.noteObjRef();
 
-        Pointer offsetP = ReferenceAccess.singleton().readObjectAsUntrackedPointer(objRef, compressed);
-        assert offsetP.isNonNull() || innerOffset == 0;
-
-        Pointer p = offsetP.subtract(innerOffset);
+        Pointer p = ReferenceAccess.singleton().readObjectAsUntrackedPointer(objRef, compressed);
         if (p.isNull()) {
             counters.noteNullReferent();
-            return true;
+            return;
         }
 
         if (HeapImpl.getHeapImpl().isInImageHeap(p)) {
             counters.noteNonHeapReferent();
-            return true;
+            return;
         }
 
         // This is the most expensive check as it accesses the heap fairly randomly, which results
@@ -97,26 +97,23 @@ public boolean visitObjectReferenceInline(Pointer objRef, int innerOffset, boole
                 counters.noteForwardedReferent();
                 // Update the reference to point to the forwarded Object.
                 Object obj = ohi.getForwardedObject(p, header);
-                Object offsetObj = (innerOffset == 0) ? obj : Word.objectToUntrackedPointer(obj).add(innerOffset).toObject();
-                ReferenceAccess.singleton().writeObjectAt(objRef, offsetObj, compressed);
+                ReferenceAccess.singleton().writeObjectAt(objRef, obj, compressed);
                 RememberedSet.get().dirtyCardIfNecessary(holderObject, obj);
-                return true;
+                return;
             }
 
-            Object obj = p.toObject();
+            Object obj = p.toObjectNonNull();
             if (SerialGCOptions.useCompactingOldGen() && ObjectHeaderImpl.isMarkedHeader(header)) {
                 RememberedSet.get().dirtyCardIfNecessary(holderObject, obj);
-                return true;
+                return;
             }
 
             // Promote the Object if necessary, making it at least grey, and ...
-            assert innerOffset < LayoutEncoding.getSizeFromObjectInGC(obj).rawValue();
             Object copy = GCImpl.getGCImpl().promoteObject(obj, header);
             if (copy != obj) {
                 // ... update the reference to point to the copy, making the reference black.
                 counters.noteCopiedReferent();
-                Object offsetCopy = (innerOffset == 0) ? copy : Word.objectToUntrackedPointer(copy).add(innerOffset).toObject();
-                ReferenceAccess.singleton().writeObjectAt(objRef, offsetCopy, compressed);
+                ReferenceAccess.singleton().writeObjectAt(objRef, copy, compressed);
             } else {
                 counters.noteUnmodifiedReference();
             }
@@ -125,7 +122,6 @@ public boolean visitObjectReferenceInline(Pointer objRef, int innerOffset, boole
             // might have to dirty the card.
             RememberedSet.get().dirtyCardIfNecessary(holderObject, copy);
         }
-        return true;
     }
 
     public Counters openCounters() {

substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/GreyToBlackObjectVisitor.java

@@ -28,11 +28,9 @@
 import org.graalvm.nativeimage.Platforms;
 
 import com.oracle.svm.core.AlwaysInline;
-import com.oracle.svm.core.NeverInline;
 import com.oracle.svm.core.Uninterruptible;
 import com.oracle.svm.core.heap.UninterruptibleObjectVisitor;
 import com.oracle.svm.core.hub.InteriorObjRefWalker;
-import com.oracle.svm.core.util.VMError;
 
 /**
  * Run an ObjectReferenceVisitor ({@link GreyToBlackObjRefVisitor}) over any interior object
@@ -48,19 +46,11 @@ public final class GreyToBlackObjectVisitor implements UninterruptibleObjectVisi
         this.objRefVisitor = greyToBlackObjRefVisitor;
     }
 
-    @Override
-    @NeverInline("Non-performance critical version")
-    @Uninterruptible(reason = "Visitor requires uninterruptible walk.", callerMustBe = true)
-    public boolean visitObject(Object o) {
-        throw VMError.shouldNotReachHere("For performance reasons, this should not be called.");
-    }
-
     @Override
     @AlwaysInline("GC performance")
     @Uninterruptible(reason = "Forced inlining (StoredContinuation objects must not move).", callerMustBe = true)
-    public boolean visitObjectInline(Object o) {
+    public void visitObject(Object o) {
         ReferenceObjectProcessing.discoverIfReference(o, objRefVisitor);
         InteriorObjRefWalker.walkObjectInline(o, objRefVisitor);
-        return true;
     }
 }

substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/HeapChunk.java

@@ -301,28 +301,25 @@ private static SignedWord offsetFromPointer(Header<?> that, PointerBase pointer)
 
     @NeverInline("Not performance critical")
     @Uninterruptible(reason = "Forced inlining (StoredContinuation objects must not move).")
-    public static boolean walkObjectsFrom(Header<?> that, Pointer start, ObjectVisitor visitor) {
-        return walkObjectsFromInline(that, start, visitor);
+    public static void walkObjectsFrom(Header<?> that, Pointer start, ObjectVisitor visitor) {
+        walkObjectsFromInline(that, start, visitor);
     }
 
     @AlwaysInline("GC performance")
     @Uninterruptible(reason = "Forced inlining (StoredContinuation objects must not move).", callerMustBe = true)
-    public static boolean walkObjectsFromInline(Header<?> that, Pointer start, ObjectVisitor visitor) {
+    public static void walkObjectsFromInline(Header<?> that, Pointer start, ObjectVisitor visitor) {
         Pointer p = start;
         while (p.belowThan(getTopPointer(that))) { // crucial: top can move, so always re-read
-            Object obj = p.toObject();
-            if (!callVisitor(visitor, obj)) {
-                return false;
-            }
+            Object obj = p.toObjectNonNull();
+            callVisitor(visitor, obj);
             p = p.add(LayoutEncoding.getSizeFromObjectInlineInGC(obj));
         }
-        return true;
     }
 
     @AlwaysInline("de-virtualize calls to ObjectReferenceVisitor")
     @Uninterruptible(reason = "Bridge between uninterruptible and potentially interruptible code.", mayBeInlined = true, calleeMustBe = false)
-    private static boolean callVisitor(ObjectVisitor visitor, Object obj) {
-        return visitor.visitObjectInline(obj);
+    private static void callVisitor(ObjectVisitor visitor, Object obj) {
+        visitor.visitObject(obj);
     }
 
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)