RFR: 8345732: Provide helpers for using PartialArrayState [v3]
Albert Mingkun Yang
ayang at openjdk.org
Tue Dec 17 15:23:43 UTC 2024
On Fri, 13 Dec 2024 22:24:07 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:
>> Please review this change that introduces two new helper classes to simplify
>> the usage of PartialArrayStates to manage splitting the processing of large
>> object arrays into parallelizable chunks. G1 and Parallel young GCs are
>> changed to use this new mechanism.
>>
>> PartialArrayTaskStats is used to collect and report statistics related to
>> array splitting. It replaces the direct implementation in PSPromotionManager,
>> and is now also used by G1 young GCs.
>>
>> PartialArraySplitter packages up most of the work involved in splitting and
>> processing tasks. It provides task allocation and release, enqueuing, chunk
>> claiming, and statistics tracking. It does this by encapsulating existing
>> objects and functionality. Using array splitting is mostly reduced to calling
>> the splitter's start function and then calling it's step function to process
>> partial states. This substantially reduces the amount of code for each client
>> to perform this work.
>>
>> Testing: mach5 tier1-5
>>
>> Manually ran some test programs with each of G1 and Parallel, with taskqueue
>> stats logging enabled, and checked that the logged statistics looked okay.
>
> Kim Barrett 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 eight additional commits since the last revision:
>
> - Merge branch 'master' into pa-splitter
> - rename splitter.step() => claim()
> - simplify comments
> - Merge branch 'master' into pa-splitter
> - parallel uses PartialArraySplitter
> - g1 uses PartialArraySplitter
> - add PartialArraySplitter
> - add PartialArrayTaskStats
src/hotspot/share/gc/shared/partialArraySplitter.hpp line 46:
> 44:
> 45: public:
> 46: explicit PartialArraySplitter(PartialArrayStateManager* manager,
Why `explicit` for a method that has two args.
src/hotspot/share/gc/shared/partialArrayTaskStats.hpp line 77:
> 75: void inc_pushed(size_t n = 1) { _pushed += n; }
> 76: void inc_stolen(size_t n = 1) { _stolen += n; }
> 77: void inc_processed(size_t n = 1) { _processed += n; }
I skimmed through callers of these, but can't find a strong reason to use default-arg-value here. Will there be more call-sites that justify this usage?
src/hotspot/share/gc/shared/partialArrayTaskStats.hpp line 96:
> 94:
> 95: template<typename StatsAccess>
> 96: void PartialArrayTaskStats::log_set(uint num_stats,
Can this be merged with its declaration? Seems kind of odd that these duplicates (method signature) are next to each other.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22622#discussion_r1888693312
PR Review Comment: https://git.openjdk.org/jdk/pull/22622#discussion_r1888684891
PR Review Comment: https://git.openjdk.org/jdk/pull/22622#discussion_r1888690919
More information about the hotspot-gc-dev
mailing list