RFR: 8339668: Parallel: Adopt PartialArrayState to consolidate marking stack in Full GC [v4]

Albert Mingkun Yang ayang at openjdk.org
Mon Jan 20 13:27:45 UTC 2025


On Fri, 3 Jan 2025 21:48:47 GMT, Zhengyu Gu <zgu at openjdk.org> wrote:

>> Please review this patch that adopts `PartialArrayState`introduced by [JDK-8337709](https://bugs.openjdk.org/browse/JDK-8337709) to consolidate `_oop_task_queues` and `_objarray_task_queues` into single `_marking_stacks`.
>> 
>> The change mirrors Kim's [JDK-8311163](https://bugs.openjdk.org/browse/JDK-8311163) work, therefore, there are methods can be consolidated and simplified, but I would like defer to a followup CR.
>
> Zhengyu Gu has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 15 additional commits since the last revision:
> 
>  - Adopt latest PartialArrayStats changes
>  - Merge branch 'master' into JDK-8339668
>  - Merge branch 'master' into JDK-8339668
>  - @tschatzl's ScannerTask changes
>  - @tschatzl's comment
>  - v8
>  - v7
>  - v6
>  - v5
>  - v4
>  - ... and 5 more: https://git.openjdk.org/jdk/compare/e753605f...0ed1a358

Could you also sync the PR with master?

src/hotspot/share/gc/parallel/psCompactionManager.cpp line 62:

> 60: ParCompactionManager::ParCompactionManager(PreservedMarks* preserved_marks,
> 61:                                            ReferenceProcessor* ref_processor)
> 62:   :_partial_array_splitter(_partial_array_state_manager, ParallelGCThreads),

I feel it's best to avoid using `ParallelGCThreads` directly; instead, pass in `parallel_gc_threads` in `new ParCompactionManager` below. WDYT?

src/hotspot/share/gc/parallel/psCompactionManager.cpp line 131:

> 129:     // The destination array is unused when processing states.
> 130:     _partial_array_splitter.start(&_marking_stack, obj_array, nullptr, array_length);
> 131:   int end = checked_cast<int>(initial_chunk_size);

I wonder if we can change the callee so that we don't need `checked_cast` here. (Intuitively, [start, end) denotes a range and both number should never be negative.)

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

PR Review: https://git.openjdk.org/jdk/pull/21089#pullrequestreview-2562355266
PR Review Comment: https://git.openjdk.org/jdk/pull/21089#discussion_r1922381325
PR Review Comment: https://git.openjdk.org/jdk/pull/21089#discussion_r1922400625


More information about the hotspot-gc-dev mailing list