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

[Mattis Castegren]

Hi

This is for an escalated customer issue, so it would be great if someone could review this today or tomorrow. The customer is pushing for an interim patch for this, so we would want to push this fix as soon as possible.

Kind Regards
/Mattis

-----Original Message-----
From: cheleswer sahu 
Sent: den 6 januari 2016 12:38
To: hotspot-runtime-dev at openjdk.java.net
Cc: Mattis Castegren; Kevin Walls
Subject: [8u] RFR: 8144483: One long Safepoint pause directly after each GC log rotation

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