[GR-67701] [GR-68034] Improve the parallelism of ImageHeapCollectionFeature and the associated ImageHeap{List,Map} data structures

PullRequest: graal/21495

Author: d-kozak

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/util/ImageHeapList.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -40,14 +40,19 @@
 /**
  * A list that is filled at image build time while the static analysis is running, and then read at
  * run time.
- *
+ * <p>
  * Filling the list at image build time is thread safe. Every object added to the list while the
  * static analysis is running is properly added to the shadow heap.
- *
+ * <p>
  * The list is immutable at run time. The run-time list can optionally be sorted, to make code at
  * run time deterministic regardless of the order in which elements are discovered and added at
  * image build time. Sorting happens at image build time, but does not affect the list that users
  * are adding to at image build time.
+ * <p>
+ * We support (mostly) <b>append-only</b> semantics. Removing elements is not allowed. Changing the
+ * content of the list via iterator is <b>not supported</b>. Note that we allow updating elements at
+ * existing indexes as this operation is already used. We have deliberately chosen this design to
+ * limit the number of analysis rescans needed.
  */
 @Platforms(Platform.HOSTED_ONLY.class) //
 public final class ImageHeapList {
@@ -76,6 +81,12 @@ public static final class HostedImageHeapList<E> extends AbstractList<E> {
         private final Comparator<E> comparator;
         private final List<E> hostedList;
         public final RuntimeImageHeapList<E> runtimeList;
+        /**
+         * Used to signal if this list has been modified. If true, the change should be propagated
+         * from the hosted list to the runtime list by calling {@link #update()}. This variable
+         * should <b>always</b> be accessed within a <code>synchronized</code> block guarded by the
+         * object's intrinsic lock.
+         */
         private boolean modified;
 
         @SuppressWarnings("unchecked")
@@ -115,9 +126,8 @@ public synchronized void add(int index, E element) {
         }
 
         @Override
-        public synchronized E remove(int index) {
-            modified = true;
-            return hostedList.remove(index);
+        public E remove(int index) {
+            throw notSupported();
         }
 
         @Override
@@ -138,6 +148,14 @@ public synchronized E set(int index, E element) {
         public synchronized int size() {
             return hostedList.size();
         }
+
+        private static UnsupportedOperationException notSupported() {
+            /*
+             * We have deliberately chosen to only support append-only behavior to limit the number
+             * of analysis rescans needed.
+             */
+            throw new UnsupportedOperationException("ImageHeapList has append-only semantics. Removing elements is forbidden.");
+        }
     }
 }
 

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/util/ImageHeapMap.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -26,6 +26,8 @@
 
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.function.BiFunction;
 
 import org.graalvm.collections.EconomicMap;
 import org.graalvm.collections.EconomicMapWrap;
@@ -51,6 +53,10 @@
  * <p>
  * This map implementation allows thread-safe collection of data at image build time and storing it
  * into a space efficient data structure at run time.
+ * <p>
+ * We support <b>append-only</b> semantics. Once added, the elements should never be removed.
+ * Changing the contents of the map via {key, value, entry} iterators is <b>not supported</b>. We
+ * have deliberately chosen this design to limit the number of analysis rescans needed.
  */
 @Platforms(Platform.HOSTED_ONLY.class) //
 public final class ImageHeapMap {
@@ -107,6 +113,12 @@ public static final class HostedImageHeapMap<K, V> extends EconomicMapWrap<K, V>
         private final EconomicMap<Object, Object> currentLayerMap;
         private final EconomicMap<Object, Object> runtimeMap;
 
+        /**
+         * Used to signal if the wrapped map has been modified. If true, the change should be
+         * propagated to the {@link #currentLayerMap} by calling {@link #update()}.
+         */
+        private volatile boolean modified;
+
         /**
          * The {code key} is only used in the Layered Image context, to link the maps across each
          * layer. If an {@link ImageHeapMap} is in a singleton that is already layer aware, there is
@@ -116,10 +128,20 @@ public static final class HostedImageHeapMap<K, V> extends EconomicMapWrap<K, V>
          */
         public HostedImageHeapMap(Map<K, V> hostedMap, EconomicMap<Object, Object> currentLayerMap, EconomicMap<Object, Object> runtimeMap) {
             super(hostedMap);
+            assert hostedMap instanceof ConcurrentMap<K, V> : "The implementation of HostedImageHeapMap assumes the wrapped map is thread-safe: " + hostedMap;
             this.currentLayerMap = currentLayerMap;
             this.runtimeMap = runtimeMap;
         }
 
+        public boolean needsUpdate() {
+            return modified;
+        }
+
+        public void update() {
+            modified = false;
+            currentLayerMap.putAll(this);
+        }
+
         @Platforms(Platform.HOSTED_ONLY.class) //
         public EconomicMap<Object, Object> getCurrentLayerMap() {
             return currentLayerMap;
@@ -155,5 +177,47 @@ public static <K, V> HostedImageHeapMap<K, V> create(Equivalence strategy, Strin
                 return new HostedImageHeapMap<>(hostedMap, currentLayerMap, currentLayerMap);
             }
         }
+
+        @Override
+        public V put(K key, V value) {
+            modified = true;
+            return super.put(key, value);
+        }
+
+        @Override
+        public V putIfAbsent(K key, V value) {
+            V previous = super.putIfAbsent(key, value);
+            if (previous == null) {
+                /*
+                 * Either there was no previous value or the previous value was null. In both cases,
+                 * the map was modified.
+                 */
+                modified = true;
+            }
+            return previous;
+        }
+
+        @Override
+        public void clear() {
+            throw notSupported();
+        }
+
+        @Override
+        public V removeKey(K key) {
+            throw notSupported();
+        }
+
+        @Override
+        public void replaceAll(BiFunction<? super K, ? super V, ? extends V> function) {
+            throw notSupported();
+        }
+
+        private static UnsupportedOperationException notSupported() {
+            /*
+             * We have deliberately chosen to only support append-only behavior to limit the number
+             * of analysis rescans needed.
+             */
+            throw new UnsupportedOperationException("ImageHeapMap has append-only semantics. Replacing or removing elements is forbidden.");
+        }
     }
 }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/ImageHeapCollectionFeature.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -28,9 +28,6 @@
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 
-import org.graalvm.collections.EconomicMap;
-import org.graalvm.collections.MapCursor;
-
 import com.oracle.svm.core.BuildPhaseProvider;
 import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
 import com.oracle.svm.core.feature.InternalFeature;
@@ -76,31 +73,46 @@ private Object replaceHostedWithRuntime(Object obj) {
         return obj;
     }
 
+    /**
+     * This method makes sure that the content of all modified {@link HostedImageHeapMap}s and
+     * {@link HostedImageHeapList}s is properly propagated to their runtime counterparts. As both
+     * the number of these collections and their individual sizes are theoretically unbounded, we
+     * use <i>parallel streams</i> to divide the load across all cores.
+     * <p>
+     * We split the process into two stages. First, the content of each modified collection is
+     * propagated from the hosted to the runtime version. Then, the modified runtime collections are
+     * rescanned. The split is done to prevent concurrent modifications of the hosted collections
+     * during the execution of this method, as they may be updated indirectly during the heap
+     * scanning.
+     */
     @Override
     public void duringAnalysis(DuringAnalysisAccess a) {
         DuringAnalysisAccessImpl access = (DuringAnalysisAccessImpl) a;
         if (ImageLayerBuildingSupport.buildingExtensionLayer()) {
             allMaps.addAll(LayeredHostedImageHeapMapCollector.singleton().getPreviousLayerReachableMaps());
         }
-        for (var hostedImageHeapMap : allMaps) {
-            if (needsUpdate(hostedImageHeapMap)) {
-                update(hostedImageHeapMap);
-                access.rescanObject(hostedImageHeapMap.getCurrentLayerMap());
-                access.requireAnalysisIteration();
+        Set<Object> objectsToRescan = ConcurrentHashMap.newKeySet();
+        allMaps.parallelStream().forEach(hostedImageHeapMap -> {
+            if (hostedImageHeapMap.needsUpdate()) {
+                hostedImageHeapMap.update();
+                objectsToRescan.add(hostedImageHeapMap.getCurrentLayerMap());
             }
-        }
-        for (var hostedImageHeapList : allLists) {
+        });
+        allLists.parallelStream().forEach(hostedImageHeapList -> {
             if (hostedImageHeapList.needsUpdate()) {
                 hostedImageHeapList.update();
-                access.rescanObject(hostedImageHeapList.runtimeList);
-                access.requireAnalysisIteration();
+                objectsToRescan.add(hostedImageHeapList.runtimeList);
             }
+        });
+        if (!objectsToRescan.isEmpty()) {
+            objectsToRescan.parallelStream().forEach(access::rescanObject);
+            access.requireAnalysisIteration();
         }
     }
 
     public boolean needsUpdate() {
         for (var hostedImageHeapMap : allMaps) {
-            if (needsUpdate(hostedImageHeapMap)) {
+            if (hostedImageHeapMap.needsUpdate()) {
                 return true;
             }
         }
@@ -115,7 +127,7 @@ public boolean needsUpdate() {
     @Override
     public void afterImageWrite(AfterImageWriteAccess access) {
         for (var hostedImageHeapMap : allMaps) {
-            if (needsUpdate(hostedImageHeapMap)) {
+            if (hostedImageHeapMap.needsUpdate()) {
                 throw VMError.shouldNotReachHere("ImageHeapMap modified after static analysis:%n%s%n%s",
                                 hostedImageHeapMap, hostedImageHeapMap.getCurrentLayerMap());
             }
@@ -128,25 +140,4 @@ public void afterImageWrite(AfterImageWriteAccess access) {
             }
         }
     }
-
-    private static boolean needsUpdate(HostedImageHeapMap<?, ?> hostedMap) {
-        EconomicMap<Object, Object> runtimeMap = hostedMap.getCurrentLayerMap();
-        if (hostedMap.size() != runtimeMap.size()) {
-            return true;
-        }
-        MapCursor<?, ?> hostedEntry = hostedMap.getEntries();
-        while (hostedEntry.advance()) {
-            Object hostedValue = hostedEntry.getValue();
-            Object runtimeValue = runtimeMap.get(hostedEntry.getKey());
-            if (hostedValue != runtimeValue) {
-                return true;
-            }
-        }
-        return false;
-    }
-
-    private static void update(HostedImageHeapMap<?, ?> hostedMap) {
-        hostedMap.getCurrentLayerMap().clear();
-        hostedMap.getCurrentLayerMap().putAll(hostedMap);
-    }
 }