RFR (M): 8226913: Scale cards per chunk used during heap root scanning with region

Kim Barrett kim.barrett at oracle.com
Tue Jul 16 16:35:54 UTC 2019


> On Jul 16, 2019, at 4:49 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> On Sat, 2019-07-13 at 19:59 -0400, Kim Barrett wrote:
>> src/hotspot/share/gc/g1/g1RemSet.cpp
>> 103     assert(log_region_size >= 20 && log_region_size <= 25,
>> 104            "expected value in [20,25], but got %u",
>> log_region_size);
>> 
>> This is hard-coded to the current region size range limits, and
>> doesn't seem to be related to any requirements of the calcuation
>> being done. Any future expansion of the permitted region size range
>> would fail here for no immediately obvious reason. The only reason to
>> choose those bounds here seems to be that those are what's actually
>> been tested, so as to not make any assumptions about what might be
>> good outside that range.
> 
> That is the reason - if you do not mind I would prefer the code to fail
> and the person changing this think about this code once more (and do at
> least cursory testing).
> I added a comment indicating that.

The comment is good and sufficient.

> If you feel strongly about this, I can remove the assert completely -
> maybe at least give a "reasonable" lower bound like 2^16 or so (so that
> the calculation does not give ridiculously low values or even
> underflow).
> 
> […]
> 
> New webrevs:
> http://cr.openjdk.java.net/~tschatzl/8226913/webrev.0_to_1
> 
> http://cr.openjdk.java.net/~tschatzl/8226913/webrev.1
> 
> Thanks a lot,
>  Thomas

Looks good.




More information about the hotspot-gc-dev mailing list