Skip to content

Conversation

@graalvmbot
Copy link
Collaborator

As a default using -g will create DebugLocationInfo only for the body of compiled method but not for any method inlined into the compiled method. Despite that we always produced a full FrameTree even though only the first two levels are only every visited in this mode. This is a huge waste of time in the debug info generation part of the image built time.

This PR ensures that only when -H:-OmitInlinedMethodDebugLineInfo is used a full FrameTree gets created.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Apr 26, 2023
@olpaw olpaw requested a review from adinn April 26, 2023 16:04
if (!SubstrateOptions.DebugCodeInfoUseSourceMappings.hasBeenSet()) {
/* Skip expensive CompilationResultFrameTree building with SourceMappings */
useSourceMappings = false;
}
Copy link
Member

@olpaw olpaw Apr 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If DebugCodeInfoMaxDepth or DebugCodeInfoUseSourceMappings are set explicitly by the user (-H:...) they override any adjustment of the settings performed for OmitInlinedMethodDebugLineInfo.

@olpaw olpaw self-assigned this Apr 26, 2023
@olpaw
Copy link
Member

olpaw commented Apr 27, 2023

@adinn ideally I would want to switch to OmitInlinedMethodDebugLineInfo to be disabled per default. But to get there I need more testing and maybe a bit of optimizations in com.oracle.svm.core.code.CompilationResultFrameTree.Builder#build. This will likely require a follow up PR (that will not be backported to 23.0).

@olpaw
Copy link
Member

olpaw commented Apr 27, 2023

I tested with the example that caused most debuginfo generation time increase. And it turns out now with SourceMappings not being used anymore we can have -H:-OmitInlinedMethodDebugLineInfo as default and we are still much faster than before:

dacapo-9-12-mr1-git+2baec49-fop-pgo-ee -H:-UseOldDebugInfo

  • With -H:-OmitInlinedMethodDebugLineInfo
60.49MB (38.51%) for debug info generated in 7.4s

vs. master

213.61MB (68.86%) for debug info generated in 41.0s
  • Without -H:-OmitInlinedMethodDebugLineInfo
  41.42MB (30.01%) for debug info generated in 6.2s

vs. master

  54.83MB (36.21%) for debug info generated in 28.1s

Copy link
Collaborator

@adinn adinn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Paul. This looks good modulo one opportunity to remove a small bit of technical debt.

log(context, " Unable to retrieve call line for inlined method %s", callee.getFullMethodName());
/* continue with line 0 and fileIndex 1 as we must insert a tree node */
callLine = 0;
fileIndex = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local var fileIndex is declared a few lines up as an Integer, a legacy of an earlier version where the index for a given source dir was stored in a per-CU (i.e. per ClassEntry) map. This no longer applies with the latest code where the file index is defined globally across all classes, hence stored as an int in the FileEntry. It would be good to take the opportunity to clean this up by declaring fileIndex as an int.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OCA Verified All contributors have signed the Oracle Contributor Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants