RFR: 8319373: Serial: Refactor dirty cards scanning during Young GC [v12]

[Kim Barrett]

On Tue, 28 Nov 2023 15:57:53 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> Reading the new code directly is probably easier. The structure is quite similar to its counterpart in Parallel.
>> 
>> It's mostly perf-neutral, except when dirty cards are scarce. Using `card_scan.java` in [JDK-8310031](https://bugs.openjdk.org/browse/JDK-8310031), I observed ~40% reduction in young-gc pause time with `stride = 32 * 64`.
>
> Albert Mingkun Yang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review

Changes requested by kbarrett (Reviewer).

src/hotspot/share/gc/serial/cardTableRS.hpp line 57:

> 55: 
> 56:   static CardValue* find_first_dirty_card(CardValue* const start_card,
> 57:                                           CardValue* const end_card);

const qualifiers are noise here, having no effect.  Similarly in find_first_clean_card.  (And why isn't ct
similarly qualified there?)  The second const qualifiers in is_dirty/clean don't add much, and it's not
typical usage, but at least they have an effect.

The const qualifiers in the implementations of find_first_xxx are fine, its an implementation detail
that those variables don't get modified.  That implementation detail doesn't belong here though.

src/hotspot/share/gc/serial/cardTableRS.hpp line 58:

> 56:   static CardValue* find_first_dirty_card(CardValue* const start_card,
> 57:                                           CardValue* const end_card);
> 58:   template<typename Func>

Please add a blank line between these two function declarations.

src/hotspot/share/gc/serial/cardTableRS.hpp line 62:

> 60:                                    CardValue* const end_card,
> 61:                                    CardTableRS* ct,
> 62:                                    Func&& object_start);

Neither rvalue references nor universal references are permitted in HotSpot code (yet).  Usual style is to
take function arguments by value.  The copy may not even matter (may be elided/optimized away), given
this function is called once and potentially inlined.  Maybe declare this function inline to encourage that?
I'm waffling over rejecting this, since I think we ought to at least permit universal references, but that
hasn't been proposed yet.

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

PR Review: https://git.openjdk.org/jdk/pull/16492#pullrequestreview-1753267658
PR Review Comment: https://git.openjdk.org/jdk/pull/16492#discussion_r1408000710
PR Review Comment: https://git.openjdk.org/jdk/pull/16492#discussion_r1407999822
PR Review Comment: https://git.openjdk.org/jdk/pull/16492#discussion_r1408087620