RFR: 8241828: JFR: Some streaming tests require a larger heap size with ZGC
Stefan Karlsson
stefan.karlsson at oracle.com
Wed Apr 8 08:30:33 UTC 2020
OK, so I wasn't crazy when I said that the code pasted below didn't
match the code in TestFilledChunk.
I've run the jdk/jfr tests with ZGC and they all now passes. So, this
looks good from that POV (haven't looked at what setOrdered actually does).
Thanks for fixing this,
StefanK
On 2020-04-08 02:43, Erik Gahlin wrote:
> Sorry for the delay. Here is the webrev.
>
> http://cr.openjdk.java.net/~egahlin/8241828/
>
> Erik
>
>> On 2 Apr 2020, at 12:20, Stefan Karlsson <stefan.karlsson at oracle.com
>> <mailto:stefan.karlsson at oracle.com>> wrote:
>>
>> Do you intend to fix this soon? Otherwise I suggest we correct the
>> TestChunkGap test, and increase the memory for the other, so that we
>> can get clean runs when we run JFR with ZGC.
>>
>> Thanks,
>> StefanK
>>
>> On 2020-03-31 14:48, Erik Gahlin wrote:
>>> I can take over the bug.
>>>
>>> Erik
>>>
>>>> On 31 Mar 2020, at 13:49, Stefan Karlsson
>>>> <stefan.karlsson at oracle.com <mailto:stefan.karlsson at oracle.com>
>>>> <mailto:stefan.karlsson at oracle.com>> wrote:
>>>>
>>>> On 2020-03-31 12:46, Erik Gahlin wrote:
>>>>>
>>>>>> On 31 Mar 2020, at 11:03, Stefan Karlsson
>>>>>> <stefan.karlsson at oracle.com <mailto:stefan.karlsson at oracle.com>
>>>>>> <mailto:stefan.karlsson at oracle.com>> wrote:
>>>>>>
>>>>>> On 2020-03-31 10:43, Erik Gahlin wrote:
>>>>>>> Doesn’t look good.
>>>>>>> One problem is that TestChunkGap has TestFilledChunks in @run
>>>>>>> main. If you change it to TestChunkGap, that test should be fine.
>>>>>> OK. So we found an old bug lurking here. It's not the first time I
>>>>>> see this kind of bugs. It would be nice if we could detect those
>>>>>> @run copy-n-paste bugs.
>>>>> I agree.
>>>>>
>>>>> If you omit the class in @run, jtreg should use the main method in
>>>>> the file. If there are multiple main methods, it should fail
>>>>> stating that it must be specified.
>>>>>
>>>>>>> When it comes to TestFilledChunk, I can now see how this can
>>>>>>> happen due to events being sorted by default and the high rate
>>>>>>> events are emitted, You could try to sett s.setSorted(false), and
>>>>>>> it should be fine as well.
>>>>>>> 63 try (EventStream s = EventStream.openRepository()) {
>>>>>>> 64 try (Recording r1 = new Recording()) {
>>>>>>> +65 s.setSorted(false);
>>>>>>> 66 s.setStartTime(Instant.EPOCH);
>>>>>> That seems to be the code in TestChunkGap, that now (after the fix
>>>>>> above) works OK with ZGC. Do you have a similar fix for
>>>>>> TestFilledCunk?
>>>>> The above fix is for TestFilledChunk.
>>>>
>>>> This doesn't match the code I have. The code you pasted above is
>>>> from TestChunkGap. Can you provide a proper patch or webrev so that
>>>> I understand what you are proposing. Alternative, I'd be happy to
>>>> leave the fix of this over to you.
>>>>
>>>> Thanks,
>>>> StefanK
>>>>
>>>>>
>>>>> TestChunkGap should work after the change of @run, at least it does
>>>>> for me locally.
>>>>>
>>>>> Erik
>>>>>
>>>>>> Thanks,
>>>>>> StefanK
>>>>>>
>>>>>>> Thanks
>>>>>>> Erik
>>>>>>>> On 31 Mar 2020, at 10:08, Stefan Karlsson
>>>>>>>> <stefan.karlsson at oracle.com <mailto:stefan.karlsson at oracle.com>
>>>>>>>> <mailto:stefan.karlsson at oracle.com>
>>>>>>>> <mailto:stefan.karlsson at oracle.com>> wrote:
>>>>>>>>
>>>>>>>> Did some extra testing on TestChunkGap and G1:
>>>>>>>>
>>>>>>>> With compressed oops:
>>>>>>>> -Xmx300m : OOME
>>>>>>>> -Xmx350m : Barely passes
>>>>>>>>
>>>>>>>> Without compressed oops:
>>>>>>>> -Xmx450m : OOME
>>>>>>>> -Xmx500m : Passes
>>>>>>>>
>>>>>>>> I issued a 'jcmd GC.class_histogram' in the middle of the run
>>>>>>>> and got:
>>>>>>>>
>>>>>>>> num #instances #bytes class name (module)
>>>>>>>> -------------------------------------------------------
>>>>>>>> 1: 2473067 158635784 [Ljava.lang.Object;
>>>>>>>> (java.base at 15-internal)
>>>>>>>> 2: 2463224 118234752
>>>>>>>> jdk.jfr.consumer.RecordedEvent (jdk.jfr at 15-internal)
>>>>>>>> 3: 4894598 117470352 java.lang.Integer
>>>>>>>> (java.base at 15-internal)
>>>>>>>> 4: 157 39960680
>>>>>>>> [Ljdk.jfr.consumer.RecordedEvent; (jdk.jfr at 15-internal)
>>>>>>>> 5: 369731 18447144 [B (java.base at 15-internal)
>>>>>>>> 6: 368815 11802080 java.lang.String
>>>>>>>> (java.base at 15-internal)
>>>>>>>> 7: 331 584344 [I (java.base at 15-internal)
>>>>>>>> ...
>>>>>>>>
>>>>>>>> Does this look right?
>>>>>>>>
>>>>>>>> StefanK
>>>>>>>>
>>>>>>>> On 2020-03-30 22:03, Erik Gahlin wrote:
>>>>>>>>> Seems strange that the test would require over 512 MB. I would
>>>>>>>>> expect the TestChunkGap not to use a live set of more than 25
>>>>>>>>> MB. The test only emits three events.
>>>>>>>>> TestFilledChunks is easier to understand as it may emit
>>>>>>>>> millions of events. Still it must be a bug in JFR then, because
>>>>>>>>> it should not keep them around.
>>>>>>>>> Erik
>>>>>>>>>> On 30 Mar 2020, at 19:24, mikhailo.seledtsov at oracle.com
>>>>>>>>>> <mailto:mikhailo.seledtsov at oracle.com>
>>>>>>>>>> <mailto:mikhailo.seledtsov at oracle.com>
>>>>>>>>>> <mailto:mikhailo.seledtsov at oracle.com>
>>>>>>>>>> <mailto:mikhailo.seledtsov at oracle.com> wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Stefan,
>>>>>>>>>>
>>>>>>>>>> I would recommend adding "@requires os.maxMemory > " to the
>>>>>>>>>> test, to make sure that test does not execute on a host/node
>>>>>>>>>> that does not have sufficient memory. E.g. "@requires
>>>>>>>>>> os.maxMemory > 1G"
>>>>>>>>>>
>>>>>>>>>> Otherwise the change looks good to me.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Misha
>>>>>>>>>>
>>>>>>>>>> On 3/30/20 4:07 AM, Stefan Karlsson wrote:
>>>>>>>>>>> Hi all,
>>>>>>>>>>>
>>>>>>>>>>> Please review this small patch to increase the max heap size
>>>>>>>>>>> of a couple of JFR streaming tests.
>>>>>>>>>>>
>>>>>>>>>>> https://cr.openjdk.java.net/~stefank/8241828/webrev.01/
>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8241828
>>>>>>>>>>>
>>>>>>>>>>> The default heap size is 512m when these tests are run. G1
>>>>>>>>>>> uses almost 500m, but ZGC needs a bit more. I propose that we
>>>>>>>>>>> set the max heap size to 768m.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> StefanK
>>>
>>
>
More information about the hotspot-jfr-dev
mailing list