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

stefan.johansson at oracle.com stefan.johansson at oracle.com
Sun May 3 13:38:57 UTC 2020


Hi Kim,

On 2020-05-03 10:10, Kim Barrett wrote:
>> On Apr 29, 2020, at 8:50 AM, stefan.johansson at oracle.com wrote:
>>
>> Full: http://cr.openjdk.java.net/~sjohanss/8233439/01/
>> Inc: http://cr.openjdk.java.net/~sjohanss/8233439/00-01/
> 
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/g1/g1RegionToSpaceMapper.cpp
>   113     // At least one bit must be set if the page is committed.
>   114     return _region_commit_map.count_one_bits(region, region + _regions_per_page) > 0;
> 
> Instead of counting 1-bits, this could search for one and return true
> if found.
I like it!

> 
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/g1/g1RegionToSpaceMapper.cpp
>   139   virtual void commit_regions(uint start_idx, size_t num_regions, WorkGang* pretouch_gang) {
>   140     assert(_region_commit_map.count_one_bits(start_idx, start_idx + num_regions) == 0,
> 
> Rather than counting 1-bits, this could search for 1-bits and complain
> if there are any.
> 
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/g1/g1RegionToSpaceMapper.cpp
>   184   virtual void uncommit_regions(uint start_idx, size_t num_regions) {
>   185     assert(_region_commit_map.count_one_bits(start_idx, start_idx + num_regions) == num_regions,
> 
> Rather than counting 1-bits, this could search for 0-bits and complain
> if there are any.
Changed the two asserts as well, was a bit afraid it wouldn't be as easy 
to read and understand, but I think it is ok.

> 
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/g1/g1RegionToSpaceMapper.cpp
>   184   virtual void uncommit_regions(uint start_idx, size_t num_regions) {
> 
> I think the new algorithm expects num_regions > 0, else the end_page
> calculation could underflow; consider adding an assertion for that.
> 
> Similarly in commit_regions.
There are assertion for num_regions in the callers, but I just realized 
there is a test calling directly to these functions so I'll add the asserts.
> 
> ------------------------------------------------------------------------------
> 

Also fixed two spelling errors, here are the new webrevs.
Full: http://cr.openjdk.java.net/~sjohanss/8233439/02
Inc: http://cr.openjdk.java.net/~sjohanss/8233439/01-02

Thanks for reviewing,
Stefan





More information about the hotspot-gc-dev mailing list