8319548: Unexpected internal name for Filler array klass causes error in VisualVM

[tschatzl]

Hi all,

please review this change that changes the filler array class name (again) after user feedback.

In particular, the previous name Ljdk/internal/vm/FillerArray; confuses some tools (oracle/visualvm#523). I.e. it's not an array, but still variable sized.
This change adds the [ array bracket, and renames the element name to not have Array inside to not try to pretend that the element is some other kind of array.

Testing: tier1-6

Thanks,
Thomas

---

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-8319548: Unexpected internal name for Filler array klass causes error in VisualVM (Bug - P4)

Reviewers

• Albert Mingkun Yang (@albertnetymk - Reviewer)
• David Holmes (@dholmes-ora - Reviewer)

Contributors

• Tomáš Hůrka <tomas.hurka@oracle.com>

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 17155

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

Using diff file

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

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back tschatzl! 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.

[tschatzl]

/author add tomas.hurka@oracle.com

[openjdk]

@tschatzl add tomas.hurka@oracle.com is not a valid name and email string.

Syntax: /author [set|remove] [@user | openjdk-user | Full Name <email@address>]. For example:

• /author set @openjdk-bot
• /author set duke
• /author set J. Duke <duke@openjdk.org>
• /author @openjdk-bot
• /author remove @openjdk-bot
• /author remove

User names can only be used for users in the census associated with this repository. For other authors you need to supply the full name and email address.

[openjdk]

@tschatzl 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.

[tschatzl]

/label add hotspot-gc

[tschatzl]

/contributor add Tomáš Hůrka tomas.hurka@oracle.com

[openjdk]

@tschatzl
The hotspot-gc label was successfully added.

[openjdk]

@tschatzl
Contributor Tomáš Hůrka <tomas.hurka@oracle.com> successfully added.

[mlbridge]

Webrevs

• 00: Full (167354ae)

[openjdk]

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

8319548: Unexpected internal name for Filler array klass causes error in VisualVM

Co-authored-by: Tomáš Hůrka <tomas.hurka@oracle.com>
Reviewed-by: ayang, dholmes

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 41 new commits pushed to the master branch:

• e8768ae: 8321972: test runtime/Unsafe/InternalErrorTest.java timeout on linux-riscv64 platform
• f6fe39f: 8322078: ZipSourceCache.testKeySourceMapping() test fails with The process cannot access the file because it is being used by another process
• e204242: 8321017: Record in JFR that IEEE rounding mode was corrupted by loading a library
• 2d60955: 8322141: SequenceInputStream.transferTo should not return as soon as Long.MAX_VALUE bytes have been transferred
• e0bad51: 8322543: Parallel: Remove unused _major_pause_old_slope_counter
• 424c58f: 8187634: keystore.getCertificateAlias(cert) returns original alias, inconsistent with fix of JDK-6483657
• 14dab31: 8322377: Parallel: Remove unused arg in adjust_promo_for_pause_time and adjust_eden_for_pause_time
• 5fcac7c: 8322364: Parallel: Remove unused SizePolicyTrueValues enum members
• 2f917bf: 8322417: Console read line with zero out should zero out when throwing exception
• 7db69e6: 8322513: Build failure with minimal
• ... and 31 more: https://git.openjdk.org/jdk/compare/f55381950266088cc0284754b16663675867ac87...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.

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

[dholmes-ora]

I'm still struggling with what we are doing with this filler array stuff. IIUC the underlying type is actually int[] but we pretend it is FillerElement[]. Won't exposing this fake type just lead to further problems if tools try to inspect one of these arrays as-if it were an Object[] ??

[tschatzl]

I'm still struggling with what we are doing with this filler array stuff. IIUC the underlying type is actually int[] but we pretend it is FillerElement[].

The reason for having dedicated filler array klasses is detecting internally in the GC that we are currently looking at a dead object. That the VM can distinguish dead objects from live objects is required for some functionality (like heap dumping only live classes, but also verification) without undue performance (basically need to redo marking every time this is needed) or memory usage hit (1.5% of maximum Java heap size) when invoking some APIs.

E.g. any reference from a live object to a dead object will immediately assert.

There is also improved debugability due to that - i.e. if you see a reference to any such klass in a hs_err file, it is 99% an error with a reference where the barrier has been forgotten.

The reason for having a custom array type and not having a klass referencing int elements is because Hotspot does not support that.

Won't exposing this fake type just lead to further problems if tools try to inspect one of these arrays as-if it were an Object[] ??

We do not expose these arrays to regular APIs (e.g. we filter them out when iterating available klasses), and they are never referenced by other live objects by definition. So it is impossible for a Java application to get a reference to such objects or these klasses.

The only way they are exposed is in a heap dump also containing dead objects (and related functionality like the jcmd class histogram). Somebody at some time thought that this would be an interesting feature so we need to support it.

Previously one would get the same dead objects in that output just intermingled with other int[] arrays. Imo this discrimination is preferable and much more useful for heap analysis.

[tschatzl]

Fwiw, the original issue introducing details the advantages too https://bugs.openjdk.org/browse/JDK-8284435; it does not particularly point out how much memory this saves, but it mentions that it removes the need for keeping around an extra mark bitmap covering the whole Java heap (that 1.5%).

[dholmes-ora]

Thanks for clarifying @tschatzl that these pretend arrays are only exposed in this one case.

[dholmes-ora]

Looks good. Thanks

[tschatzl]

Thanks @albertnetymk @dholmes-ora for your reviews
/integrate

[openjdk]

Going to push as commit 05745e3.
Since your change was applied there have been 41 commits pushed to the master branch:

• e8768ae: 8321972: test runtime/Unsafe/InternalErrorTest.java timeout on linux-riscv64 platform
• f6fe39f: 8322078: ZipSourceCache.testKeySourceMapping() test fails with The process cannot access the file because it is being used by another process
• e204242: 8321017: Record in JFR that IEEE rounding mode was corrupted by loading a library
• 2d60955: 8322141: SequenceInputStream.transferTo should not return as soon as Long.MAX_VALUE bytes have been transferred
• e0bad51: 8322543: Parallel: Remove unused _major_pause_old_slope_counter
• 424c58f: 8187634: keystore.getCertificateAlias(cert) returns original alias, inconsistent with fix of JDK-6483657
• 14dab31: 8322377: Parallel: Remove unused arg in adjust_promo_for_pause_time and adjust_eden_for_pause_time
• 5fcac7c: 8322364: Parallel: Remove unused SizePolicyTrueValues enum members
• 2f917bf: 8322417: Console read line with zero out should zero out when throwing exception
• 7db69e6: 8322513: Build failure with minimal
• ... and 31 more: https://git.openjdk.org/jdk/compare/f55381950266088cc0284754b16663675867ac87...master

Your commit was automatically rebased without conflicts.

[openjdk]

@tschatzl Pushed as commit 05745e3.

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

[jjscl8888]

I observed a phenomenon on our application. When there is no traffic on a certain instance, the number of old generation objects suddenly increases at a certain moment. After dumping the object instances, I found a large number of jdk.internal.vm.FillerArray objects, occupying more than 10G of memory. Have you ever encountered this?

[tschatzl]

I observed a phenomenon on our application. When there is no traffic on a certain instance, the number of old generation objects suddenly increases at a certain moment. After dumping the object instances, I found a large number of jdk.internal.vm.FillerArray objects, occupying more than 10G of memory. Have you ever encountered this?

You probably did the heap dump right after G1 managed to free lots of memory - these FillerArray elements represent unused memory within regions.

Previously one would have seen a huge amount of int-arrays ([I) staying around.

Normally G1 would then incrementally reduce these FillerArrays in subsequent mixed collections (after marking etc) by evacuating the live objects between these filler objects, making these regions completely empty.

From the given output of jmap and not knowing what else is going on with the GC this behavior seems normal.

[jjscl8888]

I observed a phenomenon on our application. When there is no traffic on a certain instance, the number of old generation objects suddenly increases at a certain moment. After dumping the object instances, I found a large number of jdk.internal.vm.FillerArray objects, occupying more than 10G of memory. Have you ever encountered this?

You probably did the heap dump right after G1 managed to free lots of memory - these FillerArray elements represent unused memory within regions.

Previously one would have seen a huge amount of int-arrays ([I) staying around.

Normally G1 would then incrementally reduce these FillerArrays in subsequent mixed collections (after marking etc) by evacuating the live objects between these filler objects, making these regions completely empty.

From the given output of jmap and not knowing what else is going on with the GC this behavior seems normal.

Thank you for your clarification. if the instance in question had no traffic but you observed a sudden increase in the old generation size at 2:35 in the graph, and subsequent garbage collections (GCs) did not reduce the size of the old generation back to its original value

[tschatzl]

(because the bot does not seem to forward the answer from the mailing list within a few hours; fwiw, it has been pure luck that I stumbled across that question within github):

On 30.04.24 03:38, jjscl8888 wrote:

Thank you for your clarification. if the instance in question had no
traffic but you observed a sudden increase in the old generation size
at 2:35 in the graph, and subsequent garbage collections (GCs) did not
reduce the size of the old generation back to its original value

Collectors are fairly reluctant to give back memory to the OS.

For G1 in particular, there are the options MinHeapFreeRatio and MaxHeapFreeRatio which to some degree steer commit and uncommit.

• MinHeapFreeRatio is "The minimum percentage of heap free after GC to avoid expansion", i.e. minimum amount of memory should be kept free. Default is 40%, i.e. expands if less than that amount of memory is free.

• MaxHeapFreeRatio is "The maximum percentage of heap free after GC to avoid shrinking", i.e. maximum amount of memory that should be kept free. Default is 70%; i.e. only shrinks the heap if more than 70% of memory is free.

Not sure the latter condition is met here to shrink, and without logs (-Xlog:gc+ergo+heap=debug) this is just a guess. Also, this kind of heap resizing (including shrinking) only occurs in the Remark pause.

So to decrease the heap more aggressively, it might work to decrease MaxHeapFreeRatio (and probably MinHeapFreeRatio too because for such large heaps the default values are maybe not optimal).

Hth,
Thomas

[mlbridge]

Mailing list message from Thomas Schatzl on hotspot-dev:

On 30.04.24 03:38, jjscl8888 wrote:

Thank you for your clarification. if the instance in question had no
traffic but you observed a sudden increase in the old generation size
at 2:35 in the graph, and subsequent garbage collections (GCs) did not
reduce the size of the old generation back to its original value

Collectors are fairly reluctant to give back memory to the OS.

For G1 in particular, there are the options `MinHeapFreeRatio` and
`MaxHeapFreeRatio` which to some degree steer commit and uncommit.

* `MinHeapFreeRatio` is "The minimum percentage of heap free after GC to
avoid expansion", i.e. minimum amount of memory should be kept free.
Default is 40%, i.e. expands if less than that amount of memory is free.

* `MaxHeapFreeRatio` is "The maximum percentage of heap free after GC to
avoid shrinking", i.e. maximum amount of memory that should be kept
free. Default is 70%; i.e. only shrinks the heap if more than 70% of
memory is free.

Not sure the latter condition is met here to shrink, and without logs
(`-Xlog:gc+ergo+heap=debug`) this is just a guess. Also, this kind of
heap resizing (including shrinking) only occurs in the Remark pause.

So to decrease the heap more aggressively, it might work to decrease
`MaxHeapFreeRatio` (and probably `MinHeapFreeRatio` because for such
large heaps the default values are maybe not optimal).

Hth,
Thomas

[jjscl8888]

(because the bot does not seem to forward the answer from the mailing list within a few hours; fwiw, it has been pure luck that I stumbled across that question within github):

On 30.04.24 03:38, jjscl8888 wrote:

Thank you for your clarification. if the instance in question had no
traffic but you observed a sudden increase in the old generation size
at 2:35 in the graph, and subsequent garbage collections (GCs) did not
reduce the size of the old generation back to its original value

Collectors are fairly reluctant to give back memory to the OS.

For G1 in particular, there are the options MinHeapFreeRatio and MaxHeapFreeRatio which to some degree steer commit and uncommit.

• MinHeapFreeRatio is "The minimum percentage of heap free after GC to avoid expansion", i.e. minimum amount of memory should be kept free. Default is 40%, i.e. expands if less than that amount of memory is free.
• MaxHeapFreeRatio is "The maximum percentage of heap free after GC to avoid shrinking", i.e. maximum amount of memory that should be kept free. Default is 70%; i.e. only shrinks the heap if more than 70% of memory is free.

Not sure the latter condition is met here to shrink, and without logs (-Xlog:gc+ergo+heap=debug) this is just a guess. Also, this kind of heap resizing (including shrinking) only occurs in the Remark pause.

So to decrease the heap more aggressively, it might work to decrease MaxHeapFreeRatio (and probably MinHeapFreeRatio too because for such large heaps the default values are maybe not optimal).

Hth, Thomas

Thank you for your previous question. I have another inquiry regarding compiling the JDK source code. I've noticed that when I compile the JDK without selecting specific configure parameters, the resulting JDK size differs from the official version available on the website. I'm curious to know which configuration parameters were used for the official LTS (Long-Term Support) version of the JDK.

[mlbridge]

Mailing list message from Thomas Schatzl on hotspot-gc-dev:

(The mail probably did not get sent to the correct recipient, so it did
not show up in the PR again; retry)

Hi,

On 08.05.24 04:21, jjscl8888 wrote:

On Fri, 3 May 2024 12:50:45 GMT, Thomas Schatzl <tschatzl at openjdk.org>
wrote:

Thank you for your previous question. I have another inquiry regarding
compiling the JDK source code. I've noticed that when I compile the
JDK without selecting specific configure parameters, the resulting JDK
size differs from the official version available on the website. I'm
curious to know which configuration parameters were used for the
official LTS (Long-Term Support) version of the JDK.

The page at
https://wiki.openjdk.org/display/Build/Supported+Build+Platforms
contains some information about the build platforms.

I will ask around if there is anything more specific than that.

Otherwise the people on the build-dev openjdk mailing list may be able
to answer you more appropriately.

Further I would like to recommend you to join the appropriate mailing
list (hotspot-gc-dev at openjdk.org or build-dev at openjdk.org) for asking
questions unrelated to this long-closed PR.

Hth,
Thomas