8375051: [lworld] debugging methods should provide more Valhalla related information

[Arraying]

Hi all,

This adds some more debug prints to various internals.

When printing an InstanceKlass, the kind is now shown. For example:

InstanceKlass (kind=1): java.lang.Integer {0x00001ff00022c318}

The InlineKlass::Members are also printed. For example (note that to get individual array entries WizardMode must be enabled):

 - ---- inline type members:
 - extended signature registers:      Array<T>(0x00000001330002f8)
0 : SigEntry: type=18 offset=0 null_marker=0 Symbol: 'java/lang/Integer' count 65535
1 : SigEntry: type=10 offset=16 null_marker=0 Symbol: 'value' count 65535
2 : SigEntry: type=14 offset=24 null_marker=0 Symbol: 'java/lang/Integer' count 65535
 - return registers:                  Array<T>(0x0000000133000348)
0 : (c_rarg0,c_rarg0)
1 : (c_rarg7,BAD!)
 - pack handler:                      0x0000000119b21238
 - pack handler (jobject):            0x0000000119b21140
 - unpack handler:                    0x0000000119b21244
 - null reset offset:                 128
 - payload offset:                    16
 - payload size (bytes):              8
 - payload alignment:                 8
 - null-free non-atomic size (bytes): 4
 - null-free non-atomic alignment:    4
 - null-free atomic size (bytes):     4
 - nullable atomic size (bytes):      8
 - nullable non-atomic size (bytes):  5
 - null marker offset:                20

Fields now indicate their field flags. Non-static fields indicate (a) when they are flat and (b) their layout when flat. For example:

 - ---- non-static fields (1 words):
 - private final value flat 'inner' (fields 0x00000006) 'LAValue;' @12 LayoutKind: NULLABLE_NON_ATOMIC_FLAT

Arrays already have plenty of flattening information that can be printed.

Testing: tiers 1-3.

---

Progress

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

Issue

• JDK-8375051: [lworld] debugging methods should provide more Valhalla related information (Enhancement - P4)

Reviewers

• Axel Boldt-Christmas (@xmas92 - Committer)
• Frederic Parain (@fparain - Committer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2208

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/2208.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

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

[openjdk]

@Arraying 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:

8375051: [lworld] debugging methods should provide more Valhalla related information

Reviewed-by: aboldtch, fparain

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 377 new commits pushed to the lworld branch:

• 2701ce2: 8380191: [lworld] [BACKOUT] OptimizePtrCompare is too conservative
• 329029d: 8370070: [lworld] Merge of 8342382 causes IR mismatches in TestLWorld test85 & test86
• fbaf93a: 8380006: [lworld] Missing InternalOOMEMark in Deoptimization::realloc_objects
• ... and 374 more: https://git.openjdk.org/valhalla/compare/8893112af9c0f21751c798d19dec3e67b7ecf1bf...lworld

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.

➡️ To integrate this PR with the above commit message to the lworld branch, type /integrate in a new comment.

[mlbridge]

Webrevs

• 02: Full - Incremental (cd9b96b2)
• 01: Full - Incremental (cd8b1364)
• 00: Full (32d175fd)

[xmas92]

Very nice to add more of this printing. I added something similar my self while debugging.

I tried to mimic the PrintInlineLayout and PrintFieldLayout a bit more. But I think this more generally useful and extendable for debugging purposes.

I also like to use the StreamIndentor to get a more tree view when doing nested printing / printing subcategories. But not sure all agree here.

Regardless this is a much needed improvement (which we can extend in the future)

[fparain]

This patch is showing a bug in the way the AccessFlags are printed (the bug is not in this patch, but this patch is using the buggy code):

` - ---- static fields (6 words):

• private static final value 'FS' (fields 0x00000000) 'Ljava/io/FileSystem;' @120
• public static final value 'separatorChar' (fields 0x00000000) 'C' @160
• public static final value 'separator' (fields 0x00000000) 'Ljava/lang/String;' @124
• public static final value 'pathSeparatorChar' (fields 0x00000000) 'C' @162
• public static final value 'pathSeparator' (fields 0x00000000) 'Ljava/lang/String;' @128
• private static final value 'UNSAFE' (fields 0x00000000) 'Ljdk/internal/misc/Unsafe;' @132
• private static final value 'PATH_OFFSET' (fields 0x00000000) 'J' @136
• private static final value 'PREFIX_LENGTH_OFFSET' (fields 0x00000000) 'J' @144
• private static final value 'serialVersionUID' (fields 0x00000008) 'J' @152
• static final synthetic value '$assertionsDisabled' (fields 0x00000000) 'Z' @164
• ---- non-static fields (4 words):
• private final transient value 'prefixLength' (fields 0x00000000) 'I' @12
• private final value 'path' (fields 0x00000000) 'Ljava/lang/String;' @16
• private transient value 'status' (fields 0x00000000) 'Ljava/io/File$PathStatus;' @20
• private volatile transient value 'filePath' (fields 0x00000000) 'Ljava/nio/file/Path;' @24 `

All fields are marked as 'value' when obviously, none of them are storing values from value classes!
The culprit is in AccessFlags::print_on(outputStream* st):

if (Arguments::is_valhalla_enabled()) { if (is_identity_class()) st->print("identity "); if (!is_identity_class()) st->print("value " ); }

This piece of code was added considering class AccessFlags and not field AccessFlags. ACC_IDENTITY has value 0x20, and no field flag is using this value, leading the code above to always print "value ".

[Arraying]

I also like to use the StreamIndentor to get a more tree view when doing nested printing / printing subcategories. But not sure all agree here.

I'd also like to see indentation, esp. for array elements. I think this is something we can/should address in mainline. I've filed JDK-8380133 for tracking.

This piece of code was added considering class AccessFlags and not field AccessFlags. ACC_IDENTITY has value 0x20, and no field flag is using this value, leading the code above to always print "value ".

Great catch. I've filed JDK-8380129 as a follow-up. Could you confirm you're okay with this set of changes going in?

[fparain]

Looks good to me, with the recommendation to fix JDK-8380129 a soon as possible to prevent confusion for people looking at the new output.

[Arraying]

Thanks for the reviews @xmas92 @fparain. Recommendation noted.
/integrate

[openjdk]

Going to push as commit 2e98a4d.
Since your change was applied there have been 377 commits pushed to the lworld branch:

• 2701ce2: 8380191: [lworld] [BACKOUT] OptimizePtrCompare is too conservative
• 329029d: 8370070: [lworld] Merge of 8342382 causes IR mismatches in TestLWorld test85 & test86
• fbaf93a: 8380006: [lworld] Missing InternalOOMEMark in Deoptimization::realloc_objects
• ... and 374 more: https://git.openjdk.org/valhalla/compare/8893112af9c0f21751c798d19dec3e67b7ecf1bf...lworld

Your commit was automatically rebased without conflicts.

[openjdk]

@Arraying Pushed as commit 2e98a4d.

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