RFR: JDK-8261034: improve jcmd GC.class_histogram to support parallel

[Hamlin Li]

On Wed, 10 Feb 2021 02:21:03 GMT, Lin Zang <lzang at openjdk.org> wrote:

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

Hi Lin,
Sure, let's discuss -histo in this thread first.

Hi @sspitsyn @plummercj @linzang 
IMHO, maybe we could make the option simple and automatically speed up heap scan by disabling "parallel=" option and adding "noparallel" at the same time? From user's point of view, "noparallel" will speed up their histo/dump automatically, if they are migrating from jdk15 or prior to jdk15, and also give them a way to fall back to previous serial scan if they would like to do that.
Although This way will introduce the compability issue, fortunately JDK16 is not LTS, and maybe we could backport to 16 updates if necessary?
How do you think about it? Thanks for discussion.

( BTW, I might not be able to response timely, as it will be long public holiday the next few days, I'm sorry for the inconvenience. )

-------------

PR: https://git.openjdk.java.net/jdk/pull/2379