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 <[email protected]>: > > 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 > >>>
