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

[cheleswer sahu]

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.

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