Debug info: Remove methods from class entry

[zakkak]

The current implementation goes through all the declared methods of reachable classes and adds them to a list called methods in the corresponding ClassEntry instance. This list is only used in

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

Lines 343 to 353 in b609631

	/*
	* Search for a matching method to supply the file entry or failing that use the one
	* from this class.
	*/
	for (MethodEntry methodEntry : methods) {
	if (methodEntry.match(methodName, paramSignature, returnTypeName)) {
	/* maybe the method's file entry */
	fileEntryToUse = methodEntry.getFileEntry();
	break;
	}
	}

which I think is redundant given that the FileEntry created in processMethod should be essentially the same with the corresponding FileEntry created in installDebugInfo, so if the latter returns null I would expect the former to do the same.

[adinn]

@zakkak There are cases where a range is not associated with a source file. That happens when the CompiledMethod that provides the info used to construct the range does not have an associated file. So, that's why we check the input FileEntry for null. At that point the question is what to do to try to find an alternative FileEntry for the method source.

The obvious thing is to use the FileEntry of the ClassEntry whose name qualifies the CompiledMethod/Range name. However, that's not necessarily correct. When we have a substitution the name used for the Range retains the class qualifier of the original method (e.g. java.lang/Class) even though it may be declared on a target class (e.g. DynamicHuub). With a wholesale class replacement that does not matter because the FileEntry for the original is replaced by the by the FileEntry for the target. However, it matters when individual methods are substituted. In that case the file associated with class org.Foo might be Foo.java but the method may belong to class target_Foo in file FooSubstitutions.java.

So, the loop looks through all the methods that were reported as belonging to that class to see if there is one with the same signature. If so then the FileEntry from the method is used. If not then the FileEntry from the class is used as a last resort.

Now, it may well be that whenever a Range -- or rather the CompiledMethod from which it is derived -- has no associated file then any matching source method will also be guaranteed to have no associated file. If so then execution of this loop is redundant. I am not sure that is the case though.

@olpaw perhaps you can resolve that doubt?

[adinn]

Also, as a more general caution: it may be warranted to get rid of all the code you have removed but I am not sure it is yet clear that it is a good idea.

The issue here is that the HostedUniverse provides one view of the code base and the CompiledMethod list provides another. These are the sources, respectively, for the Class/MethodEntry records and the Range records. Those two views ought at least to be consistent. It may even be that the MethodEntry info is redundant because all the details currently derived from HostedMethods are also provided by the CompiledMethods. However I am not clear that either of those two propositions can be assumed to be the case.

Specifically, the code and data you are trying to remove is the only ting at present that enables these two views to be reconciled against each other. It merely attempts to ensure that gaps in one view (Ranges which omit source files) are filled, if possible, with info provided by the other (source of the corresponding MethodEntry). I planned originally to go further and attempt to validate consistency/equivalence by comparing the two views to ensure every Range has a MethodEntry and vice versa but I didn't do that before pushing this change. So, before making this change I wonder if we ought first to add some extra asserts before we lose our chance to do so.

[zakkak]

I planned originally to go further and attempt to validate consistency/equivalence by comparing the two views to ensure every Range has a MethodEntry

Well, that fits well with what I am currently trying to do for #2880, i.e. link each range with a MethodEntry :)
This is still WIP though and might blow up, I'll keep you posted.

[zakkak]

Superseded by #3304