[GR-59590] Reuse dispatch table slots from parents in open type world hub layout.

teshull · teshull · commit 5c69def84e78 · 2025-08-13T07:54:11.000Z
PullRequest: graal/21703
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/OpenTypeWorldFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/OpenTypeWorldFeature.java
@@ -194,6 +194,10 @@ private static HostedType[] getAllInterfaces(Map<HostedType, HostedType[]> allIn
         return result;
     }
 
+    public static boolean matchingSignature(HostedMethod o1, HostedMethod o2) {
+        return matchingSignature(o1.wrapped, o2.wrapped);
+    }
+
     private static boolean matchingSignature(AnalysisMethod o1, AnalysisMethod o2) {
         if (o1.equals(o2)) {
             return true;
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LayeredDispatchTableFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LayeredDispatchTableFeature.java
@@ -36,6 +36,7 @@
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Supplier;
+import java.util.stream.IntStream;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 
@@ -49,7 +50,6 @@
 import com.oracle.graal.pointsto.meta.AnalysisType;
 import com.oracle.objectfile.ObjectFile;
 import com.oracle.svm.core.InvalidMethodPointerHandler;
-import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.config.ConfigurationValues;
 import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
 import com.oracle.svm.core.feature.InternalFeature;
@@ -145,6 +145,8 @@ public static final class Options {
 
     static final int INVALID_HOSTED_METHOD_INDEX = -1;
 
+    private static final String UNRESOLVED_VTABLE_ENTRY_SYMBOL = "__svm_vtableSym_";
+
     @Override
     public boolean isInConfiguration(Feature.IsInConfigurationAccess access) {
         return ImageLayerBuildingSupport.buildingImageLayer();
@@ -485,7 +487,7 @@ private void markRelocsWordInBitmap(BitSet bitmap, int offsetInHeap, ImageHeapLa
         bitmap.set(offsetInHeapRelocs / wordSize);
     }
 
-    private static String computeUnresolvedMethodSymbol(HostedDispatchSlot slotInfo, Map<ResolvedJavaMethod, String> methodToSymbolMap) {
+    private static String computeUnresolvedMethodSymbol(HostedDispatchSlot slotInfo, Map<ResolvedJavaMethod, String> methodToSymbolMap, Supplier<String> symbolNameSupplier) {
         /*
          * First try to determine the resolved method. This is useful for deduplication.
          */
@@ -503,10 +505,9 @@ private static String computeUnresolvedMethodSymbol(HostedDispatchSlot slotInfo,
 
         String unresolvedTableSymbol;
         if (resolvedMethod != null) {
-            unresolvedTableSymbol = methodToSymbolMap.computeIfAbsent(resolvedMethod, k -> String.format("%s_unresolvedVTableSym", NativeImage.localSymbolNameForMethod(k)));
+            unresolvedTableSymbol = methodToSymbolMap.computeIfAbsent(resolvedMethod, k -> symbolNameSupplier.get());
         } else {
-            unresolvedTableSymbol = SubstrateOptions.ImageSymbolsPrefix.getValue() +
-                            String.format("unresolvedVTableSym_typeid%s_slot%s", slotInfo.dispatchTable.type.getWrapped().getId(), slotInfo.slotIndex);
+            unresolvedTableSymbol = symbolNameSupplier.get();
         }
         return unresolvedTableSymbol;
     }
@@ -523,6 +524,11 @@ public void defineDispatchTableSlotSymbols(ObjectFile objectFile, ObjectFile.Sec
         Set<String> unresolvedVTableSymbolNames = generateUnresolvedSymbolNames ? new HashSet<>() : null;
 
         Map<ResolvedJavaMethod, String> deduplicatedMethodMap = new HashMap<>();
+        final var unresolvedSlotCount = IntStream.iterate(0, i -> i + 1).iterator();
+        Supplier<String> symbolNameSupplier = () -> {
+            int count = unresolvedSlotCount.nextInt();
+            return String.format("%s_%s", UNRESOLVED_VTABLE_ENTRY_SYMBOL, count);
+        };
 
         /*
          * First calculate symbols for all slots
@@ -543,9 +549,9 @@ public void defineDispatchTableSlotSymbols(ObjectFile objectFile, ObjectFile.Sec
                      * We need to make a symbol for this method which can be resolved in a later
                      * build if the target is compiled.
                      */
-                    symbol = computeUnresolvedMethodSymbol(slotInfo, deduplicatedMethodMap);
+                    symbol = computeUnresolvedMethodSymbol(slotInfo, deduplicatedMethodMap, symbolNameSupplier);
                     if (unresolvedVTableSymbolNames.add(symbol)) {
-                        objectFile.createUndefinedSymbol(symbol, 0, true);
+                        objectFile.createUndefinedSymbol(symbol, wordSize, true);
                     }
                 }
             } else {
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/VTableBuilder.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/VTableBuilder.java
@@ -42,6 +42,7 @@
 import com.oracle.svm.core.SubstrateUtil;
 import com.oracle.svm.core.hub.RuntimeClassLoading;
 import com.oracle.svm.core.imagelayer.ImageLayerBuildingSupport;
+import com.oracle.svm.hosted.OpenTypeWorldFeature;
 import com.oracle.svm.hosted.imagelayer.LayeredDispatchTableFeature;
 
 import jdk.graal.compiler.debug.Assertions;
@@ -182,15 +183,40 @@ private boolean verifyOpenTypeWorldDispatchTables() {
     }
 
     private List<HostedMethod> generateITable(HostedType type) {
-        return generateDispatchTable(type, 0);
+        return generateDispatchTable(type, List.of());
     }
 
-    private List<HostedMethod> generateDispatchTable(HostedType type, int startingIndex) {
+    /**
+     * Tries to find an existing parent slot with an identical signature. If successful, this
+     * method's index can be assigned to the same slot and no new dispatch slot is needed.
+     */
+    private static boolean findAndLinkToParentSlot(HostedMethod hMethod, List<HostedMethod> parentSlots) {
+        for (int i = 0; i < parentSlots.size(); i++) {
+            HostedMethod candidate = parentSlots.get(i);
+            if (OpenTypeWorldFeature.matchingSignature(hMethod, candidate)) {
+                assert candidate.computedVTableIndex == i : candidate.computedVTableIndex;
+                installVTableIndex(hMethod, candidate.computedVTableIndex);
+                return true;
+            }
+        }
+        return false;
+    }
+
+    private static void installVTableIndex(HostedMethod hMethod, int index) {
+        assert hMethod.computedVTableIndex == HostedMethod.MISSING_VTABLE_IDX : hMethod.computedVTableIndex;
+        hMethod.computedVTableIndex = index;
+    }
+
+    private List<HostedMethod> generateDispatchTable(HostedType type, List<HostedMethod> parentClassTable) {
         Predicate<HostedMethod> includeMethod;
         if (openHubUtils.filterVTableMethods(type)) {
             // include only methods which will be indirect calls
             includeMethod = m -> {
                 assert !m.isConstructor() : Assertions.errorMessage("Constructors should never be in dispatch tables", m);
+                if (findAndLinkToParentSlot(m, parentClassTable)) {
+                    // a prior slot has been found
+                    return false;
+                }
                 if (m.implementations.length > 1) {
                     return true;
                 } else {
@@ -204,6 +230,10 @@ private List<HostedMethod> generateDispatchTable(HostedType type, int startingIn
         } else {
             includeMethod = m -> {
                 assert !m.isConstructor() : Assertions.errorMessage("Constructors should never be in dispatch tables", m);
+                if (findAndLinkToParentSlot(m, parentClassTable)) {
+                    // a prior slot has been found
+                    return false;
+                }
                 /*
                  * We have to use the analysis method's canBeStaticallyBound implementation because
                  * within HostedMethod we sometimes do additional pruning when operating under the
@@ -214,11 +244,10 @@ private List<HostedMethod> generateDispatchTable(HostedType type, int startingIn
         }
         var table = type.getWrapped().getOpenTypeWorldDispatchTableMethods().stream().map(hUniverse::lookup).filter(includeMethod).sorted(HostedUniverse.METHOD_COMPARATOR).toList();
 
-        int index = startingIndex;
+        int index = parentClassTable.size();
         for (HostedMethod typeMethod : table) {
             assert typeMethod.getDeclaringClass().equals(type) : typeMethod;
-            assert typeMethod.computedVTableIndex == HostedMethod.MISSING_VTABLE_IDX : typeMethod.computedVTableIndex;
-            typeMethod.computedVTableIndex = index;
+            installVTableIndex(typeMethod, index);
             index++;
         }
 
@@ -229,40 +258,42 @@ private List<HostedMethod> generateDispatchTable(HostedType type, int startingIn
         return table;
     }
 
-    private void generateOpenTypeWorldDispatchTable(HostedInstanceClass type, Map<HostedType, List<HostedMethod>> dispatchTablesMap, HostedMethod invalidDispatchTableEntryHandler) {
+    private void generateOpenTypeWorldDispatchTable(HostedInstanceClass type, Map<HostedType, List<HostedMethod>> classTablesMap, HostedMethod invalidDispatchTableEntryHandler) {
         var superClass = type.getSuperclass();
-        List<HostedMethod> parentClassTable = superClass == null ? List.of() : dispatchTablesMap.get(superClass);
-        List<HostedMethod> classTableWithoutSuper = generateDispatchTable(type, parentClassTable.size());
-        List<HostedMethod> resultClassTableMethods;
+        List<HostedMethod> parentClassTable = superClass == null ? List.of() : classTablesMap.get(superClass);
+        List<HostedMethod> classTableWithoutSuper = generateDispatchTable(type, parentClassTable);
+        List<HostedMethod> classTableMethods;
         if (!classTableWithoutSuper.isEmpty()) {
-            resultClassTableMethods = new ArrayList<>(parentClassTable);
-            resultClassTableMethods.addAll(classTableWithoutSuper);
+            classTableMethods = new ArrayList<>(parentClassTable);
+            classTableMethods.addAll(classTableWithoutSuper);
         } else {
             /*
              * If the type doesn't declare any new methods, then we can use the parent's class
              * table.
              */
-            resultClassTableMethods = parentClassTable;
+            classTableMethods = parentClassTable;
         }
-        dispatchTablesMap.put(type, resultClassTableMethods);
+        classTablesMap.put(type, classTableMethods);
 
         if (!type.isAbstract()) {
             // create concrete dispatch classes
-            List<HostedMethod> aggregatedTable = new ArrayList<>(resultClassTableMethods);
+            List<HostedMethod> aggregatedTable = new ArrayList<>(classTableMethods);
             HostedType[] interfaces = type.typeCheckInterfaceOrder;
             type.itableStartingOffsets = new int[interfaces.length];
-            int currentITableOffset = resultClassTableMethods.size();
+            int currentITableOffset = classTableMethods.size();
             for (int i = 0; i < interfaces.length; i++) {
                 HostedType interfaceType = interfaces[i];
-                List<HostedMethod> interfaceMethods = dispatchTablesMap.get(interfaceType);
+                List<HostedMethod> interfaceMethods = classTablesMap.get(interfaceType);
 
                 type.itableStartingOffsets[i] = currentITableOffset;
                 aggregatedTable.addAll(interfaceMethods);
                 currentITableOffset += interfaceMethods.size();
             }
             type.openTypeWorldDispatchTables = new HostedMethod[aggregatedTable.size()];
             type.openTypeWorldDispatchTableSlotTargets = aggregatedTable.toArray(HostedMethod[]::new);
+
             boolean[] validTarget = new boolean[aggregatedTable.size()];
+            Set<HostedMethod> seenResolvedMethods = SubstrateUtil.assertionsEnabled() ? new HashSet<>() : null;
             for (int i = 0; i < aggregatedTable.size(); i++) {
                 HostedMethod method = aggregatedTable.get(i);
                 /*
@@ -275,6 +306,13 @@ private void generateOpenTypeWorldDispatchTable(HostedInstanceClass type, Map<Ho
                     if (resolvedMethod != null) {
                         targetMethod = resolvedMethod;
                         validTarget[i] = true;
+                        if (seenResolvedMethods != null && i < classTableMethods.size()) {
+                            /*
+                             * Check that each resolved method within the class table is unique
+                             */
+                            var added = seenResolvedMethods.add(resolvedMethod);
+                            assert added : Assertions.errorMessage("Multiple slots with same resolution method", resolvedMethod);
+                        }
                     }
                 }
 
@@ -289,7 +327,7 @@ private void generateOpenTypeWorldDispatchTable(HostedInstanceClass type, Map<Ho
             assert !type.isInterface();
             List<HostedMethod> sourceTable;
             if (type.isAbstract()) {
-                sourceTable = resultClassTableMethods;
+                sourceTable = classTableMethods;
             } else {
                 sourceTable = Arrays.asList(type.openTypeWorldDispatchTableSlotTargets);
             }
@@ -306,13 +344,16 @@ private void generateOpenTypeWorldDispatchTable(HostedInstanceClass type, Map<Ho
 
         for (HostedType subType : type.subTypes) {
             if (subType instanceof HostedInstanceClass instanceClass && openHubUtils.shouldIncludeType(subType)) {
-                generateOpenTypeWorldDispatchTable(instanceClass, dispatchTablesMap, invalidDispatchTableEntryHandler);
+                generateOpenTypeWorldDispatchTable(instanceClass, classTablesMap, invalidDispatchTableEntryHandler);
             }
         }
     }
 
     private void buildOpenTypeWorldDispatchTables() {
-        Map<HostedType, List<HostedMethod>> dispatchTablesMap = new HashMap<>();
+        /*
+         * Map from type to class table (i.e. the type's vtable w/o any appended itables).
+         */
+        Map<HostedType, List<HostedMethod>> classTablesMap = new HashMap<>();
 
         for (HostedType type : hUniverse.getTypes()) {
             /*
@@ -321,7 +362,7 @@ private void buildOpenTypeWorldDispatchTables() {
              */
             if (type.isInterface() && openHubUtils.shouldIncludeType(type)) {
                 List<HostedMethod> itable = generateITable(type);
-                dispatchTablesMap.put(type, itable);
+                classTablesMap.put(type, itable);
                 if (RuntimeClassLoading.isSupported()) {
                     type.cremaOpenTypeWorldDispatchTables = new HostedMethod[itable.size()];
                     for (int i = 0; i < itable.size(); i++) {
@@ -332,7 +373,7 @@ private void buildOpenTypeWorldDispatchTables() {
         }
 
         HostedMethod invalidDispatchTableEntryHandler = hMetaAccess.lookupJavaMethod(InvalidMethodPointerHandler.INVALID_VTABLE_ENTRY_HANDLER_METHOD);
-        generateOpenTypeWorldDispatchTable((HostedInstanceClass) hUniverse.objectType(), dispatchTablesMap, invalidDispatchTableEntryHandler);
+        generateOpenTypeWorldDispatchTable((HostedInstanceClass) hUniverse.objectType(), classTablesMap, invalidDispatchTableEntryHandler);
 
         int[] emptyITableOffsets = new int[0];
         var objectType = hUniverse.getObjectClass();
