-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[GR-45654] Single compile unit for Java DWARF debug info #6414
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the PR, @adinn. It seems this significantly simplifies the debug info infrastructure, great! In addition to that and the reduced size of the debug info, does this also reduce footprint and/or improve the time to generate debug info? No need to run a full set of benchmarks, a rough estimate is probably good enough. :)
substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/DebugInfoBase.java
Outdated
Show resolved
Hide resolved
@@ -37,9 +37,11 @@ | |||
*/ | |||
public class DirEntry { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems we could use records for DirEntry
, FileEntry
, and MemberEntry
in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what you mean by that. Do you mean outside of the com.oracle.objectfile
package? They are already used outside of com.oracle.objectfile.debugentry
as they are referenced by com.oracle.objectfile.elf.dwarf
classes.
substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/DirEntry.java
Outdated
Show resolved
Hide resolved
substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/FileEntry.java
Outdated
Show resolved
Hide resolved
@fniephaus I built a simple app (GC tester that turns over memory) with the following results Before:
After:
n.b. the build script I am using, I'm not sure the reported heap sizes at each stage are reliable. As far as I am aware this change could not have reduced the compiler phase heap size by such a large amount nor could it have produced such a large increase in the heap sizes for layout and image generation. The peak RSS is perhaps the most reliable indicator -- but it only drops by ~2% so that may be noise. The times for the layout and image create and the overall time do go down after this change by a couple of seconds, which might perhaps a bit more reliable, but the drop of ~4 seconds in compile time does not make much sense. So maybe any difference after this is also noise. The one thing that I am fairly confident is reliable is the size of the debug info. It has gone down from 20010752 to 18778328 bytes i.e. ~6% or ~1.2Mb. Even though this is a small app it contains many hundreds of classes so the percentage saving is probably going to be a representative for all apps. |
This PR reorganizes the DWARF debug info layout so that all info is all contained in a single compile unit (CU) rather than employing a CU for every Java class type.
It profits from that reorganization to achieve several important goals:
The re-implementation also has the benefit that it simplifies many of the DWARF generation steps, both when iterating the generic debug model to write specific debug sections and when cross-indexing location references within and across sections.