RFR: 8364423: G1: Refactor G1UpdateRegionLivenessAndSelectForRebuildTask
Albert Mingkun Yang
ayang at openjdk.org
Thu Jul 31 15:37:55 UTC 2025
On Thu, 31 Jul 2025 15:23:13 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:
>> src/hotspot/share/gc/g1/g1ConcurrentMark.cpp line 1262:
>>
>>> 1260: }
>>> 1261: _cm->update_top_at_rebuild_start(hr);
>>> 1262: }
>>
>> I feel this logic is too caller-specific -- reading its name `update_live_region` doesn't reveal much what this method should do/deliver.
>
> Since it is the same, and the only per-region work to do for all callers, not sure what is caller-specific here. Maybe change the name to something like `update_for_rebuild` is more appropriate? Or just leave it as is until more common work needs to be done?
Usually, it's the method spec that determines its impl, because a method doesn't know all the callers. However, give its name + signature, it's unclear what the impl should be -- instead, it's determined by its caller context (select-for-rebuild), hence, caller-specific.
> Or just leave it as is until more common work needs to be done?
Maybe by then some common logic can be extracted into a self-contained method.
YMMV.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26574#discussion_r2245738251
More information about the hotspot-gc-dev
mailing list