RFR (S) 8220350: Refactor ShenandoahHeap::initialize

Zhengyu Gu zgu at redhat.com
Fri Mar 8 19:12:14 UTC 2019



On 3/8/19 2:05 PM, Aleksey Shipilev wrote:
> On 3/8/19 7:45 PM, Zhengyu Gu wrote:
>> On 3/8/19 12:14 PM, Aleksey Shipilev wrote:
>>> Fix:
>>>     http://cr.openjdk.java.net/~shade/8220350/webrev.01/
>>
>> With large pages, there is a possibility that a single page can host multiple regions. Our region
>> based commit/uncommit might not work in this scenario. Have you tested it?
> 
> This is handled already by rounding up region size to page size in ShenandoahHeapRegion::setup_sizes
> (yes, it is awkward, but well...):
> 
>    // Make sure region size is at least one large page, if enabled.
>    // Otherwise, uncommitting one region may falsely uncommit the adjacent
>    // regions too.
>    // Also see shenandoahArguments.cpp, where it handles UseLargePages.
>    if (UseLargePages && ShenandoahUncommit) {
>      region_size = MAX2(region_size, os::large_page_size());
>    }
Ah, missed this.

> 
> I think it is tested by one of our regression tests.
> 
>> SH::initialize() still a 200 lines function, might be good to break it up, e.g. a method for each
>> section? Otherwise, looks good.

Okay. Reviewed.


-Zhengyu
> 
> Not here. I really need to push the other two bugfixes.
> 
> -Aleksey
> 


More information about the hotspot-gc-dev mailing list