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.spit...@oracle.com
<mailto:serguei.spit...@oracle.com> <serguei.spit...@oracle.com
<mailto:serguei.spit...@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.li...@oracle.com
<mailto:per.li...@oracle.com>
>> <mailto:per.li...@oracle.com <mailto:per.li...@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
