8196989: Revamp G1 JMX MemoryPool and GarbageCollector MXBean definitions
David Holmes
david.holmes at oracle.com
Sun Oct 21 22:07:57 UTC 2018
Hi Paul,
On 20/10/2018 2:05 AM, Hohensee, Paul wrote:
> If we put the flag into deprecation, I’d like to keep it for a year so
> people have time to change their monitoring code (one release to change
> their code, and another to run with their new code), which would be two
> releases. I don’t know how the deprecation process works either. Note
> that if/when this gets backported to jdk8u and/or jdk11u, there’s no
> mechanism there to obsolete a flag.
First the new flag has to be added to the CSR to get approval to be
added. It would be quite strange to add a new flag and deprecate it at
the same time - not completely out of the question given its legacy
compatibility nature, but still odd. So I'd suggest not initially
deprecating it but rather file a new bug and CSR to deprecate in 13,
obsolete in 14 and expire in 15. That gives you 12 and 13 where the flag
is active - though you'll only get a deprecation warning in 13. The
obsoletion in 14 will require new bug, but not CSR. The expiration is
normally handled en-masse when we bump the JDK release version.
For 8u the deprecation process is more manual. You need to add an
explicit check and warning in arguments.cpp, then when you're ready to
obsolete it add it to the obsolete flag table and remove the deprecation
warning.
Cheers,
David
-----
> Discovered an issue with the
> jdk/java/lang/management/MemoryMXBean/CollectionUsageThreshold.java
> test, new new webrev at
>
> http://cr.openjdk.java.net/~phh/8196989/webrev.04/
> <http://cr.openjdk.java.net/%7Ephh/8196989/webrev.04/>
>
> Paul
>
> *From: *JC Beyler <jcbeyler at google.com>
> *Date: *Thursday, October 18, 2018 at 10:47 PM
> *To: *"Hohensee, Paul" <hohensee at amazon.com>
> *Cc: *"hotspot-gc-dev at openjdk.java.net"
> <hotspot-gc-dev at openjdk.java.net>, "serviceability-dev at openjdk.java.net"
> <serviceability-dev at openjdk.java.net>
> *Subject: *Re: 8196989: Revamp G1 JMX MemoryPool and GarbageCollector
> MXBean definitions
>
> Hi Paul,
>
> Looks much better to me. The other question I have is about the legacy
> mode. I understand why, from a tool's perspective, having a legacy mode
> is practical. By doing it this way, we are ensuring we don't break any
> tools (or at least they can use a flag to be "unbroken") and give time
> to migrate. This also provides an easier means to backport this fix to
> older JDKs because now the legacy mode can be used to not break anything
> and yet provide a means to migrate to a more sane vision of G1 collector
> definitions.
>
> Should the flag perhaps be automatically put in deprecation and then we
> can mark it as obsolete for JDK13? That would give a limited time for a
> flag but again I'm not sure this is really done?
>
> Or is the plan to keep the flag for a given number of versions, try out
> these new pools and ensure they provide what we need?
>
> Thanks!
>
> Jc
>
> On Thu, Oct 18, 2018 at 3:18 PM Hohensee, Paul <hohensee at amazon.com
> <mailto:hohensee at amazon.com>> wrote:
>
> Thanks for your review, JC. New webrev:
> http://cr.openjdk.java.net/~phh/8196989/webrev.03/
> <http://cr.openjdk.java.net/%7Ephh/8196989/webrev.03/>
>
> I updated the copyright date in memoryService.hpp because I forgot
> to do so in the patch for
> https://bugs.openjdk.java.net/browse/JDK-8195115. Thomas asked me to
> fix in it a separate CR, so I’ve reverted it. Ditto the #include
> fixes in g1FullGCOopClosures.inline.hpp and g1HeapVerifier.cpp. At
> one point during development, clang complained about the latter, but
> no longer does.
>
> The ‘break’ on the same line as the ‘}’ was in the original version,
> but I’ve moved it. :)
>
> The comment is indeed a bit opaque. I changed it to:
>
> // Only check heap pools that support a usage threshold.
>
> // This is typically only the old generation space
>
> // since the other spaces are expected to get filled up.
>
> if (p.getType() == MemoryType.HEAP &&
>
> p.isUsageThresholdSupported()) {
>
> // In all collectors except G1, only the old
> generation supports a
>
> // usage threshold. The G1 legacy mode "G1 Old Gen"
> also does. In
>
> // G1 default mode, both the old space ("G1 Old
> Space": it's not
>
> // really a generation in the non-G1 collector
> sense) and the
>
> // humongous space ("G1 Humongous Space"), support
> a usage threshold.
>
> // So, the following condition is true for all
> non-G1 old generations,
>
> // for the G1 legacy old gen, and for the G1
> default humongous space.
>
> // It is not true for the G1 default old gen.
>
> //
>
> // We're allocating humongous objects in this test,
> so the G1 default
>
> // mode "G1 Old Space" occupancy doesn't change,
> because humongous
>
> // objects are allocated in the "G1 Humongous
> Space". If we allowed
>
> // the G1 default mode "G1 Old Space", notification
> would never
>
> // happen because no objects are allocated there.
>
> if (!p.getName().equals("G1 Old Space")) {
>
> Finally, the G1MonitoringScope constructor now does a better job of
> selecting a memory manager.
>
> Paul
>
> *From: *JC Beyler <jcbeyler at google.com <mailto:jcbeyler at google.com>>
> *Date: *Wednesday, October 17, 2018 at 4:47 PM
> *To: *"Hohensee, Paul" <hohensee at amazon.com
> <mailto:hohensee at amazon.com>>
> *Cc: *"hotspot-gc-dev at openjdk.java.net
> <mailto:hotspot-gc-dev at openjdk.java.net>"
> <hotspot-gc-dev at openjdk.java.net
> <mailto:hotspot-gc-dev at openjdk.java.net>>,
> "serviceability-dev at openjdk.java.net
> <mailto:serviceability-dev at openjdk.java.net>"
> <serviceability-dev at openjdk.java.net
> <mailto:serviceability-dev at openjdk.java.net>>
> *Subject: *Re: 8196989: Revamp G1 JMX MemoryPool and
> GarbageCollector MXBean definitions
>
> Hi Paul,
>
> I looked at this but it took time for me to "digest" it and I
> haven't entirely gone through the real GC changes :)
>
> My few remarks on the webrev itself are:
>
> -
> http://cr.openjdk.java.net/~phh/8196989/webrev.02/src/hotspot/share/services/memoryService.hpp.udiff.html
> <http://cr.openjdk.java.net/%7Ephh/8196989/webrev.02/src/hotspot/share/services/memoryService.hpp.udiff.html>
>
> - There is no need to change the copyright, right?
>
> -
> http://cr.openjdk.java.net/~phh/8196989/webrev.02/test/hotspot/jtreg/gc/TestMemoryMXBeansAndPoolsPresence.java.udiff.html
> <http://cr.openjdk.java.net/%7Ephh/8196989/webrev.02/test/hotspot/jtreg/gc/TestMemoryMXBeansAndPoolsPresence.java.udiff.html>
>
> - the break seems to be on the wrong line, no?
>
> + } break;
>
> -
> http://cr.openjdk.java.net/~phh/8196989/webrev.02/test/jdk/java/lang/management/MemoryMXBean/LowMemoryTest.java.udiff.html
> <http://cr.openjdk.java.net/%7Ephh/8196989/webrev.02/test/jdk/java/lang/management/MemoryMXBean/LowMemoryTest.java.udiff.html>
>
> and
>
> http://cr.openjdk.java.net/~phh/8196989/webrev.02/test/jdk/java/lang/management/MemoryMXBean/MemoryManagement.java.udiff.html
> <http://cr.openjdk.java.net/%7Ephh/8196989/webrev.02/test/jdk/java/lang/management/MemoryMXBean/MemoryManagement.java.udiff.html>
>
> + // In G1, humongous objects are tracked in
> the old space only in
>
> + // legacy monitoring mode. In default mode,
> G1 tracks humongous
>
> + // objects in the humongous space, which
> latter also supports a
>
> + // usage threshold. Since we're allocating
> humongous objects in
>
> + // this test, in default mode the old space
> doesn't change. For
>
> + // this test, we use the old space in
> legacy mode (it's called
>
> + // "G1 Old Gen" and the humongous space in
> default mode. If we
>
> + // used "G1 Old Space" in default mode,
> notification would never
>
> + // happen.
>
> -> latter seems ot be the wrong word or something is missing in that
> sentence
>
> -> the parenthesis is never closed (it's called.... is missing a )
> somewhere
>
> Thanks,
>
> Jc
>
> On Wed, Oct 17, 2018 at 1:18 PM Hohensee, Paul <hohensee at amazon.com
> <mailto:hohensee at amazon.com>> wrote:
>
> Ping.
>
> *From: *serviceability-dev
> <serviceability-dev-bounces at openjdk.java.net
> <mailto:serviceability-dev-bounces at openjdk.java.net>> on behalf
> of "Hohensee, Paul" <hohensee at amazon.com
> <mailto:hohensee at amazon.com>>
> *Date: *Thursday, October 11, 2018 at 6:46 PM
> *To: *"hotspot-gc-dev at openjdk.java.net
> <mailto:hotspot-gc-dev at openjdk.java.net>"
> <hotspot-gc-dev at openjdk.java.net
> <mailto:hotspot-gc-dev at openjdk.java.net>>,
> "serviceability-dev at openjdk.java.net
> <mailto:serviceability-dev at openjdk.java.net>"
> <serviceability-dev at openjdk.java.net
> <mailto:serviceability-dev at openjdk.java.net>>
> *Subject: *Re: 8196989: Revamp G1 JMX MemoryPool and
> GarbageCollector MXBean definitions
>
> Any takers? :)
>
> *From: *serviceability-dev
> <serviceability-dev-bounces at openjdk.java.net
> <mailto:serviceability-dev-bounces at openjdk.java.net>> on behalf
> of "Hohensee, Paul" <hohensee at amazon.com
> <mailto:hohensee at amazon.com>>
> *Date: *Monday, October 8, 2018 at 7:50 PM
> *To: *"hotspot-gc-dev at openjdk.java.net
> <mailto:hotspot-gc-dev at openjdk.java.net>"
> <hotspot-gc-dev at openjdk.java.net
> <mailto:hotspot-gc-dev at openjdk.java.net>>,
> "serviceability-dev at openjdk.java.net
> <mailto:serviceability-dev at openjdk.java.net>"
> <serviceability-dev at openjdk.java.net
> <mailto:serviceability-dev at openjdk.java.net>>
> *Subject: *RFR: 8196989: Revamp G1 JMX MemoryPool and
> GarbageCollector MXBean definitions
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8196989
>
> CSR: https://bugs.openjdk.java.net/browse/JDK-8196991
>
> Webrev: http://cr.openjdk.java.net/~phh/8196989/webrev.02/
> <http://cr.openjdk.java.net/%7Ephh/8196989/webrev.02/>
>
> As requested, I split the jstat counter update off from the
> MXBean update. This is the MXBean update. The jstat counter RFE
> is https://bugs.openjdk.java.net/browse/JDK-8210965 and its CSR
> is https://bugs.openjdk.java.net/browse/JDK-8210966.
>
> The MXBean CSR is in draft state, I’d greatly appreciate review
> and sign-off.
>
> It’s been suggested that we add another pool to represent the
> free region set, but doing so would be incompatible with
> existing MXBean use invariants for all GCs. These are:
>
> 1. The sum of the pools’ MemoryUsage.max properties is the
> total reserved heap size.
> 2. The sum of the pools’ MemoryUsage.committed properties is
> the total committed size.
> 3. The sum of the pools’ MemoryUsage.used properties is the
> total size of the memory containing objects, live and
> dead-and-yet-to-be-collected, as the case might be, plus
> intentional gaps between them.
> 4. The total free space is (sum of the max properties – sum of
> the used properties).
> 5. The total uncommitted space is (sum of the max properties –
> sum of the committed properties).
> 6. The total committed free space is (2) – (3).
>
> To keep invariants 1, 2 and 3, the free region pool’s “max”
> property should be “undefined” (i.e., -1). The intuitive, to me,
> “used” property value would be the total free space, but that
> would violate invariant 4 above. Defining the “committed”
> property as the total committed free space would violate
> invariants 2 and 6.
>
> The patch passes the submit repo, hotspot tier1, and,
> separately, the serviceability, jfr, and gc jtreg tests. I’m
> uncertain how to construct a test that checks for valid MXBean
> content: the existing tests don’t. Any such test will be fragile
> due to possible future Hotspot changes that affect the values,
> and to run-to-run variability. I’ve done by-hand comparisons
> between the old and new MXBean content using the SwingSet2 demo,
> including using App CDS, and the numbers look reasonable.
>
> The guts of the change are in
> G1MonitoringSupport::recalculate_sizes,
> initialize_serviceability, memory_managers, memory_pools, and
> G1MonitoringScope. I also defined TraceConcMemoryManagerStats to
> track the concurrent cycle in a way analogous to
> TraceCMSMemoryManagerStats. The changes to the includes in
> g1FullGCOopClosures.inline.hpp and g1HeapVerifier.cpp are to
> satisfy compiler complaints. I changed the 3^rd argument to the
> G1MonitoringScope constructor to a mixed_gc flag, and use
> accessor methods instead of direct field accesses when accessor
> methods exist. I believe I’ve minimized the latter. I updated
> the copyright date to 2018 in memoryService.hpp because I
> neglected to do so in my previous G1 MXBean patch.
>
> Thanks,
>
> Paul
>
>
> --
>
> Thanks,
>
> Jc
>
>
> --
>
> Thanks,
>
> Jc
>
More information about the serviceability-dev
mailing list