[GR-67560] Remove unnecessary image heap partition alignment and filler object support.

PullRequest: graal/21532

Author: peter-hofer

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

@@ -28,19 +28,13 @@
 import java.util.List;
 
 import com.oracle.svm.core.config.ConfigurationValues;
-import com.oracle.svm.core.image.ImageHeap;
 import com.oracle.svm.core.image.ImageHeapObject;
-import com.oracle.svm.core.image.ImageHeapPartition;
 import com.oracle.svm.core.util.UnsignedUtils;
 import com.oracle.svm.core.util.VMError;
 
-import jdk.graal.compiler.core.common.NumUtil;
 import jdk.graal.compiler.word.Word;
 
 class ChunkedImageHeapAllocator {
-    /** A pseudo-partition for filler objects, see {@link FillerObjectDummyPartition}. */
-    private static final ImageHeapPartition FILLERS_DUMMY_PARTITION = new FillerObjectDummyPartition();
-
     abstract static class Chunk {
         private final long begin;
         private final long endOffset;
@@ -116,35 +110,6 @@ public long allocate(ImageHeapObject obj, boolean writable) {
             return objStart;
         }
 
-        public boolean tryAlignTop(int multiple) {
-            long padding = computePadding(getTop(), multiple);
-            if (padding != 0 && padding < minimumObjectSize) {
-                /*
-                 * Cannot fit a filler object into the remaining padding space, so need to go up to
-                 * the next alignment multiple.
-                 */
-                padding = padding + multiple;
-            }
-            if (padding > getUnallocatedBytes()) {
-                return false;
-            }
-            allocateFiller(padding);
-            assert getTop() % multiple == 0;
-            return true;
-        }
-
-        private void allocateFiller(long size) {
-            if (size != 0) {
-                ImageHeapObject filler = imageHeap.addFillerObject(NumUtil.safeToInt(size));
-                if (filler == null) {
-                    throw VMError.shouldNotReachHere("Failed to leave enough space for a filler object: " + size + " byte remaining");
-                }
-                filler.setHeapPartition(FILLERS_DUMMY_PARTITION);
-                long location = allocate(filler, false);
-                filler.setOffsetInPartition(location);
-            }
-        }
-
         public List<ImageHeapObject> getObjects() {
             return objects;
         }
@@ -163,7 +128,6 @@ public long getUnallocatedBytes() {
         }
     }
 
-    private final ImageHeap imageHeap;
     private final int alignedChunkSize;
     private final int alignedChunkAlignment;
     private final int alignedChunkObjectsOffset;
@@ -176,8 +140,7 @@ public long getUnallocatedBytes() {
 
     final int minimumObjectSize;
 
-    ChunkedImageHeapAllocator(ImageHeap imageHeap, long position) {
-        this.imageHeap = imageHeap;
+    ChunkedImageHeapAllocator(long position) {
         this.alignedChunkSize = UnsignedUtils.safeToInt(HeapParameters.getAlignedHeapChunkSize());
         this.alignedChunkAlignment = UnsignedUtils.safeToInt(HeapParameters.getAlignedHeapChunkAlignment());
         this.alignedChunkObjectsOffset = UnsignedUtils.safeToInt(AlignedHeapChunk.getObjectsStartOffset());
@@ -192,11 +155,6 @@ public long getPosition() {
         return (currentAlignedChunk != null) ? currentAlignedChunk.getTop() : position;
     }
 
-    public void alignBetweenChunks(int multiple) {
-        assert currentAlignedChunk == null;
-        allocateRaw(computePadding(position, multiple));
-    }
-
     public long allocateUnalignedChunkForObject(ImageHeapObject obj, boolean writable) {
         assert currentAlignedChunk == null;
         long objSize = obj.getSize();
@@ -220,6 +178,11 @@ public void startNewAlignedChunk() {
         alignedChunks.add(currentAlignedChunk);
     }
 
+    private void alignBetweenChunks(int multiple) {
+        assert currentAlignedChunk == null;
+        allocateRaw(computePadding(position, multiple));
+    }
+
     public long getRemainingBytesInAlignedChunk() {
         return currentAlignedChunk.getUnallocatedBytes();
     }
@@ -228,15 +191,6 @@ public long allocateObjectInAlignedChunk(ImageHeapObject obj, boolean writable)
         return currentAlignedChunk.allocate(obj, writable);
     }
 
-    public void alignInAlignedChunk(int multiple) {
-        if (!currentAlignedChunk.tryAlignTop(multiple)) {
-            startNewAlignedChunk();
-            if (!currentAlignedChunk.tryAlignTop(multiple)) {
-                throw VMError.shouldNotReachHere("Cannot align to " + multiple + " bytes within an aligned chunk's object area");
-            }
-        }
-    }
-
     public void finishAlignedChunk() {
         currentAlignedChunk = null;
     }
@@ -261,40 +215,3 @@ private static long computePadding(long offset, int alignment) {
         return (remainder == 0) ? 0 : (alignment - remainder);
     }
 }
-
-/**
- * A pseudo-partition for filler objects, which does not really exist at runtime, in any statistics,
- * or otherwise. Necessary because like all other {@link ImageHeapObject}s, filler objects must be
- * assigned to a partition, the start offset of which is used to compute their absolute locations.
- * <p>
- * For filler objects in the middle of a partition (between genuine objects of that partition), it
- * would be acceptable to assign them to their enclosing partition. However, filler objects that are
- * inserted between partitions for alignment purposes are problematic because if they were assigned
- * to either partition, they would either be out of the partition's boundaries, or they would change
- * those boundaries, which would make them useless because that's exactly why they are needed there.
- */
-final class FillerObjectDummyPartition implements ImageHeapPartition {
-    /**
-     * Zero so that the partition-relative offsets of filler objects are always their absolute
-     * locations.
-     */
-    @Override
-    public long getStartOffset() {
-        return 0;
-    }
-
-    @Override
-    public String getName() {
-        throw VMError.shouldNotReachHereAtRuntime(); // ExcludeFromJacocoGeneratedReport
-    }
-
-    @Override
-    public long getSize() {
-        throw VMError.shouldNotReachHereAtRuntime(); // ExcludeFromJacocoGeneratedReport
-    }
-
-    @Override
-    public boolean isFiller() {
-        return true;
-    }
-}

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

@@ -30,7 +30,6 @@
 import org.graalvm.nativeimage.ImageInfo;
 import org.graalvm.word.UnsignedWord;
 
-import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.SubstrateUtil;
 import com.oracle.svm.core.config.ConfigurationValues;
 import com.oracle.svm.core.genscavenge.ChunkedImageHeapAllocator.AlignedChunk;
@@ -39,8 +38,8 @@
 import com.oracle.svm.core.genscavenge.remset.RememberedSet;
 import com.oracle.svm.core.hub.DynamicHub;
 import com.oracle.svm.core.image.ImageHeap;
-import com.oracle.svm.core.image.ImageHeapLayouter;
 import com.oracle.svm.core.image.ImageHeapLayoutInfo;
+import com.oracle.svm.core.image.ImageHeapLayouter;
 import com.oracle.svm.core.image.ImageHeapObject;
 import com.oracle.svm.core.option.SubstrateOptionsParser;
 import com.oracle.svm.core.util.UserError;
@@ -89,14 +88,13 @@ public class ChunkedImageHeapLayouter implements ImageHeapLayouter {
     /** @param startOffset Offset relative to the heap base. */
     @SuppressWarnings("this-escape")
     public ChunkedImageHeapLayouter(ImageHeapInfo heapInfo, long startOffset) {
-        int alignment = ConfigurationValues.getObjectLayout().getAlignment();
         this.partitions = new ChunkedImageHeapPartition[PARTITION_COUNT];
-        this.partitions[READ_ONLY_REGULAR] = new ChunkedImageHeapPartition("readOnly", false, false, alignment, alignment);
-        this.partitions[READ_ONLY_RELOCATABLE] = new ChunkedImageHeapPartition("readOnlyRelocatable", false, false, alignment, alignment);
-        this.partitions[WRITABLE_PATCHED] = new ChunkedImageHeapPartition("writablePatched", true, false, alignment, alignment);
-        this.partitions[WRITABLE_REGULAR] = new ChunkedImageHeapPartition("writable", true, false, alignment, alignment);
-        this.partitions[WRITABLE_HUGE] = new ChunkedImageHeapPartition("writableHuge", true, true, alignment, alignment);
-        this.partitions[READ_ONLY_HUGE] = new ChunkedImageHeapPartition("readOnlyHuge", false, true, alignment, SubstrateOptions.getPageSize());
+        this.partitions[READ_ONLY_REGULAR] = new ChunkedImageHeapPartition("readOnly", false, false);
+        this.partitions[READ_ONLY_RELOCATABLE] = new ChunkedImageHeapPartition("readOnlyRelocatable", false, false);
+        this.partitions[WRITABLE_PATCHED] = new ChunkedImageHeapPartition("writablePatched", true, false);
+        this.partitions[WRITABLE_REGULAR] = new ChunkedImageHeapPartition("writable", true, false);
+        this.partitions[WRITABLE_HUGE] = new ChunkedImageHeapPartition("writableHuge", true, true);
+        this.partitions[READ_ONLY_HUGE] = new ChunkedImageHeapPartition("readOnlyHuge", false, true);
 
         this.heapInfo = heapInfo;
         this.startOffset = startOffset;
@@ -166,16 +164,20 @@ public ImageHeapLayoutInfo layout(ImageHeap imageHeap, int pageSize, ImageHeapLa
 
         ImageHeapLayoutInfo layoutInfo = doLayout(imageHeap, pageSize, control);
 
+        /*
+         * In case there is a need for more alignment between partitions or between objects in a
+         * chunk, see the version history of this file (and package) for a earlier implementation.
+         */
         for (ChunkedImageHeapPartition partition : getPartitions()) {
-            assert partition.getStartOffset() % partition.getStartAlignment() == 0 : partition;
-            assert (partition.getStartOffset() + partition.getSize()) % partition.getEndAlignment() == 0 : partition;
+            assert partition.getStartOffset() % objectAlignment == 0 : partition;
+            assert (partition.getStartOffset() + partition.getSize()) % objectAlignment == 0 : partition;
         }
         assert layoutInfo.getImageHeapSize() % pageSize == 0 : "Image heap size is not a multiple of page size";
         return layoutInfo;
     }
 
     private ImageHeapLayoutInfo doLayout(ImageHeap imageHeap, int pageSize, ImageHeapLayouterControl control) {
-        allocator = new ChunkedImageHeapAllocator(imageHeap, startOffset);
+        allocator = new ChunkedImageHeapAllocator(startOffset);
         for (ChunkedImageHeapPartition partition : getPartitions()) {
             control.poll();
             partition.layout(allocator, control);
@@ -219,7 +221,9 @@ private ImageHeapLayoutInfo populateInfoObjects(int dynamicHubCount, int pageSiz
 
         long writableEnd = getWritableHuge().getStartOffset() + getWritableHuge().getSize();
         long writableSize = writableEnd - offsetOfFirstWritableAlignedChunk;
-        long imageHeapSize = getReadOnlyHuge().getStartOffset() + getReadOnlyHuge().getSize() - startOffset;
+        /* Aligning the end to the page size can be required for mapping into memory. */
+        long imageHeapEnd = NumUtil.roundUp(getReadOnlyHuge().getStartOffset() + getReadOnlyHuge().getSize(), pageSize);
+        long imageHeapSize = imageHeapEnd - startOffset;
         return new ImageHeapLayoutInfo(startOffset, imageHeapSize, offsetOfFirstWritableAlignedChunk, writableSize, getReadOnlyRelocatable().getStartOffset(), getReadOnlyRelocatable().getSize(),
                         getWritablePatched().getStartOffset(), getWritablePatched().getSize());
     }

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

@@ -46,8 +46,6 @@ public class ChunkedImageHeapPartition implements ImageHeapPartition {
     private final String name;
     private final boolean writable;
     private final boolean hugeObjects;
-    private final int startAlignment;
-    private final int endAlignment;
     private final int minimumObjectSize;
     private final List<ImageHeapObject> objects = new ArrayList<>();
 
@@ -57,12 +55,10 @@ public class ChunkedImageHeapPartition implements ImageHeapPartition {
     long startOffset = -1;
     long endOffset = -1;
 
-    ChunkedImageHeapPartition(String name, boolean writable, boolean hugeObjects, int startAlignment, int endAlignment) {
+    ChunkedImageHeapPartition(String name, boolean writable, boolean hugeObjects) {
         this.name = name;
         this.writable = writable;
         this.hugeObjects = hugeObjects;
-        this.startAlignment = startAlignment;
-        this.endAlignment = endAlignment;
 
         /* Cache to prevent frequent lookups of the object layout from ImageSingletons. */
         this.minimumObjectSize = ConfigurationValues.getObjectLayout().getMinImageHeapObjectSize();
@@ -82,27 +78,32 @@ void layout(ChunkedImageHeapAllocator allocator, ImageHeapLayouterControl contro
     }
 
     private void layoutInUnalignedChunks(ChunkedImageHeapAllocator allocator, ImageHeapLayouterControl control) {
+        if (objects.isEmpty()) {
+            /*
+             * Without objects, don't force finishing the current chunk and therefore committing
+             * space for the rest of it. Another partition might be able to continue filling it, or,
+             * if no more objects follow, we don't need to dedicate space in the image at all.
+             */
+            startOffset = allocator.getPosition();
+            endOffset = startOffset;
+            return;
+        }
+
         allocator.finishAlignedChunk();
-        allocator.alignBetweenChunks(getStartAlignment());
         startOffset = allocator.getPosition();
 
         for (ImageHeapObject info : objects) { // No need to sort by size
             appendAllocatedObject(info, allocator.allocateUnalignedChunkForObject(info, isWritable()));
             control.poll();
         }
 
-        allocator.alignBetweenChunks(getEndAlignment());
         endOffset = allocator.getPosition();
     }
 
     private void layoutInAlignedChunks(ChunkedImageHeapAllocator allocator, ImageHeapLayouterControl control) {
         allocator.maybeStartAlignedChunk();
-        allocator.alignInAlignedChunk(getStartAlignment());
         startOffset = allocator.getPosition();
-
         allocateObjectsInAlignedChunks(allocator, control);
-
-        allocator.alignInAlignedChunk(getEndAlignment());
         endOffset = allocator.getPosition();
     }
 
@@ -176,14 +177,6 @@ boolean usesUnalignedObjects() {
         return hugeObjects;
     }
 
-    final int getStartAlignment() {
-        return startAlignment;
-    }
-
-    final int getEndAlignment() {
-        return endAlignment;
-    }
-
     @Override
     public long getStartOffset() {
         assert startOffset >= 0 : "Start offset not yet set";
@@ -200,11 +193,6 @@ public long getSize() {
         return getEndOffset() - getStartOffset();
     }
 
-    @Override
-    public boolean isFiller() {
-        return false;
-    }
-
     @Override
     public String toString() {
         return name;

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

@@ -331,15 +331,12 @@ public static boolean isConsumedHeapChunkZapped(UnsignedWord header) {
     public long encodeHubPointerForImageHeap(ImageHeapObject obj, long hubOffsetFromHeapBase) {
         long header = hubOffsetFromHeapBase << numReservedExtraHubBits;
         assert (header & reservedHubBitsMask) == 0 : "Object header bits must be zero initially";
-        if (obj.getPartition() instanceof ChunkedImageHeapPartition partition) {
-            if (partition.isWritable() && HeapImpl.usesImageHeapCardMarking()) {
-                header |= REMSET_OR_MARKED1_BIT.rawValue();
-            }
-            if (partition.usesUnalignedObjects()) {
-                header |= UNALIGNED_BIT.rawValue();
-            }
-        } else {
-            assert obj.getPartition() instanceof FillerObjectDummyPartition;
+        ChunkedImageHeapPartition partition = (ChunkedImageHeapPartition) obj.getPartition();
+        if (partition.isWritable() && HeapImpl.usesImageHeapCardMarking()) {
+            header |= REMSET_OR_MARKED1_BIT.rawValue();
+        }
+        if (partition.usesUnalignedObjects()) {
+            header |= UNALIGNED_BIT.rawValue();
         }
         if (isIdentityHashFieldOptional()) {
             header |= (IDHASH_STATE_IN_FIELD.rawValue() << IDHASH_STATE_SHIFT);

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/image/ImageHeapPartition.java

@@ -39,7 +39,4 @@ public interface ImageHeapPartition {
      * Returns the size of the partition (i.e., the sum of all allocated objects + some overhead).
      */
     long getSize();
-
-    /* Returns true if this partition is only used as a filler. */
-    boolean isFiller();
 }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageDebugInfoProvider.java

@@ -150,7 +150,7 @@ public Stream<DebugCodeInfo> codeInfoProvider() {
 
     @Override
     public Stream<DebugDataInfo> dataInfoProvider() {
-        return heap.getObjects().stream().filter(this::acceptObjectInfo).map(this::createDebugDataInfo);
+        return heap.getObjects().stream().map(this::createDebugDataInfo);
     }
 
     private abstract class NativeImageDebugFileInfo implements DebugFileInfo {
@@ -2658,11 +2658,6 @@ public long getSize() {
         }
     }
 
-    private boolean acceptObjectInfo(ObjectInfo objectInfo) {
-        /* This condition rejects filler partition objects. */
-        return !objectInfo.getPartition().isFiller();
-    }
-
     private DebugDataInfo createDebugDataInfo(ObjectInfo objectInfo) {
         return new NativeImageDebugDataInfo(objectInfo);
     }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageHeap.java

@@ -1148,9 +1148,4 @@ public String getName() {
     public long getSize() {
         throw VMError.shouldNotReachHereAtRuntime(); // ExcludeFromJacocoGeneratedReport
     }
-
-    @Override
-    public boolean isFiller() {
-        return false;
-    }
 }

web-image/src/com.oracle.svm.hosted.webimage/src/com/oracle/svm/hosted/webimage/wasmgc/image/WasmGCPartition.java

@@ -100,9 +100,4 @@ public void add(ImageHeapObject obj) {
         objects.add(obj);
         obj.setHeapPartition(this);
     }
-
-    @Override
-    public boolean isFiller() {
-        return false;
-    }
 }