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