RFR: Handle old, pinned regions [v5]

[Y. Srinivas Ramakrishna]

On Tue, 2 Aug 2022 00:04:58 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> Prior to this change, mixed collections were entirely ignoring the pinned status of old regions. Pinned old regions would not be coalesced and filled (which could lead to a crash) and their pin status was not being synchronized with the pin count (which could also lead to a crash). To address these problems, the following changes have been made:
>> 
>> * Before adding old regions to the collection set, their pinned status is synchronized.
>> * Pinned old regions will _not_ be added to a mixed collection, they will be skipped and considered for inclusion in the next mixed collection.
>> * Any old regions which have not been evacuated after the last mixed collection will be made parseable (coalesced and filled) _at the start of the next old marking cycle_.
>
> 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.

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?

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).

-------------

PR: https://git.openjdk.org/shenandoah/pull/149