RFR(S): 8230126: delay_to_keep_mmu can delay shutdown

[Kim Barrett]

> On Aug 28, 2019, at 12:37 AM, sangheon.kim at oracle.com wrote:
> 
> 
> 
> On 8/27/19 4:19 PM, Kim Barrett wrote:
>>> On Aug 27, 2019, at 7:06 PM, sangheon.kim at oracle.com
>>>  wrote:
>>> 
>>> Hi Kim,
>>> 
>>> On 8/27/19 12:22 PM, Kim Barrett wrote:
>>> 
>>>> […]
>>>> CR:
>>>> 
>>>> https://bugs.openjdk.java.net/browse/JDK-8230126
>>>> 
>>>> 
>>>> Webrev:
>>>> 
>>>> http://cr.openjdk.java.net/~kbarrett/8230126/open.00/
>>> Couldn't always use 'ms'?
>>> I think 'os::elapsedTime() * 1000' seems simpler instead of 'ms -> sec -> ms' conversion and may remove ceil() call.
>>> 
>> The only ms -> sec conversion is the prediction in mmu_delay_end, and
>> that's necessary because we have a prediction value provided in ms and
>> the tracker wants sec.  Having the delay_end return a ms value would
>> add another sec -> ms conversion (using when_ms rather than when_sec;
>> remember that both take arguments in secs, and there was a bug in the
>> old code wrto that).
>> 
>> In delay_to_keep_mmu, there's necessarily going to be a sec -> ms
>> conversion somewhere because os::elapsedTime() returns secs and we
>> need to compute a ms value based on it, for Monitor::wait.
>> 
> You are right. 
> I misunderstood the signature of when_ms().
> I thought it gets (ms, ms) but it isn't. Sorry for the noise.

Yeah, the name can easily fool one.

>> The use of ceil doesn't go away if the sec -> ms conversion is moved.
>> The floating point time value is a minimum delay; conversion of a
>> positive value to a jlong will truncate, which is the wrong rounding
>> direction.
>> 
> I do understand your intent here, but I think GC codes don't care less than 1ms. Do we?
> I'm okay with ceil().
> 
> Thanks,
> Sangheon

Thanks.