[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR (XS) 8215329: Modify ZGC requirement for HeapMonitorThreadTest.java
From: Per Liden <per.liden () oracle ! com>
Date: 2018-12-15 14:37:57
Message-ID: 79179951-346d-f3ac-747c-122668d14612 () oracle ! com
[Download RAW message or body]
Just a follow up. After some further debugging, the real cause for this
issue turned out to be a bug in JNI IsSameObject(). Fix currently out
for review on hotspot-runtime-dev.
https://bugs.openjdk.java.net/browse/JDK-8215451
cheers,
Per
On 12/14/2018 07:52 PM, JC Beyler wrote:
> Thanks all, tested & pushed.
>
> @Per: let me know when you resolve your bug and if you want me to do
> anything from my side
>
> On Thu, Dec 13, 2018 at 10:25 AM serguei.spitsyn@oracle.com
> <mailto:serguei.spitsyn@oracle.com> <serguei.spitsyn@oracle.com
> <mailto:serguei.spitsyn@oracle.com>> wrote:
>
> +1
>
> Thanks,
> Serguei
>
>
> On 12/13/18 09:40, Alex Menkov wrote:
> > +1
> >
> > --alex
> >
> > On 12/13/2018 08:59, JC Beyler wrote:
> >> Hi Per,
> >>
> >> Thanks for the messages and review :-). I believe that actually
> what
> >> happened was that when JDK11 was close to release both ZGC and
> >> HeapMonitoring tried to get in. In a last effort, we turned off
> this
> >> test for ZGC as it was showing test failures for ZGC. It's a bit
> >> fuzzy to be honest (I was also on paternity leave and Serguei &
> >> Jeremy were helping out here). But anyway, what seems to be true
> now
> >> is that you found a bug (yeah I guess?) and we can remove
> >> the @requires once you fix your race.
> >>
> >> Therefore, could I please get another review to update
> the @requires
> >> to be correct then until then?
> >>
> >> (@Per: if you want, I can update the test once it's done; either
> >> assign a JBS bug to me or send me an email)
> >>
> >> Thanks,
> >> Jc
> >>
> >> On Thu, Dec 13, 2018 at 4:47 AM Per Liden <per.liden@oracle.com
> <mailto:per.liden@oracle.com>
> >> <mailto:per.liden@oracle.com <mailto:per.liden@oracle.com>>> wrote:
> >>
> >> Hi, me again ;)
> >>
> >> I think I've found the root cause of this. There's a tiny
> race in
> >> the
> >> ZGC allocation path, which can lead to pre-mature OOME being
> thrown.
> >> It's not trivial to fix, so I suggest you go ahead with your
> >> original
> >> patch (Looks good btw), and I'll file a separate bug to fix
> the ZGC
> >> issue (and update this test to run with ZGC again).
> >>
> >> cheers,
> >> Per
> >>
> >> On 12/13/2018 12:21 PM, Per Liden wrote:
> >> > Hi again,
> >> >
> >> > I ran this test some more and managed to get an OOME even
> with a
> >> 768M
> >> > heap. I'm getting a bit suspicious that something else is
> wrong
> >> here.
> >> > Let me dig into this some more and see if I can
> understand what
> >> the real
> >> > issue is.
> >> >
> >> > cheers,
> >> > Per
> >> >
> >> > On 12/13/2018 10:31 AM, Per Liden wrote:
> >> >> Hi JC,
> >> >>
> >> >> What's the reason to exclude ZGC from this test to begin
> >> with? From
> >> >> what I can tell, it's because the test is using a
> slightly too
> >> small
> >> >> heap, or are there some other reason? I ran it a few
> times using
> >> >> various heap sizes and the test passes with ZGC when using
> >> anything
> >> >> above 612M. So if we instead just dump the heap size a bit,
> >> then we
> >> >> get test coverage with ZGC too. I picked 768M here to
> have some
> >> >> headroom in case the exact limit is run-to-run dependent.
> >> >>
> >> >> diff --git
> >> >>
> >>
> a/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorThreadTest.java
> >>
> >> >>
> >>
> b/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorThreadTest.java
> >>
> >> >>
> >> >> ---
> >> >>
> >>
> a/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorThreadTest.java
> >>
> >> >>
> >> >> +++
> >> >>
> >>
> b/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorThreadTest.java
> >>
> >> >>
> >> >> @@ -29,8 +29,7 @@
> >> >> * @build Frame HeapMonitor ThreadInformation
> >> >> * @summary Verifies the JVMTI Heap Monitor Thread
> information
> >> sanity.
> >> >> * @compile HeapMonitorThreadTest.java
> >> >> - * @run main/othervm/native -Xmx512m
> -agentlib:HeapMonitorTest
> >> >> MyPackage.HeapMonitorThreadTest
> >> >> - * @requires !vm.gc.Z
> >> >> + * @run main/othervm/native -Xmx768m
> -agentlib:HeapMonitorTest
> >> >> MyPackage.HeapMonitorThreadTest
> >> >> */
> >> >>
> >> >> import java.util.List;
> >> >>
> >> >> cheers,
> >> >> Per
> >> >>
> >> >> On 12/13/2018 05:44 AM, JC Beyler wrote:
> >> >>> Hi all,
> >> >>>
> >> >>> When working on another webrev, I saw this problem:
> >> >>>
> >> >>> Webrev:
> http://cr.openjdk.java.net/~jcbeyler/8215329/webrev.00/
> >> >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8215329
> >> >>>
> >> >>> (Basically, from what I understood from an email from Per
> >> Liden:
> >> >>> - @requires !vm.gc.Z -> ZGC is built in the JDK
> >> >>> - @requires vm.gc != "Z" -> ZGC is being used for the
> >> runtime
> >> >>> )
> >> >>>
> >> >>> Thanks,
> >> >>> Jc
> >>
> >>
> >>
> >> --
> >>
> >> Thanks,
> >> Jc
>
>
>
> --
>
> Thanks,
> Jc
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic