PING: RFR: 8217845: SA should refer const values for JVMFlag from HotSpot
Jini George
jini.george at oracle.com
Tue Feb 5 05:18:41 UTC 2019
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
>>>>>
More information about the serviceability-dev
mailing list