RFR(s): 8144527: NewSizeThreadIncrease would make an overflow

[sangheon]

Hi Mikael,

Thank you for reviewing this.

> On 12/22/2015 01:31 AM, Mikael Gerdin wrote:
> Hi Sangheon, 
> 
>> On 12/22/2015 01:40 AM, sangheon wrote:       
>> Hi Jesper, 
>> 
>> Thank you for the review. 
>> 
>> Sangheon 
>> 
>> 
>>> On 12/21/2015 04:39 PM, Jesper Wilhelmsson wrote: 
>>> Looks good! 
>>> /Jesper 
>>> 
>>>> Den 21/12/15 kl. 21:24, skrev sangheon: 
>>>> Hi Jesper, 
>>>> 
>>>> Thank you for looking at this. 
>>>> 
>>>>> On 12/21/2015 06:35 AM, Jesper Wilhelmsson wrote: 
>>>>> Hi Sangheon, 
>>>>> 
>>>>> Did you consider a simpler approach with only nested ifs instead of the 
>>>>> do-while with breaks? At least to me, that would be easier to read. 
>>>> You are right. 
>>>> 
>>>> Here's updated webrev. 
>>>> http://cr.openjdk.java.net/~sangheki/8144527/webrev.01 
> 
> Would you mind moving the code for NewSizeThreadIncrease to a method? 
> Something like "DefNewGeneration::adjust_for_thread_increase()" 
> 
> Otherwise it looks good. 
> (I'll be on vacation after today so I don't need to see a new webrev for the update) 
Okay.

To make sure, I'm posting next webrev which includes Mikael's comment.

Webrev: http://cr.openjdk.java.net/~sangheki/8144527/webrev.02
Testing: JPRT

Thanks,
Sangheon


> /Mikael 
> 
>>>> 
>>>> Thanks, 
>>>> Sangheon 
>>>> 
>>>> 
>>>>> 
>>>>> Besides that it looks good. 
>>>>> /Jesper 
>>>>> 
>>>>> 
>>>>>> Den 21/12/15 kl. 07:52, skrev sangheon:               
>>>>>> Hi all, 
>>>>>> 
>>>>>> Can I have reviews for this change to prevent an overflow for 
>>>>>> NewSizeThreadIncrease? 
>>>>>> 
>>>>>> This option is used with non-daemon threads count when calculate 
>>>>>> new size. And 
>>>>>> as we can't know the thread count at start-up time, this overflow 
>>>>>> can't be 
>>>>>> checked by argument validation framework. Instead I changed related 
>>>>>> routines to 
>>>>>> prevent the overflow and when it happens desired new size will be 
>>>>>> previous size. 
>>>>>> This will affect only for Serial GC. 
>>>>>> 
>>>>>> I added simple test to have 5 threads and then check the log 
>>>>>> whether we had heap 
>>>>>> expansion or not. 
>>>>>> 
>>>>>> CR: https://bugs.openjdk.java.net/browse/JDK-8144527 
>>>>>> Webrev: http://cr.openjdk.java.net/~sangheki/8144527/webrev.00 
>>>>>> Testing: JPRT 
>>>>>> 
>>>>>> Thanks, 
>>>>>> Sangheon

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20151222/63c45a19/attachment.htm>