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

[Jini George]

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.geo...@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