8248426: NMT: VirtualMemoryTracker::split_reserved_region() does not …

…properly update summary counting

Reviewed-by: stuefe, minqi

src/hotspot/share/services/virtualMemoryTracker.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2020, 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
@@ -445,6 +445,19 @@ bool VirtualMemoryTracker::remove_uncommitted_region(address addr, size_t size)
   return result;
 }
 
+bool VirtualMemoryTracker::remove_released_region(ReservedMemoryRegion* rgn) {
+  assert(rgn != NULL, "Sanity check");
+  assert(_reserved_regions != NULL, "Sanity check");
+
+  // uncommit regions within the released region
+  if (!rgn->remove_uncommitted_region(rgn->base(), rgn->size())) {
+    return false;
+  }
+
+  VirtualMemorySummary::record_released_memory(rgn->size(), rgn->flag());
+  return _reserved_regions->remove(*rgn);
+}
+
 bool VirtualMemoryTracker::remove_released_region(address addr, size_t size) {
   assert(addr != NULL, "Invalid address");
   assert(size > 0, "Invalid size");
@@ -454,15 +467,17 @@ bool VirtualMemoryTracker::remove_released_region(address addr, size_t size) {
   ReservedMemoryRegion* reserved_rgn = _reserved_regions->find(rgn);
 
   assert(reserved_rgn != NULL, "No reserved region");
+  if (reserved_rgn->same_region(addr, size)) {
+    return remove_released_region(reserved_rgn);
+  }
 
   // uncommit regions within the released region
   if (!reserved_rgn->remove_uncommitted_region(addr, size)) {
     return false;
   }
 
   if (reserved_rgn->flag() == mtClassShared &&
-      reserved_rgn->contain_region(addr, size) &&
-      !reserved_rgn->same_region(addr, size)) {
+      reserved_rgn->contain_region(addr, size)) {
     // This is an unmapped CDS region, which is part of the reserved shared
     // memory region.
     // See special handling in VirtualMemoryTracker::add_reserved_region also.
@@ -471,29 +486,25 @@ bool VirtualMemoryTracker::remove_released_region(address addr, size_t size) {
 
   VirtualMemorySummary::record_released_memory(size, reserved_rgn->flag());
 
-  if (reserved_rgn->same_region(addr, size)) {
-    return _reserved_regions->remove(rgn);
+  assert(reserved_rgn->contain_region(addr, size), "Not completely contained");
+  if (reserved_rgn->base() == addr ||
+      reserved_rgn->end() == addr + size) {
+      reserved_rgn->exclude_region(addr, size);
+    return true;
   } else {
-    assert(reserved_rgn->contain_region(addr, size), "Not completely contained");
-    if (reserved_rgn->base() == addr ||
-        reserved_rgn->end() == addr + size) {
-        reserved_rgn->exclude_region(addr, size);
-      return true;
+    address top = reserved_rgn->end();
+    address high_base = addr + size;
+    ReservedMemoryRegion high_rgn(high_base, top - high_base,
+      *reserved_rgn->call_stack(), reserved_rgn->flag());
+
+    // use original region for lower region
+    reserved_rgn->exclude_region(addr, top - addr);
+    LinkedListNode<ReservedMemoryRegion>* new_rgn = _reserved_regions->add(high_rgn);
+    if (new_rgn == NULL) {
+      return false;
     } else {
-      address top = reserved_rgn->end();
-      address high_base = addr + size;
-      ReservedMemoryRegion high_rgn(high_base, top - high_base,
-        *reserved_rgn->call_stack(), reserved_rgn->flag());
-
-      // use original region for lower region
-      reserved_rgn->exclude_region(addr, top - addr);
-      LinkedListNode<ReservedMemoryRegion>* new_rgn = _reserved_regions->add(high_rgn);
-      if (new_rgn == NULL) {
-        return false;
-      } else {
-        reserved_rgn->move_committed_regions(addr, *new_rgn->data());
-        return true;
-      }
+      reserved_rgn->move_committed_regions(addr, *new_rgn->data());
+      return true;
     }
   }
 }
@@ -512,7 +523,7 @@ bool VirtualMemoryTracker::split_reserved_region(address addr, size_t size, size
   NativeCallStack original_stack = *reserved_rgn->call_stack();
   MEMFLAGS original_flags = reserved_rgn->flag();
 
-  _reserved_regions->remove(rgn);
+  remove_released_region(reserved_rgn);
 
   // Now, create two new regions.
   add_reserved_region(addr, split, original_stack, original_flags);

src/hotspot/share/services/virtualMemoryTracker.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2020, 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
@@ -382,6 +382,7 @@ class VirtualMemoryTracker : AllStatic {
   static bool add_committed_region      (address base_addr, size_t size, const NativeCallStack& stack);
   static bool remove_uncommitted_region (address base_addr, size_t size);
   static bool remove_released_region    (address base_addr, size_t size);
+  static bool remove_released_region    (ReservedMemoryRegion* rgn);
   static void set_reserved_region_type  (address addr, MEMFLAGS flag);
 
   // Given an existing memory mapping registered with NMT, split the mapping in