-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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-33516] Updated fix for inline debuginfo to fix issue 2701 #3745
Conversation
If line gets successfully retrieved from subrange instead of primaryrange get file index from the subrange since the line might be from a different file for inlined methods.
When using -H:+OmitInlinedMethodDebugLineInfo we know that ranges are not expected to have inlined subranges so we can avoid looping over them to see if we need to generate inline debug info.
This way we save space and time by not replicating abstract inline DIEs in the caller's CU.
* remove line records with zero extent * remove caller marking substitution entry (bci = -1)
PrimaryEntry provides a top-down subrange iterator that performs a depth-first traversal of the call-graph and a leaf subrange iterator that performs a depth-first traversal returning only the leafs. The sibling nodes in the tree are being merged whenever possible to reduce the debuginfo size. Co-authored-by: Foivos Zakkak <fzakkak@redhat.com>
Co-authored-by: Andrew Dinn <adinn@redhat.com>
This way we avoid the expensive calls to "toJavaName". This patch reduces the time spend in debug info generation from ~12s to 5s.
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.
Looks fine overall.
I will run this in our CI now and give it a deeper second look.
Please amend b08c6ce to have the commit message start with uppercase (like all other commits).
Copy original source position in InvokeWithExceptionNode.replaceWithInvoke Update debug info test to resstore check for resulting inline frames
@olpaw wrote:
Done |
One of our Windows gates triggers our deadlock-during-image-building detection with the following stacks:
I will see if I can come up with a reproducer. |
Interesting that this is wedged in the code that is sorting the class's method list. Foivos employed a sort to make it quicker to lookup existing entries when a method already exists . I think it is a much better idea to employ a separate hash table index to ensure that this check is constant time (that's what I did for all the other parts of the model). I'll prepare a revision to the patch do that. |
@olpaw I just pushed a fix to use a hash index rather than sorting the methods list. Let's see if that fixes the gate timeout. n.b. I also had to fix a regression I introduced into the debuginfo test script when I was updating to allow for proper inline info. This meant some inline backtrace output was not being correctly validated (the backtraces were still correct). The tests are now all passing with the expected output. |
Hi @adinn |
@adinn it seems you have a leftover from sorting methods list:
|
@olpaw The latest push has the redundant code removed. Testing has one failure which seems to be spurious (on jdk11 GraalCheckIntrinsics.test checks for float and double copySign intrinsics are marked as ignored then that is raised as a red flag when intrinsics are found). |
@adinn please start commit messages with uppercase letters. Apart from that it looks good now. All tests passed except for the style gates (I started running gates before you added 21d9ca3) |
Aaargh! Sorry for that. I pushed a corrected update. |
Rerunning in internal CI ... |
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.
All gates passed. The PR is in the merge queue.
@olpaw Excellent! Thanks. |
This PR supersedes PR #2880 posted by @zakkak with one extra commit. With that extra change the PR should be ready for review and able to close issue #2701.
The PR supplies a small fix to the compiler which ensures that inline call source positions are propagated through to compilation results.
It also updates and corrects the debuginfotest driver script to correctly validate the processing of those inline source positions into debug info embedded in a native image.