Skip to content

Commit

Permalink
Return to separate CUs per class to improve gdb startup time.
Browse files Browse the repository at this point in the history
style fixes

style fixes

more style fixes
  • Loading branch information
adinn committed Jul 11, 2023
1 parent e748693 commit 4d518b0
Show file tree
Hide file tree
Showing 16 changed files with 908 additions and 362 deletions.
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 @@ -313,8 +364,99 @@ public int lowpc() {
*
* @return the highest code section offset for compiled method code belonging to this class
*/
@SuppressWarnings("unused")
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);
// cannot add files after index has been created
assert fileIndex == null;
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) {
dirs = new ArrayList<>();
}
assert !dirs.contains(dirEntry);
// cannot add dirs after index has been created
assert dirIndex == null;
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;
assert dirIndex == null;
if (files == null) {
// should not have any dirs if we have no files
assert dirs == null;
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) {
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 {
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

0 comments on commit 4d518b0

Please sign in to comment.