[GR-39018] Debug id cleanup #4627.

PullRequest: graal/11916

Author: fniephaus

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/ArrayTypeEntry.java

@@ -29,6 +29,7 @@
 import com.oracle.objectfile.debuginfo.DebugInfoProvider.DebugArrayTypeInfo;
 import com.oracle.objectfile.debuginfo.DebugInfoProvider.DebugTypeInfo;
 import com.oracle.objectfile.debuginfo.DebugInfoProvider.DebugTypeInfo.DebugTypeKind;
+import jdk.vm.ci.meta.ResolvedJavaType;
 import org.graalvm.compiler.debug.DebugContext;
 
 public class ArrayTypeEntry extends StructureTypeEntry {
@@ -48,13 +49,13 @@ public DebugTypeKind typeKind() {
     @Override
     public void addDebugInfo(DebugInfoBase debugInfoBase, DebugTypeInfo debugTypeInfo, DebugContext debugContext) {
         DebugArrayTypeInfo debugArrayTypeInfo = (DebugArrayTypeInfo) debugTypeInfo;
-        String elementTypeName = TypeEntry.canonicalize(debugArrayTypeInfo.elementType());
-        this.elementType = debugInfoBase.lookupTypeEntry(elementTypeName);
+        ResolvedJavaType eltType = debugArrayTypeInfo.elementType();
+        this.elementType = debugInfoBase.lookupTypeEntry(eltType);
         this.baseSize = debugArrayTypeInfo.baseSize();
         this.lengthOffset = debugArrayTypeInfo.lengthOffset();
         /* Add details of fields and field types */
         debugArrayTypeInfo.fieldInfoProvider().forEach(debugFieldInfo -> this.processField(debugFieldInfo, debugInfoBase, debugContext));
-        debugContext.log("typename %s element type %s base size %d length offset %d\n", typeName, elementTypeName, baseSize, lengthOffset);
+        debugContext.log("typename %s element type %s base size %d length offset %d\n", typeName, this.elementType.getTypeName(), baseSize, lengthOffset);
     }
 
     public TypeEntry getElementType() {

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/ClassEntry.java

@@ -32,6 +32,8 @@
 import java.util.List;
 import java.util.Map;
 
+import jdk.vm.ci.meta.ResolvedJavaMethod;
+import jdk.vm.ci.meta.ResolvedJavaType;
 import org.graalvm.compiler.debug.DebugContext;
 
 import com.oracle.objectfile.debuginfo.DebugInfoProvider.DebugFieldInfo;
@@ -64,9 +66,10 @@ public class ClassEntry extends StructureTypeEntry {
      */
     protected List<MethodEntry> methods;
     /**
-     * An index of all currently known methods keyed by the unique local symbol name of the method.
+     * An index of all currently known methods keyed by the unique, associated, identifying
+     * ResolvedJavaMethod.
      */
-    private Map<String, MethodEntry> methodsIndex;
+    private Map<ResolvedJavaMethod, MethodEntry> methodsIndex;
     /**
      * A list recording details of all primary ranges included in this class sorted by ascending
      * address range.
@@ -127,18 +130,21 @@ public DebugTypeKind typeKind() {
 
     @Override
     public void addDebugInfo(DebugInfoBase debugInfoBase, DebugTypeInfo debugTypeInfo, DebugContext debugContext) {
-        assert TypeEntry.canonicalize(debugTypeInfo.typeName()).equals(typeName);
+        assert debugTypeInfo.typeName().equals(typeName);
         DebugInstanceTypeInfo debugInstanceTypeInfo = (DebugInstanceTypeInfo) debugTypeInfo;
         /* Add details of super and interface classes */
-        String superName = debugInstanceTypeInfo.superName();
-        if (superName != null) {
-            superName = TypeEntry.canonicalize(superName);
+        ResolvedJavaType superType = debugInstanceTypeInfo.superClass();
+        String superName;
+        if (superType != null) {
+            superName = superType.toJavaName();
+        } else {
+            superName = "";
         }
         debugContext.log("typename %s adding super %s\n", typeName, superName);
-        if (superName != null) {
-            this.superClass = debugInfoBase.lookupClassEntry(superName);
+        if (superType != null) {
+            this.superClass = debugInfoBase.lookupClassEntry(superType);
         }
-        debugInstanceTypeInfo.interfaces().forEach(interfaceName -> processInterface(interfaceName, debugInfoBase, debugContext));
+        debugInstanceTypeInfo.interfaces().forEach(interfaceType -> processInterface(interfaceType, debugInfoBase, debugContext));
         /* Add details of fields and field types */
         debugInstanceTypeInfo.fieldInfoProvider().forEach(debugFieldInfo -> this.processField(debugFieldInfo, debugInfoBase, debugContext));
         /* Add details of methods and method types */
@@ -177,11 +183,10 @@ public void indexSubRange(Range subrange) {
         }
     }
 
-    private void indexMethodEntry(MethodEntry methodEntry) {
-        String methodName = methodEntry.getSymbolName();
-        assert methodsIndex.get(methodName) == null : methodName;
+    private void indexMethodEntry(MethodEntry methodEntry, ResolvedJavaMethod idMethod) {
+        assert methodsIndex.get(idMethod) == null : methodEntry.getSymbolName();
         methods.add(methodEntry);
-        methodsIndex.put(methodName, methodEntry);
+        methodsIndex.put(idMethod, methodEntry);
     }
 
     private void indexLocalFileEntry(FileEntry localFileEntry) {
@@ -273,9 +278,10 @@ public String getCachePath() {
         return "";
     }
 
-    private void processInterface(String interfaceName, DebugInfoBase debugInfoBase, DebugContext debugContext) {
+    private void processInterface(ResolvedJavaType interfaceType, DebugInfoBase debugInfoBase, DebugContext debugContext) {
+        String interfaceName = interfaceType.toJavaName();
         debugContext.log("typename %s adding interface %s\n", typeName, interfaceName);
-        ClassEntry entry = debugInfoBase.lookupClassEntry(TypeEntry.canonicalize(interfaceName));
+        ClassEntry entry = debugInfoBase.lookupClassEntry(interfaceType);
         assert entry instanceof InterfaceClassEntry;
         InterfaceClassEntry interfaceClassEntry = (InterfaceClassEntry) entry;
         interfaces.add(interfaceClassEntry);
@@ -284,26 +290,27 @@ private void processInterface(String interfaceName, DebugInfoBase debugInfoBase,
 
     protected MethodEntry processMethod(DebugMethodInfo debugMethodInfo, DebugInfoBase debugInfoBase, DebugContext debugContext) {
         String methodName = debugMethodInfo.name();
-        String resultTypeName = TypeEntry.canonicalize(debugMethodInfo.valueType());
+        ResolvedJavaType resultType = debugMethodInfo.valueType();
+        String resultTypeName = resultType.toJavaName();
         int modifiers = debugMethodInfo.modifiers();
         DebugLocalInfo[] paramInfos = debugMethodInfo.getParamInfo();
         DebugLocalInfo thisParam = debugMethodInfo.getThisParamInfo();
         int paramCount = paramInfos.length;
         debugContext.log("typename %s adding %s method %s %s(%s)\n",
                         typeName, memberModifiers(modifiers), resultTypeName, methodName, formatParams(paramInfos));
-        TypeEntry resultType = debugInfoBase.lookupTypeEntry(resultTypeName);
+        TypeEntry resultTypeEntry = debugInfoBase.lookupTypeEntry(resultType);
         TypeEntry[] typeEntries = new TypeEntry[paramCount];
         for (int i = 0; i < paramCount; i++) {
-            typeEntries[i] = debugInfoBase.lookupTypeEntry(TypeEntry.canonicalize(paramInfos[i].typeName()));
+            typeEntries[i] = debugInfoBase.lookupTypeEntry(paramInfos[i].valueType());
         }
         /*
          * n.b. the method file may differ from the owning class file when the method is a
          * substitution
          */
         FileEntry methodFileEntry = debugInfoBase.ensureFileEntry(debugMethodInfo);
         MethodEntry methodEntry = new MethodEntry(debugInfoBase, debugMethodInfo, methodFileEntry, methodName,
-                        this, resultType, typeEntries, paramInfos, thisParam);
-        indexMethodEntry(methodEntry);
+                        this, resultTypeEntry, typeEntries, paramInfos, thisParam);
+        indexMethodEntry(methodEntry, debugMethodInfo.idMethod());
 
         return methodEntry;
     }
@@ -345,7 +352,7 @@ public ClassEntry getSuperClass() {
 
     public MethodEntry ensureMethodEntryForDebugRangeInfo(DebugRangeInfo debugRangeInfo, DebugInfoBase debugInfoBase, DebugContext debugContext) {
 
-        MethodEntry methodEntry = methodsIndex.get(debugRangeInfo.symbolNameForMethod());
+        MethodEntry methodEntry = methodsIndex.get(debugRangeInfo.idMethod());
         if (methodEntry == null) {
             methodEntry = processMethod(debugRangeInfo, debugInfoBase, debugContext);
         } else {

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/DebugInfoBase.java

@@ -72,7 +72,7 @@ public abstract class DebugInfoBase {
      *
      * An alternative traversal option is
      *
-     * 1) by top level class (String id)
+     * 1) by top level class (unique ResolvedJavaType id)
      *
      * 2) by top level compiled method (primary Range) within a class ordered by ascending address
      *
@@ -83,6 +83,18 @@ public abstract class DebugInfoBase {
      * means we can treat each class as a compilation unit, allowing data common to all methods of
      * the class to be shared.
      *
+     * Just as an aside, for full disclosure, this is not strictly the full story. Sometimes a class
+     * can include speculatively optimized, compiled methods plus deopt fallback compiled variants
+     * of those same methods. In such cases the normal and/or speculatively compiled methods occupy
+     * one contiguous range and deopt methods occupy a separate higher range. The current
+     * compilation strategy ensures that the union across all classes of the normal/speculative
+     * ranges and the union across all classes of the deopt ranges lie in two distinct intervals
+     * where the highest address in the first union is strictly less than the lowest address in the
+     * second union. The implication is twofold. An address order traversal requires generating
+     * details for classes, methods and non-deopt primary ranges before generating details for the
+     * deopt primary ranges. The former details need to be generated in a distinct CU from deopt
+     * method details.
+     *
      * A third option appears to be to traverse via files, then top level class within file etc.
      * Unfortunately, files cannot be treated as the compilation unit. A file F may contain multiple
      * classes, say C1 and C2. There is no guarantee that methods for some other class C' in file F'
@@ -96,17 +108,22 @@ public abstract class DebugInfoBase {
      */
     private List<TypeEntry> types = new ArrayList<>();
     /**
-     * index of already seen classes.
+     * Index of already seen classes keyed by the unique, associated, identifying ResolvedJavaType
+     * or, in the single special case of the TypeEntry for the Java header structure, by key null.
      */
-    private Map<String, TypeEntry> typesIndex = new HashMap<>();
+    private Map<ResolvedJavaType, TypeEntry> typesIndex = new HashMap<>();
     /**
      * List of class entries detailing class info for primary ranges.
      */
     private List<ClassEntry> primaryClasses = new ArrayList<>();
     /**
-     * index of already seen classes.
+     * Index of already seen classes.
      */
     private Map<ResolvedJavaType, ClassEntry> primaryClassesIndex = new HashMap<>();
+    /**
+     * Handle on class entry for java.lang.Object.
+     */
+    private ClassEntry objectClass;
     /**
      * Index of files which contain primary or secondary ranges.
      */
@@ -213,7 +230,8 @@ public void installDebugInfo(DebugInfoProvider debugInfoProvider) {
 
         /* Create all the types. */
         debugInfoProvider.typeInfoProvider().forEach(debugTypeInfo -> debugTypeInfo.debugContext((debugContext) -> {
-            String typeName = TypeEntry.canonicalize(debugTypeInfo.typeName());
+            ResolvedJavaType idType = debugTypeInfo.idType();
+            String typeName = debugTypeInfo.typeName();
             typeName = stringTable.uniqueDebugString(typeName);
             DebugTypeKind typeKind = debugTypeInfo.typeKind();
             int byteSize = debugTypeInfo.size();
@@ -222,16 +240,17 @@ public void installDebugInfo(DebugInfoProvider debugInfoProvider) {
             String fileName = debugTypeInfo.fileName();
             Path filePath = debugTypeInfo.filePath();
             Path cachePath = debugTypeInfo.cachePath();
-            addTypeEntry(typeName, fileName, filePath, cachePath, byteSize, typeKind);
+            addTypeEntry(idType, typeName, fileName, filePath, cachePath, byteSize, typeKind);
         }));
 
         /* Now we can cross reference static and instance field details. */
         debugInfoProvider.typeInfoProvider().forEach(debugTypeInfo -> debugTypeInfo.debugContext((debugContext) -> {
-            String typeName = TypeEntry.canonicalize(debugTypeInfo.typeName());
+            ResolvedJavaType idType = debugTypeInfo.idType();
+            String typeName = debugTypeInfo.typeName();
             DebugTypeKind typeKind = debugTypeInfo.typeKind();
 
             debugContext.log(DebugContext.INFO_LEVEL, "Process %s type %s ", typeKind.toString(), typeName);
-            TypeEntry typeEntry = lookupTypeEntry(typeName);
+            TypeEntry typeEntry = lookupTypeEntry(idType);
             typeEntry.addDebugInfo(this, debugTypeInfo, debugContext);
         }));
 
@@ -309,12 +328,15 @@ private TypeEntry createTypeEntry(String typeName, String fileName, Path filePat
         return typeEntry;
     }
 
-    private TypeEntry addTypeEntry(String typeName, String fileName, Path filePath, Path cachePath, int size, DebugTypeKind typeKind) {
-        TypeEntry typeEntry = typesIndex.get(typeName);
+    private TypeEntry addTypeEntry(ResolvedJavaType idType, String typeName, String fileName, Path filePath, Path cachePath, int size, DebugTypeKind typeKind) {
+        TypeEntry typeEntry = typesIndex.get(idType);
         if (typeEntry == null) {
             typeEntry = createTypeEntry(typeName, fileName, filePath, cachePath, size, typeKind);
             types.add(typeEntry);
-            typesIndex.put(typeName, typeEntry);
+            typesIndex.put(idType, typeEntry);
+            if (typeName.equals("java.lang.Object")) {
+                objectClass = (ClassEntry) typeEntry;
+            }
         } else {
             if (!(typeEntry.isClass())) {
                 assert ((ClassEntry) typeEntry).getFileName().equals(fileName);
@@ -323,22 +345,26 @@ private TypeEntry addTypeEntry(String typeName, String fileName, Path filePath,
         return typeEntry;
     }
 
-    public TypeEntry lookupTypeEntry(String typeName) {
-        TypeEntry typeEntry = typesIndex.get(typeName);
+    public TypeEntry lookupTypeEntry(ResolvedJavaType type) {
+        TypeEntry typeEntry = typesIndex.get(type);
         if (typeEntry == null) {
-            throw new RuntimeException("type entry not found " + typeName);
+            throw new RuntimeException("type entry not found " + type.getName());
         }
         return typeEntry;
     }
 
-    ClassEntry lookupClassEntry(String typeName) {
-        TypeEntry typeEntry = typesIndex.get(typeName);
+    ClassEntry lookupClassEntry(ResolvedJavaType type) {
+        TypeEntry typeEntry = typesIndex.get(type);
         if (typeEntry == null || !(typeEntry.isClass())) {
-            throw new RuntimeException("class entry not found " + typeName);
+            throw new RuntimeException("class entry not found " + type.getName());
         }
         return (ClassEntry) typeEntry;
     }
 
+    public ClassEntry lookupObjectClass() {
+        return objectClass;
+    }
+
     /**
      * Recursively creates subranges based on DebugLocationInfo including, and appropriately
      * linking, nested inline subranges.
@@ -360,7 +386,7 @@ private Range addSubrange(DebugLocationInfo locationInfo, Range primaryRange, Cl
         DebugLocationInfo callerLocationInfo = locationInfo.getCaller();
         boolean isTopLevel = callerLocationInfo == null;
         assert (!isTopLevel || (locationInfo.name().equals(primaryRange.getMethodName()) &&
-                        TypeEntry.canonicalize(locationInfo.ownerType().toJavaName()).equals(primaryRange.getClassName())));
+                        locationInfo.ownerType().toJavaName().equals(primaryRange.getClassName())));
         Range caller = (isTopLevel ? primaryRange : subRangeIndex.get(callerLocationInfo));
         // the frame tree is walked topdown so inline ranges should always have a caller range
         assert caller != null;
@@ -396,13 +422,13 @@ private ClassEntry ensureClassEntry(ResolvedJavaType type) {
         /* See if we already have an entry. */
         ClassEntry classEntry = primaryClassesIndex.get(type);
         if (classEntry == null) {
-            TypeEntry typeEntry = typesIndex.get(TypeEntry.canonicalize(type.toJavaName()));
+            TypeEntry typeEntry = typesIndex.get(type);
             assert (typeEntry != null && typeEntry.isClass());
             classEntry = (ClassEntry) typeEntry;
             primaryClasses.add(classEntry);
             primaryClassesIndex.put(type, classEntry);
         }
-        assert (classEntry.getTypeName().equals(TypeEntry.canonicalize(type.toJavaName())));
+        assert (classEntry.getTypeName().equals(type.toJavaName()));
         return classEntry;
     }
 

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/EnumClassEntry.java

@@ -26,7 +26,7 @@
 
 package com.oracle.objectfile.debugentry;
 
-import com.oracle.objectfile.debuginfo.DebugInfoProvider;
+import com.oracle.objectfile.debuginfo.DebugInfoProvider.DebugTypeInfo;
 import com.oracle.objectfile.debuginfo.DebugInfoProvider.DebugEnumTypeInfo;
 import com.oracle.objectfile.debuginfo.DebugInfoProvider.DebugTypeInfo.DebugTypeKind;
 import org.graalvm.compiler.debug.DebugContext;
@@ -42,7 +42,7 @@ public DebugTypeKind typeKind() {
     }
 
     @Override
-    public void addDebugInfo(DebugInfoBase debugInfoBase, DebugInfoProvider.DebugTypeInfo debugTypeInfo, DebugContext debugContext) {
+    public void addDebugInfo(DebugInfoBase debugInfoBase, DebugTypeInfo debugTypeInfo, DebugContext debugContext) {
         assert debugTypeInfo instanceof DebugEnumTypeInfo;
         super.addDebugInfo(debugInfoBase, debugTypeInfo, debugContext);
     }

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/HeaderTypeEntry.java

@@ -45,7 +45,7 @@ public DebugTypeKind typeKind() {
 
     @Override
     public void addDebugInfo(DebugInfoBase debugInfoBase, DebugTypeInfo debugTypeInfo, DebugContext debugContext) {
-        assert TypeEntry.canonicalize(debugTypeInfo.typeName()).equals(typeName);
+        assert debugTypeInfo.typeName().equals(typeName);
         DebugHeaderTypeInfo debugHeaderTypeInfo = (DebugHeaderTypeInfo) debugTypeInfo;
         debugHeaderTypeInfo.fieldInfoProvider().forEach(debugFieldInfo -> this.processField(debugFieldInfo, debugInfoBase, debugContext));
     }

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/MethodEntry.java

@@ -36,6 +36,7 @@
 import com.oracle.objectfile.debuginfo.DebugInfoProvider.DebugMethodInfo;
 
 import jdk.vm.ci.meta.JavaKind;
+import jdk.vm.ci.meta.ResolvedJavaType;
 
 public class MethodEntry extends MemberEntry {
     private final TypeEntry[] paramTypes;
@@ -285,6 +286,11 @@ private static class DebugLocalInfoWrapper implements DebugLocalInfo {
             this.line = value.line();
         }
 
+        @Override
+        public ResolvedJavaType valueType() {
+            return value.valueType();
+        }
+
         @Override
         public String name() {
             return value.name();