RFR: 8339668: Parallel: Adopt PartialArrayState to consolidate marking stack in compact GC

[Thomas Schatzl]

On Thu, 19 Sep 2024 14:01:39 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.

Changes requested by tschatzl (Reviewer).

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

> 31: #include "gc/parallel/psParallelCompact.inline.hpp"
> 32: #include "gc/shared/partialArrayState.hpp"
> 33: 

Suggestion:



Unnecessary newline.

src/hotspot/share/gc/parallel/psCompactionManager.hpp line 90:

> 88:   bool is_partial_array_state() const { return ((uintptr_t)_holder & PartialArrayStateBit) != 0; }
> 89: };
> 90: 

Please use `ScannerTask` instead; it seems to be completely serviceable for that purpose. In fact, a search&replace seems just fine.

https://github.com/openjdk/jdk/compare/pr/21089...tschatzl:jdk:pull/21089-recommendations?expand=1

I am going to experiment with refactoring the other duplicated (statistics) code

src/hotspot/share/gc/parallel/psCompactionManager.hpp line 122:

> 120:   size_t                        _arrays_chunked;
> 121:   size_t                        _array_chunks_processed;
> 122: #endif // TASKQUEUE_STATS

This is a separate issue, but we did not add these counters when doing this change for g1 young gen. Filed [JDK-8341331](https://bugs.openjdk.org/browse/JDK-8341331) for that.

There is a fair amount of code duplication (definition of members, management and printing) which is not great (but you mentioned it). For now I filed [JDK-8341332](https://bugs.openjdk.org/browse/JDK-8341332) for this, but I would really prefer some refactoring in this area in _this_ change.

Initially I was kind of okay to do that separately, but the copy&paste seems too much.

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

PR Review: https://git.openjdk.org/jdk/pull/21089#pullrequestreview-2339670822
PR Review Comment: https://git.openjdk.org/jdk/pull/21089#discussion_r1782454836
PR Review Comment: https://git.openjdk.org/jdk/pull/21089#discussion_r1782794766
PR Review Comment: https://git.openjdk.org/jdk/pull/21089#discussion_r1782710439