Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GR-47106] Return to separate CUs per class to improve gdb startup time #6937

Merged
merged 7 commits into from
Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,37 @@ public class ClassEntry extends StructureTypeEntry {
*/
private final EconomicMap<Range, CompiledMethodEntry> compiledMethodIndex = EconomicMap.create();

/**
* A list of all files referenced from info associated with this class, including info detailing
* inline method ranges.
*/
private ArrayList<FileEntry> files;
/**
* A list of all directories referenced from info associated with this class, including info
* detailing inline method ranges.
*/
private ArrayList<DirEntry> dirs;
/**
* An index identifying the file table position of every file referenced from info associated
* with this class, including info detailing inline method ranges.
*/
private EconomicMap<FileEntry, Integer> fileIndex;
/**
* An index identifying the dir table position of every directory referenced from info
* associated with this class, including info detailing inline method ranges.
*/
private EconomicMap<DirEntry, Integer> dirIndex;

public ClassEntry(String className, FileEntry fileEntry, int size) {
super(className, size);
this.fileEntry = fileEntry;
this.loader = null;
// file and dir lists/indexes are populated after all DebugInfo API input has
// been received and are only created on demand
files = null;
dirs = null;
this.fileIndex = null;
this.dirIndex = null;
}

@Override
Expand Down Expand Up @@ -179,33 +206,48 @@ public FileEntry getFileEntry() {
}

public int getFileIdx() {
return fileEntry.getIdx();
return getFileIdx(this.getFileEntry());
}

public int getFileIdx(FileEntry file) {
if (file == null || fileIndex == null) {
return 0;
}
return fileIndex.get(file);
}

public DirEntry getDirEntry(FileEntry file) {
if (file == null) {
return null;
}
return file.getDirEntry();
}

public int getDirIdx(FileEntry file) {
DirEntry dirEntry = getDirEntry(file);
return getDirIdx(dirEntry);
}

public int getDirIdx(DirEntry dir) {
if (dir == null || dir.getPathString().isEmpty() || dirIndex == null) {
return 0;
}
return dirIndex.get(dir);
}

public String getLoaderId() {
return (loader != null ? loader.getLoaderId() : "");
}

/**
* Retrieve a stream of all compiled method entries for this class, including both normal and
* deopt fallback compiled methods.
* Retrieve a stream of all compiled method entries for this class.
*
* @return a stream of all compiled method entries for this class.
*/
public Stream<CompiledMethodEntry> compiledEntries() {
return compiledEntries.stream();
}

/**
* Retrieve a stream of all normal compiled method entries for this class, excluding deopt
* fallback compiled methods.
*
* @return a stream of all normal compiled method entries for this class.
*/
public Stream<CompiledMethodEntry> normalCompiledEntries() {
return compiledEntries();
}

protected void processInterface(ResolvedJavaType interfaceType, DebugInfoBase debugInfoBase, DebugContext debugContext) {
String interfaceName = interfaceType.toJavaName();
debugContext.log("typename %s adding interface %s%n", typeName, interfaceName);
Expand Down Expand Up @@ -267,8 +309,17 @@ private static String formatParams(DebugLocalInfo[] paramInfo) {
return builder.toString();
}

public int compiledEntryCount() {
return compiledEntries.size();
}

public boolean hasCompiledEntries() {
return compiledEntries.size() != 0;
return compiledEntryCount() != 0;
}

public int compiledEntriesBase() {
assert hasCompiledEntries();
return compiledEntries.get(0).getPrimary().getLo();
}

public ClassEntry getSuperClass() {
Expand Down Expand Up @@ -317,4 +368,90 @@ public int hipc() {
assert hasCompiledEntries();
return compiledEntries.get(compiledEntries.size() - 1).getPrimary().getHi();
}

/**
* Add a file to the list of files referenced from info associated with this class.
*
* @param file The file to be added.
*/
public void includeFile(FileEntry file) {
if (files == null) {
files = new ArrayList<>();
}
assert !files.contains(file) : "caller should ensure file is only included once";
assert fileIndex == null : "cannot include files after index has been created";
files.add(file);
}

/**
* Add a directory to the list of firectories referenced from info associated with this class.
*
* @param dirEntry The directory to be added.
*/
public void includeDir(DirEntry dirEntry) {
if (dirs == null) {
adinn marked this conversation as resolved.
Show resolved Hide resolved
dirs = new ArrayList<>();
}
assert !dirs.contains(dirEntry) : "caller should ensure dir is only included once";
assert dirIndex == null : "cannot include dirs after index has been created";
dirs.add(dirEntry);
}

/**
* Populate the file and directory indexes that track positions in the file and dir tables for
* this class's line info section.
*/
public void buildFileAndDirIndexes() {
// this is a one-off operation
assert fileIndex == null && dirIndex == null : "file and indexes can only be generated once";
if (files == null) {
assert dirs == null : "should not have included any dirs if we have no files";
return;
}
int idx = 1;
fileIndex = EconomicMap.create(files.size());
for (FileEntry file : files) {
fileIndex.put(file, idx++);
}
if (dirs == null) {
return;
}
dirIndex = EconomicMap.create(dirs.size());
idx = 1;
for (DirEntry dir : dirs) {
if (!dir.getPathString().isEmpty()) {
dirIndex.put(dir, idx++);
} else {
assert idx == 1;
}
}
}

/**
* Retrieve a stream of all files referenced from debug info for this class in line info file
* table order, starting with the file at index 1.
*
* @return a stream of all referenced files
*/
public Stream<FileEntry> fileStream() {
if (files != null) {
return files.stream();
} else {
return Stream.empty();
}
}

/**
* Retrieve a stream of all directories referenced from debug info for this class in line info
* directory table order, starting with the directory at index 1.
*
* @return a stream of all referenced directories
*/
public Stream<DirEntry> dirStream() {
if (dirs != null) {
return dirs.stream();
} else {
return Stream.empty();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import java.util.Map;

import org.graalvm.collections.EconomicMap;
import org.graalvm.collections.EconomicSet;
import org.graalvm.compiler.debug.DebugContext;

import com.oracle.objectfile.debugentry.range.PrimaryRange;
Expand Down Expand Up @@ -77,31 +78,14 @@
* 3) by inlined method (sub range) within top level method, also ordered by ascending address
*
* Since clients may need to generate records for classes with no compiled methods, the second
* traversal order is often employed. In rare cases clients need to sort the class list by address
* before traversal to ensure the generated debug records are also sorted by address.
* traversal order is often employed.
*
* n.b. the above strategy relies on the details that methods of a given class always appear in a
* single continuous address range with no intervening code from other methods or data values. This
* means we can treat each class as a Compilation Unit, allowing data common to all methods of the
* class to be referenced using CU-local offsets.
*
* 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 a 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'
* will not be compiled into the address space interleaved between methods of C1 and C2. That is a
* shame because generating debug info records one file at a time would allow more sharing e.g.
* enabling all classes in a file to share a single copy of the file and dir tables.
* n.b. methods of a given class do not always appear in a single continuous address range. The
* compiler choose to interleave intervening code from other classes or data values in order to get
* better cache locality. It may also choose to generate deoptimized variants of methods in a
* separate range from normal, optimized compiled code. This out of (code addess) order sorting may
* make it difficult to use a class by class traversal to generate debug info in separate per-class
* units.
*/
public abstract class DebugInfoBase {
protected ByteOrder byteOrder;
Expand Down Expand Up @@ -364,6 +348,10 @@ public void installDebugInfo(DebugInfoProvider debugInfoProvider) {
debugContext.log(DebugContext.INFO_LEVEL, "Data: address 0x%x size 0x%x type %s partition %s provenance %s ", address, size, typeName, partitionName, provenance);
}
}));
// populate a file and dir list and associated index for each class entry
getInstanceClasses().forEach(classEntry -> {
collectFilesAndDirs(classEntry);
});
}

private TypeEntry createTypeEntry(String typeName, String fileName, Path filePath, int size, DebugTypeKind typeKind) {
Expand Down Expand Up @@ -581,7 +569,7 @@ private FileEntry addFileEntry(String fileName, Path filePath) {
/* Ensure file and cachepath are added to the debug_str section. */
uniqueDebugString(fileName);
uniqueDebugString(cachePath);
fileEntry = new FileEntry(fileName, dirEntry, files.size() + 1);
fileEntry = new FileEntry(fileName, dirEntry);
files.add(fileEntry);
/* Index the file entry by file path. */
filesIndex.put(fileAsPath, fileEntry);
Expand Down Expand Up @@ -619,7 +607,7 @@ private DirEntry ensureDirEntry(Path filePath) {
if (dirEntry == null) {
/* Ensure dir path is entered into the debug_str section. */
uniqueDebugString(filePath.toString());
dirEntry = new DirEntry(filePath, dirs.size());
dirEntry = new DirEntry(filePath);
dirsIndex.put(filePath, dirEntry);
dirs.add(dirEntry);
}
Expand Down Expand Up @@ -691,7 +679,6 @@ public String uniqueDebugString(String string) {
* Indirects this call to the string table.
*
* @param string the string whose index is required.
*
* @return the offset of the string in the .debug_str section.
*/
public int debugStringIndex(String string) {
Expand Down Expand Up @@ -741,4 +728,51 @@ public boolean isHubClassEntry(ClassEntry classEntry) {
public ClassEntry getHubClassEntry() {
return hubClassEntry;
}

private static void collectFilesAndDirs(ClassEntry classEntry) {
// track files and dirs we have already seen so that we only add them once
EconomicSet<FileEntry> visitedFiles = EconomicSet.create();
EconomicSet<DirEntry> visitedDirs = EconomicSet.create();
// add the class's file and dir
checkInclude(classEntry, classEntry.getFileEntry(), visitedFiles, visitedDirs);
// add files for fields (may differ from class file if we have a substitution)
for (FieldEntry fieldEntry : classEntry.fields) {
checkInclude(classEntry, fieldEntry.getFileEntry(), visitedFiles, visitedDirs);
}
// add files for declared methods (may differ from class file if we have a substitution)
for (MethodEntry methodEntry : classEntry.getMethods()) {
checkInclude(classEntry, methodEntry.getFileEntry(), visitedFiles, visitedDirs);
}
// add files for top level compiled and inline methods
classEntry.compiledEntries().forEachOrdered(compiledMethodEntry -> {
checkInclude(classEntry, compiledMethodEntry.getPrimary().getFileEntry(), visitedFiles, visitedDirs);
// we need files for leaf ranges and for inline caller ranges
//
// add leaf range files first because they get searched for linearly
// during line info processing
compiledMethodEntry.leafRangeIterator().forEachRemaining(subRange -> {
checkInclude(classEntry, subRange.getFileEntry(), visitedFiles, visitedDirs);
});
// now the non-leaf range files
compiledMethodEntry.topDownRangeIterator().forEachRemaining(subRange -> {
if (!subRange.isLeaf()) {
checkInclude(classEntry, subRange.getFileEntry(), visitedFiles, visitedDirs);
}
});
});
// now all files and dirs are known build an index for them
classEntry.buildFileAndDirIndexes();
}

private static void checkInclude(ClassEntry classEntry, FileEntry fileEntry, EconomicSet<FileEntry> visitedFiles, EconomicSet<DirEntry> visitedDirs) {
olpaw marked this conversation as resolved.
Show resolved Hide resolved
if (fileEntry != null && !visitedFiles.contains(fileEntry)) {
visitedFiles.add(fileEntry);
classEntry.includeFile(fileEntry);
DirEntry dirEntry = fileEntry.getDirEntry();
if (dirEntry != null && !dirEntry.getPathString().isEmpty() && !visitedDirs.contains(dirEntry)) {
visitedDirs.add(dirEntry);
classEntry.includeDir(dirEntry);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,9 @@
*/
public class DirEntry {
adinn marked this conversation as resolved.
Show resolved Hide resolved
private final Path path;
private final int idx;

public DirEntry(Path path, int idx) {
public DirEntry(Path path) {
this.path = path;
this.idx = idx;
}

public Path getPath() {
Expand All @@ -51,14 +49,4 @@ public Path getPath() {
public String getPathString() {
return path.toString();
}

/**
* Retrieve the index of the dir entry in the list of all known dirs.
*
* @return the index of the file entry.
*/
public int getIdx() {
return idx;
}

}
Loading