RFR (S): 8134631: G1DummyRegionsPerGC fires assert of assert(words <= filler_array_max_size()) failed: too big for a single object

[sangheon.kim]

Hi Tom,

Thank you for looking at this!

On 11/09/2015 01:30 PM, Tom Benson wrote:
> Hi
> On 11/9/2015 4:10 PM, Tom Benson wrote:
>> Hi Sangheon,
>> Rather than changing all the product-version uses, couldn't you 
>> change _filler_array_max_size to _humongous_object_threshold_in_words 
>> around the special debug-only call to fill_with_object instead?
>
> Whoops - Meant to say... change it to HeapRegion::GrainWords. This 
> (testing-only, debug) code already assumes fill_with_object can handle 
> an object that large.
> Tom
>
>>   I think it would be better to have the special-case hack there in 
>> the testing code (only used when the G1DummyRegionsPerGC option is 
>> specified). Then you don't need the new alternate entry point and 
>> only change g1CollectedHeap.cpp.
Well, we can hack here. :)
And I also have a concern of whether is it worth to change codes for 
debug only test option or not.
However, I thought there's no reason of adding size limitation for G1 
filler.
We are using non-humongous fillers for now but it would be nice to have 
humongous filler as well.

>>
>> I'm not sure the objects allocate_dummy_regions creates really MUST 
>> be humongous - its stated purpose is just to "artificially expand the 
>> heap by allocating a number of dead regions."   But it would take 
>> more changes to allocate_dummy_regions to allocate regions and then 
>> fill with non-humongous objects, possibly not worth it for this case.
Yes, we would write filler with non-humongous objects but as we already 
have CollectedHeap::fill_with_xxxx(), it would be better utilizing them.

Thanks,
Sangheon


>>
>> Tom
>>
>> On 11/9/2015 2:28 PM, sangheon.kim wrote:
>>> Hi all,
>>>
>>> Can I get some reviews for this change for fixing this assert?
>>>
>>> Previous patch(8042668: GC Support for shared heap ranges in CDS) 
>>> limited _filler_array_max_size of G1CollectedHeap to be less than 
>>> humongous threshold.
>>> And this change led this assert as this flag is trying to fill the 
>>> rest of regions with humongous object.
>>>
>>> As a fix, I removed the limitation to allow humongous filler and 
>>> added a wrapper function, fill_with_non_humongous_objects().
>>> And replace it with current fill_with_object().
>>>
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8134631
>>> Webrev:
>>> http://cr.openjdk.java.net/~sangheki/8134631/webrev.00/
>>> Test:
>>> JPRT
>>>
>>> Thanks,
>>> Sangheon
>>>
>>
>