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

[Albert Mingkun Yang]

On Tue, 21 Jan 2025 15:53:01 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 17 additional commits since the last revision:
> 
>  - Yang's comment
>  - Merge branch 'master' into JDK-8339668
>  - 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
>  - ... and 7 more: https://git.openjdk.org/jdk/compare/7db80e2d...332c1cdd

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

> 41: #include "oops/objArrayKlass.inline.hpp"
> 42: #include "oops/oop.inline.hpp"
> 43: #include "utilities/checkedCast.hpp"

Can this be removed now?

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

> 59: ParCompactionManager::ParCompactionManager(PreservedMarks* preserved_marks,
> 60:                                            ReferenceProcessor* ref_processor)
> 61:   :_partial_array_splitter(_partial_array_state_manager, ParallelScavengeHeap::heap()->workers().max_workers()),

I would probably pass `parallel_gc_threads` into the constructor of `ParCompactionManager`, instead of calling `max_workers()` directly here, in order to maintain a single source of truth. YMMV.

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

> 126:   size_t array_length = obj_array->length();
> 127:   size_t initial_chunk_size =
> 128:     // The destination array is unused when processing states.

This comment is not very helpful; I had to look into the callee to find out it's referring to `nullptr` in the args and why one can use `nullptr` here, but that process is fairly straightforward, so this comment can probably be dropped.

src/hotspot/share/gc/shared/taskqueue.hpp line 566:

> 564: // Discriminated union over oop/oop*, narrowOop*, and PartialArrayState.
> 565: // Uses a low tag in the associated pointer to identify the category.
> 566: // Oop/oop* are overloaded using the same tag because they can not appear at the

The reason why "they can not appear at the same time" is that one is for young-gc and the other full-gc, right? If so, maybe state that explicitly.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21089#discussion_r1925139586
PR Review Comment: https://git.openjdk.org/jdk/pull/21089#discussion_r1925155413
PR Review Comment: https://git.openjdk.org/jdk/pull/21089#discussion_r1925149603
PR Review Comment: https://git.openjdk.org/jdk/pull/21089#discussion_r1925151953