RFR: JDK-8263495: Gather liveness info in the mark phase of G1 full gc [v3]

[Hamlin Li]

On Tue, 16 Mar 2021 02:03:01 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/g1RegionMarkStatsCache.cpp line 49:
>> 
>>> 47: 
>>> 48: // cache size is equal to or bigger than region size to intialize region_index
>>> 49: void G1RegionMarkStatsCache::initialize() {
>> 
>> If the change really wants to avoid the initial cache misses (for regions != 0), please merge this with the existing `reset()` method. I do not think it makes a difference to have this optimization that exploits the simplicity of `G1RegionMarkStatsCacheEntry::hash()`.
>> (And if the `hash()` implementation ever changes, this optimization won't work any more).
>> 
>> If you do so, also add a clear comment why the cache is initialized with these values. The comment
>> 
>>     // cache size is equal to or bigger than region size to intialize region_index
>> 
>> seems at least misplaced.
>> 
>> The change could, at the assignment (maybe even add an optional parameter to `clear()`) state:
>> 
>>     // Avoid the initial cache miss and eviction by setting the i'th's cache region_idx to the region_idx due to how the hash is calculated.
>> 
>> Note that resetting to constant values in that loop will likely be folded into a `memset`, while this loop needs to be translated as is, and may be a lot slower (with 100k+ regions...) in the pause.
>> 
>> In a textbook implementation, `G1RegionMarkStatsCacheEntry::clear()` should probably set the region index to `G1_NO_HRM_INDEX` (i.e. (uint)-1) to indicate emptiness so that all initial cache lookups fail (i.e. evict the previous, non-existing value). But then the simple implementation of the `G1RegionMarkStatsCache::evict` method will access the liveness array out of bounds... (that's probably why the initial `region_idx` values are zero, not as some sort of optimization - we save some conditional in that method).
>
> Thanks Thomas, I will merge reset and initialize, and add the comment you suggested.

BTW, a quick question: is there a limit on the region number in G1?

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

PR: https://git.openjdk.java.net/jdk/pull/2966