PING: RFR: 8217845: SA should refer const values for JVMFlag from HotSpot

[Yasumasa Suenaga]

Thanks Jini!

Yasumasa

2019年2月5日(火) 14:18 Jini George <jini.george at oracle.com>:
>
> Looks ok to me. Thanks!
>
> -Jini.
>
> On 2/5/2019 10:25 AM, Yasumasa Suenaga wrote:
> > Hi Jini,
> >
> > Thank you for your comment!
> > I uploaded new webrev. Could you review again?
> >
> >    http://cr.openjdk.java.net/~ysuenaga/JDK-8217845/webrev.01/
> >
> > Diff from previous webrev is here:
> >
> >    http://hg.openjdk.java.net/jdk/submit/rev/6e603980ab28
> >
> >
> > Yasumasa
> >
> > 2019年2月5日(火) 12:01 Jini George <jini.george at oracle.com>:
> >>
> >> Hi Yasumasa,
> >>
> >> Thanks a bunch for this welcome change. Looks good to me overall -- a
> >> few points though.
> >>
> >> 1. Nit: alignment:
> >>     180     // See JVMFlag::print_origin() in HotSpot
> >>
> >> 2. In test/hotspot/jtreg/serviceability/sa/ClhsdbFlags.java,
> >>
> >> 156             expStrMap.put("flags",
> >> 157                           List.of("command line", "ergonomic",
> >> "default"));
> >>
> >> I think you don't need to do one more round of testing by starting
> >> LingeredApp again and issuing the 'flags' command with
> >> runFlagOriginTest(). You should be able to check for the origin strings
> >> in runBasicTest() itself. You can add "command line", "ergonomic",
> >> "default" to
> >>
> >>    66             expStrMap.put("flags", List.of(
> >>    67                     "UnlockDiagnosticVMOptions = true",
> >>    68                     "MaxFDLimit = false",
> >>    69                     "MaxJavaStackTraceDepth = 1024",
> >>    70                     "VerifyMergedCPBytecodes",
> >>    71                     "ConcGCThreads", "UseThreadPriorities",
> >>    72                     "ShowHiddenFrames"));
> >>
> >> You can have it like this:
> >>
> >>    65             Map<String, List<String>> expStrMap = new HashMap<>();
> >>    66             expStrMap.put("flags", List.of(
> >>    67                     "command line", "ergonomic", "default",
> >>    68                     "UnlockDiagnosticVMOptions = true",
> >>    69                     "MaxFDLimit = false",
> >>    70                     "MaxJavaStackTraceDepth = 1024",
> >>    71                     "VerifyMergedCPBytecodes",
> >>    72                     "ConcGCThreads", "UseThreadPriorities",
> >>    73                     "ShowHiddenFrames"));
> >>
> >> Thank you,
> >> Jini.
> >>
> >>
> >> On 2/4/2019 6:00 PM, Yasumasa Suenaga wrote:
> >>> PING: Could you review it?
> >>>
> >>>>>     JBS: https://bugs.openjdk.java.net/browse/JDK-8217845
> >>>>>     webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8217845/webrev.00/
> >>>
> >>>
> >>> Thanks,
> >>>
> >>> Yasumasa
> >>>
> >>>
> >>> On 2019/01/28 9:35, Yasumasa Suenaga wrote:
> >>>> Hi all,
> >>>>
> >>>> This change has passed tests as below (all of jhsdb related tests):
> >>>>
> >>>>    - Submit repo
> >>>>    - All hotspot/jtreg/serviceability/sa
> >>>>    - hotspot/jtreg/gc/metaspace/CompressedClassSpaceSizeInJmapHeap.java
> >>>>    - All jdk/sun/tools/jhsdb
> >>>>    - jdk/tools/launcher/HelpFlagsTest.java
> >>>>
> >>>>
> >>>> Comments are welcome.
> >>>>
> >>>>
> >>>> Thanks,
> >>>>
> >>>> Yasumasa
> >>>>
> >>>>
> >>>> On 2019/01/26 13:43, Yasumasa Suenaga wrote:
> >>>>> Hi all,
> >>>>>
> >>>>> Please review this change:
> >>>>>
> >>>>>     JBS: https://bugs.openjdk.java.net/browse/JDK-8217845
> >>>>>     webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8217845/webrev.00/
> >>>>>
> >>>>> SA will handle `Flags` enums to get flag origin.
> >>>>> However SA has const value for bitmask for flag, and shows as raw
> >>>>> (int) value.
> >>>>> This issue is commented in [1].
> >>>>>
> >>>>>
> >>>>> Thanks,
> >>>>>
> >>>>> Yasumasa
> >>>>>
> >>>>>
> >>>>> [1]
> >>>>> http://hg.openjdk.java.net/jdk/jdk/file/8c035b34248d/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/VM.java#l166
> >>>>>