RFR (S): JDK-8006432 - Ratio flags should be unsigned

[Jesper Wilhelmsson]

On 6/2/13 1:01 AM, Tao Mao wrote:
> PS1:
> Jesper, how did you check and make sure that modified flags are treated
> properly at all their occurrences? (So I can apply generally. For the
> review, I manually checked all of their occurrences).

I went through all occurrences manually. I don't think there is a 
reliable way to do something like this automatically.

In this case it was really easy since most flags were only read in a few 
if statements and the places where their value was stored in some other 
variable that other variable was already unsigned. So the change didn't 
propagate.

If you look at the change to make tenuring threshold unsigned it was a 
lot more involved even though it only changed two flags.

http://hg.openjdk.java.net/hsx/hotspot-gc-gate/hotspot/rev/22b8d3d181d9

I don't think any refactoring tool or similar could do that and get it 
right.

> PS2:
> product(intx, SurvivorRatio, 8,                                           \
>               "Ratio of eden/survivor space
> size")                              \
> \
> product(intx, NewRatio, 2,                                                \
>               "Ratio of new/old generation
> sizes")                              \
>
> For clarity and consistency with SurvivorRatio definition, the NewRatio
> definition above should be reversed as "Ratio of *old/new* generation sizes"
>
> What are your opinions? If you favor my suggestion, I'll go ahead
> creating a CR and modify it, since it's a fairly busy VM option.

Yes I agree. Go ahead and create a CR for this. While you're at it you 
might want to go through the other ratio flags as well just to make sure 
they are OK. The ratio concept is a bit confusing and it doesn't help 
that some of our ratio flags are actually percentages...

Cheers,
/Jesper


>
> Thank you.
> Tao
>
>
> On 2/5/2013 3:41 PM, Tao Mao wrote:
>> Looks good to me. I've manually checked all occurrences of the four
>> flags. They are treated properly after the change.
>> I'm not an official reviewer. But, as I understand, code review needs
>> at least one official reviewer. So, push it!
>>
>> Thanks.
>> Tao
>>
>> On 1/25/2013 8:24 AM, Jesper Wilhelmsson wrote:
>>> Hi John,
>>>
>>> Thanks for the review!
>>> I have removed G1InitYoungSurvRatio and updated the webrev and the
>>> bug to reflect this.
>>> /Jesper
>>>
>>>
>>> On 2013-01-25 01:27, John Cuthbertson wrote:
>>>> Hi Jesper,
>>>>
>>>> Looks good to me.
>>>>
>>>> Can you also remove the unused G1InitYoungSurvRatio from
>>>> g1_globals.hpp?
>>>>
>>>> Thanks,
>>>>
>>>> JohnC
>>>>
>>>> On 1/24/2013 1:45 PM, Jesper Wilhelmsson wrote:
>>>>> Hi,
>>>>>
>>>>> I'm looking for a couple of reviews for this small change.
>>>>>
>>>>> Bug: JDK-8006432 - Ratio flags should be unsigned
>>>>>
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~jwilhelm/8006432/webrev/
>>>>>
>>>>> Summary:
>>>>> Four flags whose contents are assumed to be unsigned were stored in
>>>>> signed
>>>>> variables. I have changed these to be unsigned instead.
>>>>>
>>>>> Testing:
>>>>> Manual testing and JPRT.
>>>>>
>>>>> /Jesper
>>>>