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

[Lin Zang]

On Tue, 9 Feb 2021 19:13:55 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>>> Hi @Hamlin-Li,
>>> 
>>> It looks good in general.
>>> A question: What is going to happen if the number of parallel threads passed to the command is too big? Is there any limit, and what has to be the expected behavior in such a case? I'll look at the CSR.
>>> Thanks,
>>> Serguei
>> 
>> Hi Serguer,
>> Thanks for reviewing.
>> First, it got truncated as uint by ClassHistogramDCmd, then it got capped in HeapInspection by by total_workers of gang, so finally it will not exceed total_workers.
>> Hope this answer your questions.
>
> I think part of the concern here is that this is all hidden from the user. "total_workers of gang" is not something that is spec'd somewhere. So when the user chooses to set "parallel" to something other than 0 (the default), then there really is no guidance as to what a good alternative number is. Presumably you are setting it because you feel the default is too low or too high, but without knowing the actual number of threads used by default, it's hard to come up with a reasonable adjustment to specify. We also have the relationship to `-XX:ParallelGCThreads`, which impacts this.
> 
> Maybe the option should just be to enable or disable parallel. Before your changes it looks like you got parallel=1, so no parallel threads, and no ability to change that. With your changes the default is now parallel=0, so you get the default number of threads, and you can change it to any number. So maybe we should choose one of the following two simpler approaches:
> 
> - 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.
> 
> However, this probably needs to be consistent with the jmap command and the attach API protocol, which is being discussed in  #2261. They should both either choose one of the above simplified approaches, or do a better job of specifying defaults and limits for the number of threads. So some input from @linzang woudl be useful here.

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

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

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