8138705: Kitchen sink stress test fails
authormockner
Thu, 02 Jun 2016 12:07:55 -0400
changeset 38935 f7427b0e0d7c
parent 38929 1ee62412a66f
child 38936 fb7af95c9f40
8138705: Kitchen sink stress test fails Summary: NMT now supports overlapping commits. Reviewed-by: coleenp, zgu
hotspot/src/share/vm/services/virtualMemoryTracker.cpp
hotspot/src/share/vm/services/virtualMemoryTracker.hpp
hotspot/src/share/vm/utilities/linkedlist.hpp
hotspot/test/runtime/NMT/CommitOverlappingRegions.java
--- a/hotspot/src/share/vm/services/virtualMemoryTracker.cpp	Thu Jun 02 09:44:41 2016 +0200
+++ b/hotspot/src/share/vm/services/virtualMemoryTracker.cpp	Thu Jun 02 12:07:55 2016 -0400
@@ -23,7 +23,10 @@
  */
 #include "precompiled.hpp"
 
+#include "runtime/atomic.inline.hpp"
+#include "runtime/os.hpp"
 #include "runtime/threadCritical.hpp"
+#include "services/memTracker.hpp"
 #include "services/virtualMemoryTracker.hpp"
 
 size_t VirtualMemorySummary::_snapshot[CALC_OBJ_SIZE_IN_TYPE(VirtualMemorySnapshot, size_t)];
@@ -52,46 +55,41 @@
   if (all_committed()) return true;
 
   CommittedMemoryRegion committed_rgn(addr, size, stack);
-  LinkedListNode<CommittedMemoryRegion>* node = _committed_regions.find_node(committed_rgn);
-  if (node != NULL) {
+  LinkedListNode<CommittedMemoryRegion>* node = _committed_regions.head();
+
+  while (node != NULL) {
     CommittedMemoryRegion* rgn = node->data();
     if (rgn->same_region(addr, size)) {
       return true;
     }
 
     if (rgn->adjacent_to(addr, size)) {
-      // check if the next region covers this committed region,
-      // the regions may not be merged due to different call stacks
-      LinkedListNode<CommittedMemoryRegion>* next =
-        node->next();
-      if (next != NULL && next->data()->contain_region(addr, size)) {
-        if (next->data()->same_region(addr, size)) {
-          next->data()->set_call_stack(stack);
-        }
-        return true;
-      }
-      if (rgn->call_stack()->equals(stack)) {
+      // special case to expand prior region if there is no next region
+      LinkedListNode<CommittedMemoryRegion>* next = node->next();
+      if (next == NULL && rgn->call_stack()->equals(stack)) {
         VirtualMemorySummary::record_uncommitted_memory(rgn->size(), flag());
         // the two adjacent regions have the same call stack, merge them
         rgn->expand_region(addr, size);
         VirtualMemorySummary::record_committed_memory(rgn->size(), flag());
         return true;
       }
-      VirtualMemorySummary::record_committed_memory(size, flag());
-      if (rgn->base() > addr) {
-        return _committed_regions.insert_before(committed_rgn, node) != NULL;
-      } else {
-        return _committed_regions.insert_after(committed_rgn, node) != NULL;
       }
+
+    if (rgn->overlap_region(addr, size)) {
+      // Clear a space for this region in the case it overlaps with any regions.
+      remove_uncommitted_region(addr, size);
+      break;  // commit below
     }
-    assert(rgn->contain_region(addr, size), "Must cover this region");
-    return true;
-  } else {
+    if (rgn->end() >= addr + size){
+      break;
+    }
+    node = node->next();
+  }
+
     // New committed region
     VirtualMemorySummary::record_committed_memory(size, flag());
     return add_committed_region(committed_rgn);
   }
-}
 
 void ReservedMemoryRegion::set_all_committed(bool b) {
   if (all_committed() != b) {
@@ -175,48 +173,52 @@
       }
     }
   } else {
-    // we have to walk whole list to remove the committed regions in
-    // specified range
-    LinkedListNode<CommittedMemoryRegion>* head =
-      _committed_regions.head();
-    LinkedListNode<CommittedMemoryRegion>* prev = NULL;
-    VirtualMemoryRegion uncommitted_rgn(addr, sz);
+    CommittedMemoryRegion del_rgn(addr, sz, *call_stack());
+    address end = addr + sz;
 
-    while (head != NULL && !uncommitted_rgn.is_empty()) {
-      CommittedMemoryRegion* crgn = head->data();
-      // this committed region overlaps to region to uncommit
-      if (crgn->overlap_region(uncommitted_rgn.base(), uncommitted_rgn.size())) {
-        if (crgn->same_region(uncommitted_rgn.base(), uncommitted_rgn.size())) {
-          // find matched region, remove the node will do
-          VirtualMemorySummary::record_uncommitted_memory(uncommitted_rgn.size(), flag());
+    LinkedListNode<CommittedMemoryRegion>* head = _committed_regions.head();
+    LinkedListNode<CommittedMemoryRegion>* prev = NULL;
+    CommittedMemoryRegion* crgn;
+
+    while (head != NULL) {
+      crgn = head->data();
+
+      if (crgn->same_region(addr, sz)) {
+        VirtualMemorySummary::record_uncommitted_memory(crgn->size(), flag());
           _committed_regions.remove_after(prev);
           return true;
-        } else if (crgn->contain_region(uncommitted_rgn.base(), uncommitted_rgn.size())) {
-          // this committed region contains whole uncommitted region
-          VirtualMemorySummary::record_uncommitted_memory(uncommitted_rgn.size(), flag());
-          return remove_uncommitted_region(head, uncommitted_rgn.base(), uncommitted_rgn.size());
-        } else if (uncommitted_rgn.contain_region(crgn->base(), crgn->size())) {
-          // this committed region has been uncommitted
-          size_t exclude_size = crgn->end() - uncommitted_rgn.base();
-          uncommitted_rgn.exclude_region(uncommitted_rgn.base(), exclude_size);
+      }
+
+      // del_rgn contains crgn
+      if (del_rgn.contain_region(crgn->base(), crgn->size())) {
           VirtualMemorySummary::record_uncommitted_memory(crgn->size(), flag());
-          LinkedListNode<CommittedMemoryRegion>* tmp = head;
           head = head->next();
           _committed_regions.remove_after(prev);
-          continue;
-        } else if (crgn->contain_address(uncommitted_rgn.base())) {
-          size_t toUncommitted = crgn->end() - uncommitted_rgn.base();
-          crgn->exclude_region(uncommitted_rgn.base(), toUncommitted);
-          uncommitted_rgn.exclude_region(uncommitted_rgn.base(), toUncommitted);
-          VirtualMemorySummary::record_uncommitted_memory(toUncommitted, flag());
-        } else if (uncommitted_rgn.contain_address(crgn->base())) {
-          size_t toUncommitted = uncommitted_rgn.end() - crgn->base();
-          crgn->exclude_region(crgn->base(), toUncommitted);
-          uncommitted_rgn.exclude_region(uncommitted_rgn.end() - toUncommitted,
-            toUncommitted);
-          VirtualMemorySummary::record_uncommitted_memory(toUncommitted, flag());
+        continue;  // don't update head or prev
         }
+
+      // Found addr in the current crgn. There are 2 subcases:
+      if (crgn->contain_address(addr)) {
+
+        // (1) Found addr+size in current crgn as well. (del_rgn is contained in crgn)
+        if (crgn->contain_address(end - 1)) {
+          VirtualMemorySummary::record_uncommitted_memory(sz, flag());
+          return remove_uncommitted_region(head, addr, sz); // done!
+        } else {
+          // (2) Did not find del_rgn's end in crgn.
+          size_t size = crgn->end() - del_rgn.base();
+          crgn->exclude_region(addr, size);
+          VirtualMemorySummary::record_uncommitted_memory(size, flag());
       }
+
+      } else if (crgn->contain_address(end - 1)) {
+      // Found del_rgn's end, but not its base addr.
+        size_t size = del_rgn.end() - crgn->base();
+        crgn->exclude_region(crgn->base(), size);
+        VirtualMemorySummary::record_uncommitted_memory(size, flag());
+        return true;  // should be done if the list is sorted properly!
+      }
+
       prev = head;
       head = head->next();
     }
@@ -386,7 +388,8 @@
 
   assert(reserved_rgn != NULL, "No reserved region");
   assert(reserved_rgn->contain_region(addr, size), "Not completely contained");
-  return reserved_rgn->add_committed_region(addr, size, stack);
+  bool result = reserved_rgn->add_committed_region(addr, size, stack);
+  return result;
 }
 
 bool VirtualMemoryTracker::remove_uncommitted_region(address addr, size_t size) {
@@ -398,7 +401,8 @@
   ReservedMemoryRegion* reserved_rgn = _reserved_regions->find(rgn);
   assert(reserved_rgn != NULL, "No reserved region");
   assert(reserved_rgn->contain_region(addr, size), "Not completely contained");
-  return reserved_rgn->remove_uncommitted_region(addr, size);
+  bool result = reserved_rgn->remove_uncommitted_region(addr, size);
+  return result;
 }
 
 bool VirtualMemoryTracker::remove_released_region(address addr, size_t size) {
@@ -488,5 +492,3 @@
 
   return true;
 }
-
-
--- a/hotspot/src/share/vm/services/virtualMemoryTracker.hpp	Thu Jun 02 09:44:41 2016 +0200
+++ b/hotspot/src/share/vm/services/virtualMemoryTracker.hpp	Thu Jun 02 12:07:55 2016 -0400
@@ -261,8 +261,7 @@
     VirtualMemoryRegion(addr, size), _stack(stack) { }
 
   inline int compare(const CommittedMemoryRegion& rgn) const {
-    if (overlap_region(rgn.base(), rgn.size()) ||
-        adjacent_to   (rgn.base(), rgn.size())) {
+    if (overlap_region(rgn.base(), rgn.size())) {
       return 0;
     } else {
       if (base() == rgn.base()) {
--- a/hotspot/src/share/vm/utilities/linkedlist.hpp	Thu Jun 02 09:44:41 2016 +0200
+++ b/hotspot/src/share/vm/utilities/linkedlist.hpp	Thu Jun 02 12:07:55 2016 -0400
@@ -259,6 +259,11 @@
 
   virtual bool remove(LinkedListNode<E>* node) {
     LinkedListNode<E>* p = this->head();
+    if (p == node) {
+      this->set_head(p->next());
+      delete_node(node);
+      return true;
+    }
     while (p != NULL && p->next() != node) {
       p = p->next();
     }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/runtime/NMT/CommitOverlappingRegions.java	Thu Jun 02 12:07:55 2016 -0400
@@ -0,0 +1,144 @@
+/*
+ * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @summary Test commits of overlapping regions of memory.
+ * @key nmt jcmd
+ * @library /testlibrary /test/lib
+ * @modules java.base/jdk.internal.misc
+ *          java.management
+ * @build   CommitOverlappingRegions
+ * @run main ClassFileInstaller sun.hotspot.WhiteBox
+ *                              sun.hotspot.WhiteBox$WhiteBoxPermission
+ * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -XX:NativeMemoryTracking=detail CommitOverlappingRegions
+ */
+
+import jdk.test.lib.*;
+import sun.hotspot.WhiteBox;
+
+public class CommitOverlappingRegions {
+    public static WhiteBox wb = WhiteBox.getWhiteBox();
+    public static void main(String args[]) throws Exception {
+        OutputAnalyzer output;
+        long size = 32 * 1024;
+        long addr = wb.NMTReserveMemory(8*size);
+
+        String pid = Long.toString(ProcessTools.getProcessId());
+        ProcessBuilder pb = new ProcessBuilder();
+
+        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"), pid, "VM.native_memory", "detail"});
+        System.out.println("Address is " + Long.toHexString(addr));
+
+        // Start: . . . . . . . .
+        output = new OutputAnalyzer(pb.start());
+        output.shouldContain("Test (reserved=256KB, committed=0KB)");
+
+        // Committing: * * * . . . . .
+        // Region:     * * * . . . . .
+        // Expected Total: 3 x 32KB = 96KB
+        wb.NMTCommitMemory(addr + 0*size, 3*size);
+
+        // Committing: . . . . * * * .
+        // Region:     * * * . * * * .
+        // Expected Total: 6 x 32KB = 192KB
+        wb.NMTCommitMemory(addr + 4*size, 3*size);
+
+        // Check output after first 2 commits.
+        output = new OutputAnalyzer(pb.start());
+        output.shouldContain("Test (reserved=256KB, committed=192KB)");
+
+        // Committing: . . * * * . . .
+        // Region:     * * * * * * * .
+        // Expected Total: 7 x 32KB = 224KB
+        wb.NMTCommitMemory(addr + 2*size, 3*size);
+
+        // Check output after overlapping commit.
+        output = new OutputAnalyzer(pb.start());
+        output.shouldContain("Test (reserved=256KB, committed=224KB)");
+
+        // Uncommitting: * * * * * * * *
+        // Region:       . . . . . . . .
+        // Expected Total: 0 x 32KB = 0KB
+        wb.NMTUncommitMemory(addr + 0*size, 8*size);
+        output = new OutputAnalyzer(pb.start());
+        output.shouldContain("Test (reserved=256KB, committed=0KB)");
+
+        // Committing: * * . . . . . .
+        // Region:     * * . . . . . .
+        // Expected Total: 2 x 32KB = 64KB
+        wb.NMTCommitMemory(addr + 0*size, 2*size);
+        output = new OutputAnalyzer(pb.start());
+        output.shouldContain("Test (reserved=256KB, committed=64KB)");
+
+        // Committing: . * * * . . . .
+        // Region:     * * * * . . . .
+        // Expected Total: 4 x 32KB = 128KB
+        wb.NMTCommitMemory(addr + 1*size, 3*size);
+        output = new OutputAnalyzer(pb.start());
+        output.shouldContain("Test (reserved=256KB, committed=128KB)");
+
+        // Uncommitting: * * * . . . . .
+        // Region:       . . . * . . . .
+        // Expected Total: 1 x 32KB = 32KB
+        wb.NMTUncommitMemory(addr + 0*size, 3*size);
+        output = new OutputAnalyzer(pb.start());
+        output.shouldContain("Test (reserved=256KB, committed=32KB)");
+
+        // Committing: . . . * * . . .
+        // Region:     . . . * * . . .
+        // Expected Total: 2 x 32KB = 64KB
+        wb.NMTCommitMemory(addr + 3*size, 2*size);
+        System.out.println("Address is " + Long.toHexString(addr + 3*size));
+        output = new OutputAnalyzer(pb.start());
+        output.shouldContain("Test (reserved=256KB, committed=64KB)");
+
+        // Committing: . . . . * * . .
+        // Region:     . . . * * * . .
+        // Expected Total: 3 x 32KB = 96KB
+        wb.NMTCommitMemory(addr + 4*size, 2*size);
+        output = new OutputAnalyzer(pb.start());
+        output.shouldContain("Test (reserved=256KB, committed=96KB)");
+
+        // Committing: . . . . . * * .
+        // Region:     . . . * * * * .
+        // Expected Total: 4 x 32KB = 128KB
+        wb.NMTCommitMemory(addr + 5*size, 2*size);
+        output = new OutputAnalyzer(pb.start());
+        output.shouldContain("Test (reserved=256KB, committed=128KB)");
+
+        // Committing: . . . . . . * *
+        // Region:     . . . * * * * *
+        // Expected Total: 5 x 32KB = 160KB
+        wb.NMTCommitMemory(addr + 6*size, 2*size);
+        output = new OutputAnalyzer(pb.start());
+        output.shouldContain("Test (reserved=256KB, committed=160KB)");
+
+        // Uncommitting: * * * * * * * *
+        // Region:       . . . . . . . .
+        // Expected Total: 0 x 32KB = 32KB
+        wb.NMTUncommitMemory(addr + 0*size, 8*size);
+        output = new OutputAnalyzer(pb.start());
+        output.shouldContain("Test (reserved=256KB, committed=0KB)");
+    }
+}