RFR: Handle old, pinned regions [v5]
William Kemper
wkemper at openjdk.org
Thu Aug 4 22:22:13 UTC 2022
On Wed, 3 Aug 2022 00:06:05 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:
>> William Kemper has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Fix whitespace
>
> src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.hpp line 47:
>
>> 45: // for pinned regions.
>> 46:
>> 47: // This points to the first candidate of the current mixed collection. This
>
> There is an implied order in first, last, etc. but the criterion for the ordering isn't explained here. (May be it is later.) I think the comment would read better if the high level idea of maintaining this ordered set of candidates was made clear at the outset, and the criteria underlying the order.
They're ordered from most to least garbage. I'll call that out explicitly in the comments.
> src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.hpp line 48:
>
>> 46:
>> 47: // This points to the first candidate of the current mixed collection. This
>> 48: // is only used for an assertion when handling pinned regions.
>
> Protect field with `#ifdef ASSERT` in that case?
Will do.
> src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.hpp line 53:
>
>> 51: // Pinned regions may not be included in the collection set. Any old regions
>> 52: // which were pinned at the time when old regions were added to the mixed
>> 53: // collection will have their pointers shifted down so that they are at the
>
> "have their pointers shifted down" is not clear without more context. Nit: Also "pointers" sounds out of place here. You can identify the region pointers with the regions for the purpose of documentation comments here.
>
> Also, I'd pick one consistent terminology, may be first and last (by index), rather than down and up, or left and right, which are more ambiguous (unless it's made clear).
s/their pointers/the pointers to them ? In my head, I was mostly thinking in terms of left (most garbage) and right (least garbage). I will update the variables and comments for consistency.
-------------
PR: https://git.openjdk.org/shenandoah/pull/149
More information about the shenandoah-dev
mailing list