8314508: Improve how relativized pointers are printed by frame::describe #18102

fbredber · 2024-03-04T09:38:26Z

The output from frame::describe has been improved so that it now prints the actual derelativized pointer value, regardless if it's on the stack or on the heap. It also clearly shows which members of the fixed stackframe are relativized. See sample output of a riscv64 stackframe below:

[6.693s][trace][continuations]  0x000000409e14cc88: 0x00000000eeceaf58 locals for #10
[6.693s][trace][continuations]                                         local 0
[6.693s][trace][continuations]                                         oop for #10
[6.693s][trace][continuations]  0x000000409e14cc80: 0x00000000eec34548 local 1
[6.693s][trace][continuations]                                         oop for #10
[6.693s][trace][continuations]  0x000000409e14cc78: 0x00000000eeceac58 local 2
[6.693s][trace][continuations]                                         oop for #10
[6.693s][trace][continuations]  0x000000409e14cc70: 0x00000000eee03eb0 #10 method java.lang.invoke.LambdaForm$DMH/0x0000000050001000.invokeVirtual(Ljava/lang/Object;Ljava/lang/Object;)V @ 10
[6.693s][trace][continuations]                                         - 3 locals 3 max stack
[6.693s][trace][continuations]                                         -  codelet: return entry points
[6.693s][trace][continuations]                                         saved fp
[6.693s][trace][continuations]  0x000000409e14cc68: 0x00000040137dc790 return address
[6.693s][trace][continuations]  0x000000409e14cc60: 0x000000409e14ccf0
[6.693s][trace][continuations]  0x000000409e14cc58: 0x000000409e14cc60 interpreter_frame_sender_sp
[6.693s][trace][continuations]  0x000000409e14cc50: 0x000000409e14cc00 interpreter_frame_last_sp        (relativized: fp-14)
[6.693s][trace][continuations]  0x000000409e14cc48: 0x0000004050401a88 interpreter_frame_method
[6.693s][trace][continuations]  0x000000409e14cc40: 0x0000000000000000 interpreter_frame_mdp
[6.693s][trace][continuations]  0x000000409e14cc38: 0x000000409e14cbe0 interpreter_frame_extended_sp    (relativized: fp-18)
[6.693s][trace][continuations]  0x000000409e14cc30: 0x00000000ef093110 interpreter_frame_mirror
[6.693s][trace][continuations]                                         oop for #10
[6.693s][trace][continuations]  0x000000409e14cc28: 0x0000004050401c68 interpreter_frame_cache
[6.693s][trace][continuations]  0x000000409e14cc20: 0x000000409e14cc88 interpreter_frame_locals         (relativized: fp+3)
[6.693s][trace][continuations]  0x000000409e14cc18: 0x0000004050401a7a interpreter_frame_bcp
[6.693s][trace][continuations]  0x000000409e14cc10: 0x000000409e14cc10 interpreter_frame_initial_sp     (relativized: fp-12)
[6.693s][trace][continuations]  0x000000409e14cc08: 0x00000000eec34548
[6.693s][trace][continuations]  0x000000409e14cc00: 0x0000000000000000 locals for #9
[6.693s][trace][continuations]                                         local 0
[6.693s][trace][continuations]                                         oop for #9
[6.693s][trace][continuations]  0x000000409e14cbf8: 0x00000000eecf1370 local 1
[6.693s][trace][continuations]                                         oop for #9
[6.693s][trace][continuations]  0x000000409e14cbf0: 0x00000000eecf1380 local 2
[6.693s][trace][continuations]                                         oop for #9
[6.693s][trace][continuations]  0x000000409e14cbe8: 0x0000000000000000 local 3
[6.693s][trace][continuations]  0x000000409e14cbe0: 0x0000000000000000 local 4
[6.693s][trace][continuations]                                         unextended_sp for #10

The output is was created by running test/jdk/jdk/internal/vm/Continuation/Basic.java with -Xlog:continuations=trace:frbr_log.txt.

Output has been reviewed for x64, aarch64, ppc64le and riscv64.

The changes also passes tier1-tier5.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8314508: Improve how relativized pointers are printed by frame::describe (Enhancement - P4)

Reviewers

Coleen Phillimore (@coleenp - Reviewer) ⚠️ Review applies to 4ade221c
Patricio Chilano Mateo (@pchilano - Reviewer) ⚠️ Review applies to 4ade221c

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/18102/head:pull/18102
$ git checkout pull/18102

Update a local copy of the PR:
$ git checkout pull/18102
$ git pull https://git.openjdk.org/jdk.git pull/18102/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 18102

View PR using the GUI difftool:
$ git pr show -t 18102

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/18102.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2024-03-04T09:39:12Z

👋 Welcome back fbredberg! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2024-03-04T09:39:36Z

@fbredber The following label will be automatically applied to this pull request:

hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2024-03-04T12:39:12Z

Webrevs

coleenp

I don't know this code that well but I had a couple of questions.

src/hotspot/share/runtime/frame.cpp

coleenp

Ok, this looks good. Thank you for the explanation. I had a couple of comment ideas that might make it make sense without reading the whole file.

src/hotspot/share/runtime/frame.cpp

openjdk · 2024-03-11T19:13:09Z

@fbredber This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8314508: Improve how relativized pointers are printed by frame::describe

Reviewed-by: coleenp, pchilanomate

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 109 new commits pushed to the master branch:

107cb53: 8327701: Remove the xlc toolchain
07acc0b: 8326385: [aarch64] C2: lightweight locking nodes kill the box register without specifying this effect
cc9a8ab: 8327460: Compile tests with the same visibility rules as product code
3b18c5d: 8323605: Java source launcher should not require --source ... to enable preview
5d4bfad: 8327693: C1: LIRGenerator::_instruction_for_operand is only read by assertion code
f3d0c45: 8327829: [JVMCI] runtime/ClassUnload/ConstantPoolDependsTest.java fails on libgraal
d5b95a0: 8327631: Update IANA Language Subtag Registry to Version 2024-03-07
966a42f: 8324868: debug agent does not properly handle interrupts of a virtual thread
22f10e0: 8327856: Convert applet test SpanishDiacriticsTest.java to a main program
7283c8b: 8327972: Convert java/awt/FileDialog/SaveFileNameOverrideTest/SaveFileNameOverrideTest.html applet test to main
... and 99 more: https://git.openjdk.org/jdk/compare/e9adcebaf242843fe2004b01747b5a930b62b291...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@coleenp, @pchilano) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

pchilano

Not a fan of printing a value that differs from the real one but I like that we still get it directly from the new description you added.

I see that we cannot do this derelativization for the fp of a heap frame since to know if it was actually relativized we need the caller frame info to check if it's interpreted.

fbredber · 2024-03-13T09:49:59Z

Thanks for the review.

fbredber · 2024-03-13T09:53:39Z

/integrate

openjdk · 2024-03-13T09:54:03Z

@fbredber
Your change (at version e48f8b1) is now ready to be sponsored by a Committer.

coleenp · 2024-03-13T15:14:25Z

/sponsor

openjdk · 2024-03-13T15:15:04Z

Going to push as commit 0db6231.
Since your change was applied there have been 117 commits pushed to the master branch:

0353245: 8325874: Improve checkbox-based interface in summary pages
4d64467: 8328079: JDK-8326583 broke ccache compilation
7e05a70: 8251330: Reorder CDS archived heap to speed up relocation
7d8561d: 8327109: Refactor data graph cloning used in create_new_if_for_predicate() into separate class
a4a5196: 8327872: Convert javax/swing/JToolTip/4644444/bug4644444.java applet test to main
da4dd7c: 8327989: java/net/httpclient/ManyRequest.java should not use "localhost" in URIs
49d8008: 8327452: G1: Improve scalability of Merge Log Buffers
0ae4fa7: 8327210: AIX: Delete obsolete parameter Use64KPagesThreshold
107cb53: 8327701: Remove the xlc toolchain
07acc0b: 8326385: [aarch64] C2: lightweight locking nodes kill the box register without specifying this effect
... and 107 more: https://git.openjdk.org/jdk/compare/e9adcebaf242843fe2004b01747b5a930b62b291...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-03-13T15:15:11Z

@coleenp @fbredber Pushed as commit 0db6231.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8314508: Improve how relativized pointers are printed by frame::describe

c5457c7

openjdk bot added the hotspot [email protected] label Mar 4, 2024

fbredber marked this pull request as ready for review March 4, 2024 12:35

openjdk bot added the rfr Pull request is ready for review label Mar 4, 2024

fbredber added 2 commits March 5, 2024 11:16

Small white space fix

b833ee0

Merge branch 'master' into 8314508_relativized_ptr_frame_describe

4ade221

coleenp reviewed Mar 7, 2024

View reviewed changes

src/hotspot/share/runtime/frame.cpp Show resolved Hide resolved

coleenp approved these changes Mar 11, 2024

View reviewed changes

src/hotspot/share/runtime/frame.cpp Show resolved Hide resolved

openjdk bot added the ready Pull request is ready to be integrated label Mar 11, 2024

pchilano approved these changes Mar 12, 2024

View reviewed changes

Update after review

e48f8b1

openjdk bot added the sponsor Pull request is ready to be sponsored label Mar 13, 2024

openjdk bot added the integrated Pull request has been integrated label Mar 13, 2024

openjdk bot closed this Mar 13, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Mar 13, 2024

8314508: Improve how relativized pointers are printed by frame::describe #18102

8314508: Improve how relativized pointers are printed by frame::describe #18102

Uh oh!

Conversation

fbredber commented Mar 4, 2024 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Webrev

Uh oh!

bridgekeeper bot commented Mar 4, 2024

Uh oh!

openjdk bot commented Mar 4, 2024

Uh oh!

mlbridge bot commented Mar 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

coleenp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coleenp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

openjdk bot commented Mar 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pchilano left a comment

Choose a reason for hiding this comment

Uh oh!

fbredber commented Mar 13, 2024

Uh oh!

fbredber commented Mar 13, 2024

Uh oh!

openjdk bot commented Mar 13, 2024

Uh oh!

coleenp commented Mar 13, 2024

Uh oh!

openjdk bot commented Mar 13, 2024

Uh oh!

openjdk bot commented Mar 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

fbredber commented Mar 4, 2024 •

edited by openjdk bot

Loading

mlbridge bot commented Mar 4, 2024 •

edited

Loading

openjdk bot commented Mar 11, 2024 •

edited

Loading