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