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