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

Thomas Schatzl tschatzl at openjdk.org
Wed Oct 2 09:29:42 UTC 2024


On Tue, 1 Oct 2024 12:48:40 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Zhengyu Gu has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - @tschatzl's comment
>>  - v8
>
> 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.

Here's a commit that only touches up the worst issues in this change: https://github.com/openjdk/jdk/commit/ab6e77ed909c13458a24e9663e830cfac9c4f18e

(branch including the other suggestions so far: https://github.com/openjdk/jdk/compare/master...tschatzl:jdk:pull/21089-taskqueue-refactor)

This makes the change a lot more readable imo (and in total reduces LOC count).

The only missing improvement is to have the array statistics in the same log lines as the other statistics; that could be achieved by specializing the queues/queue set, but that requires the unified `ScannerTask` in the other change and more work.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21089#discussion_r1784112754


More information about the hotspot-gc-dev mailing list