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