Thanks Jini! Yasumasa
2019年2月5日(火) 14:18 Jini George <[email protected]>: > > 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 <[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 > >>>>>
