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

[David Holmes]

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