On Thu, 27 Feb 2025 12:07:29 GMT, Albert Mingkun Yang <ay...@openjdk.org> wrote:
>> Thomas Schatzl has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> * remove unnecessarily added logging
>
> src/hotspot/share/gc/g1/g1ConcurrentRefine.cpp line 349:
>
>> 347:
>> 348: bool do_heap_region(G1HeapRegion* r) override {
>> 349: if (!r->is_free()) {
>
> I am a bit lost on this closure; the intention seems to set unclaimed to all
> non-free regions, why can't this be done in one go, instead of first setting
> all regions to claimed (`reset_all_claims_to_claimed`), then set non-free
> ones unclaimed?
`do_heap_region()` only visits committed regions in this case. I wanted to
avoid the additional check in the iteration code. If you still think it is more
clear to filter those out later, please tell me. I'll add a comment for now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23739#discussion_r1975250646
