RFR: (S): JDK-8213323: sa/TestJmapCoreMetaspace.java and sa/TestJmapCore.java fail with ZGC
Jini George
jini.george at oracle.com
Tue Dec 4 04:30:01 UTC 2018
Thank you, Chris!
- Jini
On 12/4/2018 9:29 AM, Chris Plummer wrote:
> Looks good.
>
> Chris
>
> On 12/2/18 8:36 PM, Jini George wrote:
>> Gentle reminder !
>>
>> Thanks,
>> Jini
>>
>> On 11/29/2018 12:03 PM, Jini George wrote:
>>> Thank you very much, Chris. I have modified HeapHprofBinWriter.java
>>> slightly so that the hprof file does not dumped if this is ZGC. In
>>> the test, we check for the presence of the hprof file and throw an
>>> exception if the hprof file does not exist and only if this is not
>>> ZGC. The tests are not excluded for zgc now.
>>>
>>> The modified webrev is at:
>>>
>>> http://cr.openjdk.java.net/~jgeorge/8213323/webrev.02/index.html
>>>
>>> Thanks!
>>> Jini.
>>>
>>>
>>> On 11/27/2018 2:51 AM, Chris Plummer wrote:
>>>> Hi Jini,
>>>>
>>>> The tool changes look fine for disabling zgc support, but with the
>>>> test changes you are no longer testing what happens if you use jmap
>>>> with zgc. What would the tests do if you made no test changes? If
>>>> they still fail ungracefully, could they be fixed by catching the
>>>> RuntimeException you are now throwing? Maybe you could test that
>>>> this only happens when using zgc.
>>>>
>>>> thanks,
>>>>
>>>> Chris
>>>>
>>>> On 11/21/18 7:49 PM, Jini George wrote:
>>>>> Thanks a bunch, JC ! Could have a Reviewer also take a look at this
>>>>> please ?
>>>>>
>>>>> - Jini.
>>>>>
>>>>> On 11/21/2018 11:05 PM, JC Beyler wrote:
>>>>>> Hi Jini,
>>>>>>
>>>>>> Looks good to me, thanks for fixing the nits :)
>>>>>>
>>>>>> Thanks,
>>>>>> Jc
>>>>>>
>>>>>>
>>>>>> On Wed, Nov 21, 2018 at 9:29 AM Jini George
>>>>>> <jini.george at oracle.com <mailto:jini.george at oracle.com>> wrote:
>>>>>>
>>>>>> The modified webrev with the RuntimeException and the nit
>>>>>> removed is at:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~jgeorge/8213323/webrev.01/
>>>>>>
>>>>>> Thanks!
>>>>>> Jini.
>>>>>>
>>>>>> On 11/12/2018 12:41 PM, Jini George wrote:
>>>>>> > Thank you very much, JC, for looking into this.
>>>>>> >
>>>>>> >
>>>>>> http://mail.openjdk.java.net/pipermail/zgc-dev/2018-August/000455.html
>>>>>>
>>>>>> >
>>>>>> > The link above provides an explanation of why it is
>>>>>> difficult to
>>>>>> support
>>>>>> > ZGC with SA where there is an iteration over the heap. And
>>>>>> currently, I
>>>>>> > am unsure as to whether we will devise a way to overcome this
>>>>>> issue. We
>>>>>> > currently have the following JBS entry for tracking this.
>>>>>> >
>>>>>> > https://bugs.openjdk.java.net/browse/JDK-8207843 (SA: Add
>>>>>> support
>>>>>> for
>>>>>> > Object Histogram with ZGC)
>>>>>> >
>>>>>> > I have added a comment in the ER above to keep track of
>>>>>> enabling the
>>>>>> > tests if this is resolved.
>>>>>> >
>>>>>> > I agree with your comment on throwing a RuntimeException.
>>>>>> Doing this
>>>>>> > avoids the misleading "heap written to" message. I will modify
>>>>>> this and
>>>>>> > remove the ';' you pointed out and post another webrev.
>>>>>> >
>>>>>> > Thank you,
>>>>>> > Jini.
>>>>>> >
>>>>>> >
>>>>>> >
>>>>>> > On 11/9/2018 9:36 PM, JC Beyler wrote:
>>>>>> >> Hi Jini,
>>>>>> >>
>>>>>> >> The webrev looks good to me as well except for a few
>>>>>> questions/comments:
>>>>>> >>
>>>>>> >> I have a generic question that is related to the webrev:
>>>>>> >> - Are there plans at some point for Jmap to support ZGC
>>>>>> heaps in
>>>>>> >> the future ? Or is this infeasible?
>>>>>> >> I ask because if a lot of tests are disabled for
>>>>>> ZGC for a
>>>>>> >> limited amount of time (in order to provide time for future
>>>>>> support)
>>>>>> >> there should be a means to scrub the tests at a later date
>>>>>> to see
>>>>>> >> which are now supported, no?
>>>>>> >>
>>>>>> >> - In
>>>>>> >>
>>>>>> http://cr.openjdk.java.net/~jgeorge/8213323/webrev.00/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java.html:
>>>>>>
>>>>>>
>>>>>> >>
>>>>>> >>
>>>>>> >> 397 if (vm.getUniverse().heap() instanceof
>>>>>> ZCollectedHeap) {
>>>>>> >> 398 System.err.println("WARNING: Operation not
>>>>>> supported
>>>>>> >> for ZGC.");
>>>>>> >> 399 return;
>>>>>> >> 400 };
>>>>>> >>
>>>>>> >> - 1 nit is the ';' after the closing '}'
>>>>>> >> - Should the code throw a RuntimeException instead?
>>>>>> Just
>>>>>> >> printing an error seems "weak" for me when this really won't
>>>>>> work. Of
>>>>>> >> course, that means tracking the callers now of write and
>>>>>> probably
>>>>>> >> adding exception handling there and I'm not sure that is
>>>>>> something
>>>>>> >> that is warranted here but thought I would ask :)
>>>>>> >>
>>>>>> >> Thanks!
>>>>>> >> Jc
>>>>>> >>
>>>>>> >>
>>>>>> >>
>>>>>> >> Thanks,
>>>>>> >> Jc
>>>>>> >>
>>>>>> >> On Fri, Nov 9, 2018 at 1:47 AM gary.adams at oracle.com
>>>>>> <mailto:gary.adams at oracle.com>
>>>>>> >> <mailto:gary.adams at oracle.com <mailto:gary.adams at oracle.com>>
>>>>>> <gary.adams at oracle.com <mailto:gary.adams at oracle.com>
>>>>>> >> <mailto:gary.adams at oracle.com
>>>>>> <mailto:gary.adams at oracle.com>>>
>>>>>> wrote:
>>>>>> >>
>>>>>> >> Looks OK to me.
>>>>>> >>
>>>>>> >> On 11/9/18 1:40 AM, Jini George wrote:
>>>>>> >> > Hello!
>>>>>> >> >
>>>>>> >> > Requesting reviews for a small change for disabling
>>>>>> the
>>>>>> testing of
>>>>>> >> > TestJmapCoreMetaspace.java and TestJmapCore.java with
>>>>>> ZGC. The
>>>>>> >> change
>>>>>> >> > also includes skipping of creating the heapdump
>>>>>> file with
>>>>>> jmap if
>>>>>> >> the
>>>>>> >> > GC being used is ZGC.
>>>>>> >> >
>>>>>> >> > Webrev:
>>>>>> http://cr.openjdk.java.net/~jgeorge/8213323/webrev.00/
>>>>>> >> > Bug ID:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8213323
>>>>>> >> >
>>>>>> >> > Thanks,
>>>>>> >> > Jini.
>>>>>> >>
>>>>>> >>
>>>>>> >>
>>>>>> >>
>>>>>> >> --
>>>>>> >>
>>>>>> >> Thanks,
>>>>>> >> Jc
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>>
>>>>>> Thanks,
>>>>>> Jc
>>>>
>>>>
>
More information about the serviceability-dev
mailing list