On Wed, 10 Feb 2021 01:49:02 GMT, Hamlin Li <[email protected]> wrote:
>> Hi Chris, >> >>> * Default to not parallel. Allow the user to specify `-parallel`, but not >>> specify the thread count. They will get the default number of threads just >>> like `parallel=0` >>> * Default to parallel using the default number of threads. Allow the user >>> to turn parallel off with a `-noparallel` option. >> >> I would prefer the -noparallel option that enable parallel by default. IMHO, >> the parallel heap inspection is a little bit like parallelGCThreads - which >> allowing the hotspot to decide the parallel thread number based on the >> platfrom by default. >> >> And I remember Serguei (@sspitsyn) and I discussed the "-parallel" option at >> https://mail.openjdk.java.net/pipermail/serviceability-dev/2020-September/032836.html, >> we finally decide to not introduce it. just FYI. >> >>> However, this probably needs to be consistent with the jmap command and the >>> attach API protocol, which is being discussed in #2261. >> >> if just introducing new option like "parallel" or "noparallel", I think we >> don't need to bother about attach API protocol. we can preprocess it in >> JMap.java and then pass the value of "0" or "1" as parallel_thread_number, >> just like what it is implemented at present, no compatibility issue >> introduced :-) >> >> >> Moreover, I am thinking maybe we could print a meesage like "inspected heap >> parallelly with `parallel_numer` threads" in the result, just like -dump's >> output "Heap dump file created [18904360 bytes in 0.039 secs]" >> >> Thanks, >> Lin > >> > * Default to not parallel. Allow the user to specify `-parallel`, but not >> > specify the thread count. They will get the default number of threads just >> > like `parallel=0` >> > * Default to parallel using the default number of threads. Allow the user >> > to turn parallel off with a `-noparallel` option. >> >> I would prefer the -noparallel option that enable parallel by default. IMHO, >> the parallel heap inspection is a little bit like parallelGCThreads - which >> allowing the hotspot to decide the parallel thread number based on the >> platfrom by default. >> > > Hi Chris, Lin, > Thanks for discussion. > I'm OK with either options, both ways simplify the spec and hide details from > users. > >> And I remember Serguei (@sspitsyn) and I discussed the "-parallel" option at >> https://mail.openjdk.java.net/pipermail/serviceability-dev/2020-September/032836.html, >> we finally decide to not introduce it. just FYI. >> >> > However, this probably needs to be consistent with the jmap command and >> > the attach API protocol, which is being discussed in #2261. >> >> if just introducing new option like "parallel" or "noparallel", I think we >> don't need to bother about attach API protocol. we can preprocess it in >> JMap.java and then pass the value of "0" or "1" as parallel_thread_number, >> just like what it is implemented at present, no compatibility issue >> introduced :-) > > If we decide to take either of above ways, then we should modify the uint to > bool, it's more clear. > >> >> Moreover, I am thinking maybe we could print a meesage like "inspected heap >> parallelly with `parallel_numer` threads" in the result, just like -dump's >> output "Heap dump file created [18904360 bytes in 0.039 secs]" > > Agree, it helps. > > BTW, I think we have mixed several threads(jcmd, jmap histo/dump) together. > How about we address jmap -histo first (I filed a new bug at > https://bugs.openjdk.java.net/browse/JDK-8261482), then I adjust the behavior > of jcmd accordingly? > > Thanks. Hi Hamlin, > If we decide to take either of above ways, then we should modify the uint to > bool, it's more clear. Sorry that I am a little confuse about it: - Does it mean to disable "parallel=" option to not let user tuning the parallel thread number? - Or just add a "parallel" or "noparallel" option to change the default behavior? Both looks fine with me. But IMHO, regardless the ablility of "tunning", disabling the "parallel=" option maybe cause more backward ability issue. User of JDK16 may get error when using option like "parallel=1". Although I am not sure how this is important since the parallel options was just introduced to JDK16 and it is not LTS. BTW, I am not sure whether adding "parallel" requre a new CSR as there already have "parallel=" option. And seems like adding "noparallel" require one, right? > BTW, I think we have mixed several threads(jcmd, jmap histo/dump) together. > How about we address jmap -histo first (I filed a new bug at > https://bugs.openjdk.java.net/browse/JDK-8261482), then I adjust the behavior > of jcmd accordingly? I agree, we could start with -histo first, maybe we can discuss in this thread first and then handle it in new PR? Thanks, Lin ------------- PR: https://git.openjdk.java.net/jdk/pull/2379
