RFR: (S): JDK-8213323: sa/TestJmapCoreMetaspace.java and sa/TestJmapCore.java fail with ZGC

[JC Beyler]

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> 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> <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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181121/2d2b91da/attachment.html>