RFR: 8033426: Scale initial NewSize using NewRatio if not set on command line
Stefan Johansson
stefan.johansson at oracle.com
Mon Feb 10 15:56:59 UTC 2014
Thanks for looking at this Jon,
On 2014-02-08 02:44, Jon Masamitsu wrote:
> Stefan,
>
>
> 1043 static void verify_scaled_initial(size_t initial_heap_size) {
> 1044 MarkSweepPolicy msp;
> 1045 msp.initialize_all();
> 1046
> 1047 size_t expected =
> msp.scale_by_NewRatio_aligned(initial_heap_size);
> 1048 assert(msp.initial_gen0_size() == expected, err_msg("%zu !=
> %zu", msp.initial_gen0_size(), expected));
> 1049 }
>
>
>
> Does the assertion work if you added a check that NewSize was set
> ergonomically?
>
> assert(!FLAG_IS_ERGO(NewSize) || ...)
>
I like the idea to assert on NewSize being updated, but where
verify_scaled_initial is used I use FLAG_SET_ERGO to set the flag. So
even if the value is changed, FLAG_SET_ERGO will be used to update it.
To verify that NewSize was updated I added the assertion after the other
one:
+ assert(FLAG_IS_ERGO(NewSize) && NewSize == expected,
+ err_msg("NewSize should have been set ergonomically to %zu, but
was %zu", expected, NewSize));
Are you fine with this change?
Thanks,
Stefan
> Jon
>
>
> On 2/7/2014 4:31 AM, Stefan Johansson wrote:
>> Hi again,
>>
>> I've looked at how to test this in a good way and come up with this
>> simple unit test to verify the basic rules for NewSize. This test
>> doesn't strive to cover all possible combinations of how NewSize
>> interacts with other flags, but instead focus on testing that a few
>> well defined cases work as expected without duplicating a lot of
>> logic from the product.
>>
>> Please review the updated webrev:
>> http://cr.openjdk.java.net/~sjohanss/8033426/webrev.01/
>>
>> Cheers,
>> Stefan
>>
>> On 2014-02-04 17:08, Stefan Johansson wrote:
>>> Hi Jesper,
>>>
>>> On 2014-02-03 17:50, Jesper Wilhelmsson wrote:
>>>> Hi Stefan,
>>>>
>>>> Have you looked at how this change plays with the ergonomics for
>>>> NewSize in Arguments::set_cms_and_parnew_gc_flags()? That code will
>>>> also use NewRatio to scale NewSize, but if running with a large
>>>> CMSYoungGenPerWorker it may end up with a larger NewSize. Looks
>>>> like your change will handle it well, but it would be nice with
>>>> some test to verify it going forward.
>>>>
>>> I hadn't looked into this in depth but I took a look now and it
>>> seems to be handled well as you say. If the NewSize is set to
>>> something the new code will never shrink it.
>>>
>>> Regarding testing, Erik H will soon push his fixes for one of the
>>> HeapSize tests. I will make sure nothing is broken with the improved
>>> test and also see if we can add some test case to it to test the
>>> NewSize.
>>>
>>>> Anyways, looks good to me.
>>>> Ship it!
>>> Thanks for reviewing
>>> thishttp://cr.openjdk.java.net/~sjohanss/8033426/webrev.01/.
>>>
>>> Stefan
>>>> /Jesper
>>>>
>>>>
>>>> Stefan Johansson skrev 3/2/14 4:40 PM:
>>>>> Hi,
>>>>>
>>>>> Can I have a couple of reviews for this enhancement:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8033426
>>>>>
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~sjohanss/8033426/webrev.00/
>>>>>
>>>>> Summary:
>>>>> Currently the initial young generation size is the same as the
>>>>> minimum. If not
>>>>> using large pages this will be 1.5M for the default collector
>>>>> regardless of how
>>>>> large the whole heap is. The proposed change is to scale the
>>>>> initial young size
>>>>> the same way the maximum young size is scaled, using the NewRatio
>>>>> parameter.
>>>>>
>>>>> Note:
>>>>> This change has been out on review as part of the fix for
>>>>> JDK-8028498, but after
>>>>> discussions we have decided take another approach for that bug.
>>>>>
>>>>> Testing:
>>>>> * JPRT
>>>>> * GC tests in jtreg
>>>>>
>>>>> Thanks,
>>>>> Stefan
>>>
>>
>
More information about the hotspot-gc-dev
mailing list