RFR: 8272611: Use parallel heap inspection when print classhisto info in gc log

Thomas Schatzl tschatzl at openjdk.java.net
Mon Aug 23 10:57:25 UTC 2021


On Wed, 18 Aug 2021 07:35:50 GMT, Bin Liao <github.com+3094961+buddyliao at openjdk.org> wrote:

> This patch helps to enhance the heap inspection with parallel threads when print classhisto info in gc log.
> 
> I have built it on linux x86_64 and run test-tier1 successfully
> 
> ---------
> ### Progress
> - [x] Change must not contain extraneous whitespace
> - [x] Commit message must refer to an issue
> - [ ] Change must be properly reviewed
> 
> 
> 
> ### Reviewing
> <details><summary>Using <code>git</code></summary>
> 
> Checkout this PR locally: \
> `$ git fetch https://git.openjdk.java.net/jdk pull/5158/head:pull/5158` \
> `$ git checkout pull/5158`
> 
> Update a local copy of the PR: \
> `$ git checkout pull/5158` \
> `$ git pull https://git.openjdk.java.net/jdk pull/5158/head`
> 
> </details>
> <details><summary>Using Skara CLI tools</summary>
> 
> Checkout this PR locally: \
> `$ git pr checkout 5158`
> 
> View PR using the GUI difftool: \
> `$ git pr show -t 5158`
> 
> </details>
> <details><summary>Using diff file</summary>
> 
> Download this PR as a diff file: \
> <a href="https://git.openjdk.java.net/jdk/pull/5158.diff">https://git.openjdk.java.net/jdk/pull/5158.diff</a>
> 
> </details>

Changes requested by tschatzl (Reviewer).

src/hotspot/share/gc/shared/collectedHeap.cpp line 551:

> 549:     ResourceMark rm;
> 550:     LogStream ls(lt);
> 551:     uint parallel_thread_num = MAX2<uint>(1, (uint)os::initial_active_processor_count() * 3 / 8);

Where do the magic numbers for the formula come from? Please consider using the ergonomically determined number of threads (`CollectedHeap::safepoint_workers()->active_workers()`)? (Maybe change this in another CR as other reviewers think is better, looking whether there is a significant difference between these numbers)

src/hotspot/share/gc/shared/collectedHeap.cpp line 552:

> 550:     LogStream ls(lt);
> 551:     uint parallel_thread_num = MAX2<uint>(1, (uint)os::initial_active_processor_count() * 3 / 8);
> 552:     VM_GC_HeapInspection inspector(&ls, false /* ! full gc */, parallel_thread_num);

Pre-existing: the comment next to a bool constant in a parameter list should show the name of the parameter, i.e. s/! full_gc/request_full_gc/. Maybe change this as appropriate in other callers too.

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

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



More information about the hotspot-gc-dev mailing list