RFR: 8233439 : G1 zero_filled optimization when committing CardCountsTable does not work

[stefan.johansson at oracle.com]

Hi Thomas,

On 2020-04-29 12:38, Thomas Schatzl wrote:
> 
> Hi,
> 
> On 29.04.20 11:32, stefan.johansson at oracle.com wrote:
>> Hi,
>>
>> Please review this fix to clear memory correctly when committing regions.
>>
>> Webrev: http://cr.openjdk.java.net/~sjohanss/8233439/00/
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8233439
>>
>> Summary
>> The old code always set all_zero_filled to false if more than one 
>> region belonging to the same page was committed. This leads to 
>> unnecessary clearing of memory in some data structures.
>>
>> While fixing all_zero_filled I also cleaned up the code a bit more to 
>> get rid of the refcount array and instead use the commit bitmap to 
>> check if pages are already committed.
>>
>> Testing
>> Local testing looks good, currently running mach5 1-3. Local tests 
>> also show a slight improvement in startup.
>>
>> Thanks,
>> Stefan
> 
> Some comments:
> 
> - I would prefer if the parameter to region_idx_to_page_idx() would be a 
> uint as any other region index in g1 code.
> 
Fixed.

> - is_committed(page) is rather confusing, as talked about I would prefer 
> is_page_committed() and renaming the _commit_map to _region_commit_map 
> to make it more clear that there is a difference in what they ask/are for.
What's even worse is that when fixing this I found that the base-class 
already had an unused is_committed(), which just did _commit_map.at(). 
Removed it.

> 
> - in line 154 and 196 there should be a space after the "for"
> 
> - the "private" in line 105 is superfluous (pre-existing)

Fixed, here are new webrevs.

Full: http://cr.openjdk.java.net/~sjohanss/8233439/01/
Inc: http://cr.openjdk.java.net/~sjohanss/8233439/00-01/

Thanks for the review,
Stefan

> 
> Thanks,
>    Thomas