[8u] RFR: 8144483: One long Safepoint pause directly after each GC log rotation

[David Holmes]

On 7/01/2016 4:42 PM, cheleswer sahu wrote:
> Hi David,
> JDK9 uses unified logging and unified logging does not use the
> problematic function call. I have run the test with unified logging
> enabled and didn't  find the issue. Therefore I had started review
> request only for JDK8.

Ah I see. Okay can you update the bug report to remove "9" from the 
"Affects Version", add a 9-na label, and a comment explaining why this 
is now 9-na.

Thanks,
David

> Regards,
> Cheleswer
>
> On 1/7/2016 10:21 AM, David Holmes wrote:
>> Hi Cheleswer,
>>
>> This needs to be fixed in JDK 9 first and then flagged for backport if
>> desired. The "Fix Version" in the bug should be set to 9.
>>
>> Thanks,
>> David
>>
>> On 6/01/2016 9:37 PM, cheleswer sahu wrote:
>>> Hi,
>>>
>>> Please review the code changes for
>>> "https://bugs.openjdk.java.net/browse/JDK-8144483".
>>>
>>> webrev link: http://cr.openjdk.java.net/~kevinw/8144483/webrev.00/
>>> JBS link: https://bugs.openjdk.java.net/browse/JDK-8144483
>>>
>>> Bug brief:
>>> A long pause is observed after each gc log rotation in Solaris.
>>>
>>> Problem Identified:
>>> In each GC log rotation "print_memory_info()" is getting called through
>>> dump_loggc_header().
>>> "print_memory_info()" of solaris version calls check_addr0(st);
>>>
>>> File:
>>> http://hg.openjdk.java.net/jdk8u/jdk8u/hotspot/file/80959a760b85/src/os/solaris/vm/os_solaris.cpp
>>>
>>>
>>>
>>> void os::print_memory_info(outputStream* st) {
>>>    st->print("Memory:");
>>>
>>>    st->print(" %dk page", os::vm_page_size()>>10);
>>>
>>>    st->print(", physical " UINT64_FORMAT "k",
>>> os::physical_memory()>>10);
>>>
>>>    st->print("(" UINT64_FORMAT "k free)", os::available_memory() >> 10);
>>>
>>>    st->cr();
>>>   (void) check_addr0(st);
>>> }
>>>
>>> Now check_addr0(st) function do a lot of read call to read the data from
>>> /proc/self/map.
>>> and check if virtual address is mapped to 0x0. These read calls take lot
>>> of time which results in GC rotation pause.
>>> Here calling check_addr0() seems unnecessary for every log rotation. It
>>> will be more logical if this function gets called only
>>> when an error is reported.
>>>
>>> Solution proposed:
>>> Before GC log rotation print_memory_info() is ever getting called from
>>> Vm_error.cpp during error reporting. And in case of error reporting
>>> checking for address mapping to 0x0 looks fine. So the proposed solution
>>> is to do an extra check inside print_memory_info().
>>>
>>> -  (void) check_addr0(st);
>>> +  if (VMError::fatal_error_in_progress()){
>>> +     (void) check_addr0(st);
>>> +  }
>>>   }
>>>
>>>   This check doesn't fit well inside printing function, but at this
>>> point I don't see the need to create a new os:: method and change all
>>> the OS classes just for that check.
>>>
>>> Regards,
>>> Cheleswer
>