Skip to content

Commit 40b5e8d

Browse files
committed
Enhance MethodEntries creation and verification
Following Andrew Dinn's recommendation in #3304 (comment), this patch creates the necessary `MethodEntry`s for each `ClassEntry` using the methodInfoProvider, It then sorts the methods list to ensure faster lookups in the phase of linking `Range`s to `MethodEntry`s. It also adds assertions to ensure that the methods list is sorted and that there are no duplicate entries in it. Furthermore, it adds assertions to ensure that each Range can be mapped to a MethodEntry created by info provided through the methodInfoProvider.
1 parent acfec7a commit 40b5e8d

File tree

3 files changed

+58
-21
lines changed

3 files changed

+58
-21
lines changed

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

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
33
* Copyright (c) 2020, 2020, Red Hat Inc. All rights reserved.
44
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
55
*
@@ -135,7 +135,9 @@ public void addDebugInfo(DebugInfoBase debugInfoBase, DebugTypeInfo debugTypeInf
135135
/* Add details of fields and field types */
136136
debugInstanceTypeInfo.fieldInfoProvider().forEach(debugFieldInfo -> this.processField(debugFieldInfo, debugInfoBase, debugContext));
137137
/* Add details of methods and method types */
138-
debugInstanceTypeInfo.methodInfoProvider().forEach(methodFieldInfo -> this.ensureMethodEntry(methodFieldInfo, debugInfoBase, debugContext));
138+
debugInstanceTypeInfo.methodInfoProvider().forEach(methodFieldInfo -> this.processMethod(methodFieldInfo, debugInfoBase, debugContext));
139+
/* Sort methods to improve lookup speed */
140+
this.methods.sort(MethodEntry::compareTo);
139141
}
140142

141143
public void indexPrimary(Range primary, List<DebugFrameSizeChange> frameSizeInfos, int frameSize) {
@@ -268,7 +270,7 @@ private void processInterface(String interfaceName, DebugInfoBase debugInfoBase,
268270
interfaceClassEntry.addImplementor(this, debugContext);
269271
}
270272

271-
protected MethodEntry processMethod(DebugMethodInfo debugMethodInfo, DebugInfoBase debugInfoBase, DebugContext debugContext) {
273+
protected void processMethod(DebugMethodInfo debugMethodInfo, DebugInfoBase debugInfoBase, DebugContext debugContext) {
272274
String methodName = debugInfoBase.uniqueDebugString(debugMethodInfo.name());
273275
String resultTypeName = TypeEntry.canonicalize(debugMethodInfo.valueType());
274276
int modifiers = debugMethodInfo.modifiers();
@@ -295,7 +297,8 @@ protected MethodEntry processMethod(DebugMethodInfo debugMethodInfo, DebugInfoBa
295297
* substitution
296298
*/
297299
FileEntry methodFileEntry = debugInfoBase.ensureFileEntry(fileName, filePath, cachePath);
298-
return new MethodEntry(methodFileEntry, methodName, this, resultType, paramTypeArray, paramNameArray, modifiers, debugMethodInfo.isDeoptTarget());
300+
final MethodEntry methodEntry = new MethodEntry(methodFileEntry, methodName, this, resultType, paramTypeArray, paramNameArray, modifiers, debugMethodInfo.isDeoptTarget());
301+
methods.add(methodEntry);
299302
}
300303

301304
@Override
@@ -345,29 +348,40 @@ public Range makePrimaryRange(String symbolName, StringTable stringTable, Method
345348
return new Range(symbolName, stringTable, method, fileEntryToUse, lo, hi, primaryLine);
346349
}
347350

348-
public MethodEntry ensureMethodEntry(DebugMethodInfo debugMethodInfo, DebugInfoBase debugInfoBase, DebugContext debugContext) {
351+
public MethodEntry getMethodEntry(DebugMethodInfo debugMethodInfo, DebugInfoBase debugInfoBase) {
352+
assert listIsSorted(methods);
349353
String methodName = debugInfoBase.uniqueDebugString(debugMethodInfo.name());
350354
String paramSignature = debugMethodInfo.paramSignature();
351355
String returnTypeName = debugMethodInfo.valueType();
352356
final ListIterator<MethodEntry> methodIterator = methods.listIterator();
353-
while (true) {
354-
if (!methodIterator.hasNext()) {
355-
return addMethodEntry(methodIterator, debugMethodInfo, debugInfoBase, debugContext);
356-
}
357+
while (methodIterator.hasNext()) {
357358
final MethodEntry methodEntry = methodIterator.next();
358359
final int comparisonResult = methodEntry.compareTo(methodName, paramSignature, returnTypeName);
359360
if (comparisonResult == 0) {
360361
return methodEntry;
361-
} else if (comparisonResult > 0) {
362-
methodIterator.previous();
363-
return addMethodEntry(methodIterator, debugMethodInfo, debugInfoBase, debugContext);
364362
}
363+
assert comparisonResult < 0 : "Method " + returnTypeName + " " + methodName + "(" + paramSignature + ") not found in CLassEntry of " + getTypeName();
365364
}
365+
assert false : "Method " + returnTypeName + " " + methodName + "(" + paramSignature + ") not found in CLassEntry of " + getTypeName();
366+
return null;
366367
}
367368

368-
private MethodEntry addMethodEntry(ListIterator<MethodEntry> methodIterator, DebugMethodInfo debugMethodInfo, DebugInfoBase debugInfoBase, DebugContext debugContext) {
369-
MethodEntry newMethodEntry = processMethod(debugMethodInfo, debugInfoBase, debugContext);
370-
methodIterator.add(newMethodEntry);
371-
return newMethodEntry;
369+
private boolean listIsSorted(List<MethodEntry> list) {
370+
ListIterator<MethodEntry> iterator = list.listIterator();
371+
if (!iterator.hasNext()) {
372+
return true;
373+
}
374+
MethodEntry current = iterator.next();
375+
MethodEntry previous;
376+
while (iterator.hasNext()) {
377+
previous = current;
378+
current = iterator.next();
379+
final int result = previous.compareTo(current);
380+
assert result != 0 : "Method " + current.valueType.getTypeName() + " " + current.methodName() + "(" + current.paramTypes.toString() + ") found twice in CLassEntry of " + getTypeName();
381+
if (result >= 0) {
382+
return false;
383+
}
384+
}
385+
return true;
372386
}
373387
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
33
* Copyright (c) 2020, 2020, Red Hat Inc. All rights reserved.
44
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
55
*
@@ -245,7 +245,7 @@ public void installDebugInfo(DebugInfoProvider debugInfoProvider) {
245245

246246
/* Search for a method defining this primary range. */
247247
ClassEntry classEntry = ensureClassEntry(className);
248-
MethodEntry methodEntry = classEntry.ensureMethodEntry(debugCodeInfo, this, debugContext);
248+
MethodEntry methodEntry = classEntry.getMethodEntry(debugCodeInfo, this);
249249
Range primaryRange = classEntry.makePrimaryRange(symbolName, stringTable, methodEntry, lo, hi, primaryLine);
250250
debugContext.log(DebugContext.INFO_LEVEL, "PrimaryRange %s.%s %s %s:%d [0x%x, 0x%x]", className, methodName, filePath, fileName, primaryLine, lo, hi);
251251
classEntry.indexPrimary(primaryRange, debugCodeInfo.getFrameSizeChanges(), debugCodeInfo.getFrameSize());
@@ -263,7 +263,7 @@ public void installDebugInfo(DebugInfoProvider debugInfoProvider) {
263263
* symbol for them and don't see a break in the address range.
264264
*/
265265
ClassEntry subClassEntry = ensureClassEntry(classNameAtLine);
266-
MethodEntry subMethodEntry = subClassEntry.ensureMethodEntry(debugLineInfo, this, debugContext);
266+
MethodEntry subMethodEntry = subClassEntry.getMethodEntry(debugLineInfo, this);
267267
Range subRange = new Range(symbolNameAtLine, stringTable, subMethodEntry, loAtLine, hiAtLine, line, primaryRange);
268268
classEntry.indexSubRange(subRange);
269269
try (DebugContext.Scope s = debugContext.scope("Subranges")) {

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

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
33
* Copyright (c) 2020, 2020, Red Hat Inc. All rights reserved.
44
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
55
*
@@ -26,7 +26,7 @@
2626

2727
package com.oracle.objectfile.debugentry;
2828

29-
public class MethodEntry extends MemberEntry {
29+
public class MethodEntry extends MemberEntry implements Comparable {
3030
final TypeEntry[] paramTypes;
3131
final String[] paramNames;
3232
final boolean isDeoptTarget;
@@ -100,4 +100,27 @@ public int compareTo(String methodName, String paramSignature, String returnType
100100
public boolean isDeoptTarget() {
101101
return isDeoptTarget;
102102
}
103+
104+
@Override
105+
public int compareTo(Object o) {
106+
assert o != null;
107+
assert o instanceof MethodEntry;
108+
MethodEntry other = (MethodEntry) o;
109+
int result = methodName().compareTo(other.methodName());
110+
if (result == 0) {
111+
result = valueType.getTypeName().compareTo(other.valueType.getTypeName());
112+
}
113+
if (result == 0) {
114+
result = getParamCount() - other.getParamCount();
115+
}
116+
if (result == 0) {
117+
for (int i = 0; i < getParamCount(); i++) {
118+
result = getParamTypeName(i).compareTo(other.getParamTypeName(i));
119+
if (result != 0) {
120+
break;
121+
}
122+
}
123+
}
124+
return result;
125+
}
103126
}

0 commit comments

Comments
 (0)