8037959: BitMap::resize frees old map before copying memory if !in_resource_area

Add reallocate functionality to ArrayAllocator and use it from BitMap::resize

Reviewed-by: brutisso, tschatzl

hotspot/src/share/vm/memory/allocation.hpp

@@ -748,6 +748,12 @@ class ArrayAllocator VALUE_OBJ_CLASS_SPEC {
   bool _use_malloc;
   size_t _size;
   bool _free_in_destructor;
+
+  static bool should_use_malloc(size_t size) {
+    return size < ArrayAllocatorMallocLimit;
+  }
+
+  static char* allocate_inner(size_t& size, bool& use_malloc);
  public:
   ArrayAllocator(bool free_in_destructor = true) :
     _addr(NULL), _use_malloc(false), _size(0), _free_in_destructor(free_in_destructor) { }
@@ -759,6 +765,7 @@ class ArrayAllocator VALUE_OBJ_CLASS_SPEC {
   }
 
   E* allocate(size_t length);
+  E* reallocate(size_t new_length);
   void free();
 };
 

hotspot/src/share/vm/memory/allocation.inline.hpp

@@ -122,35 +122,57 @@ template <MEMFLAGS F> void CHeapObj<F>::operator delete [](void* p){
 }
 
 template <class E, MEMFLAGS F>
-E* ArrayAllocator<E, F>::allocate(size_t length) {
-  assert(_addr == NULL, "Already in use");
+char* ArrayAllocator<E, F>::allocate_inner(size_t &size, bool &use_malloc) {
+  char* addr = NULL;
 
-  _size = sizeof(E) * length;
-  _use_malloc = _size < ArrayAllocatorMallocLimit;
-
-  if (_use_malloc) {
-    _addr = AllocateHeap(_size, F);
-    if (_addr == NULL && _size >=  (size_t)os::vm_allocation_granularity()) {
+  if (use_malloc) {
+    addr = AllocateHeap(size, F);
+    if (addr == NULL && size >= (size_t)os::vm_allocation_granularity()) {
       // malloc failed let's try with mmap instead
-      _use_malloc = false;
+      use_malloc = false;
     } else {
-      return (E*)_addr;
+      return addr;
     }
   }
 
   int alignment = os::vm_allocation_granularity();
-  _size = align_size_up(_size, alignment);
+  size = align_size_up(size, alignment);
 
-  _addr = os::reserve_memory(_size, NULL, alignment, F);
-  if (_addr == NULL) {
-    vm_exit_out_of_memory(_size, OOM_MMAP_ERROR, "Allocator (reserve)");
+  addr = os::reserve_memory(size, NULL, alignment, F);
+  if (addr == NULL) {
+    vm_exit_out_of_memory(size, OOM_MMAP_ERROR, "Allocator (reserve)");
   }
 
-  os::commit_memory_or_exit(_addr, _size, !ExecMem, "Allocator (commit)");
+  os::commit_memory_or_exit(addr, size, !ExecMem, "Allocator (commit)");
+  return addr;
+}
+
+template <class E, MEMFLAGS F>
+E* ArrayAllocator<E, F>::allocate(size_t length) {
+  assert(_addr == NULL, "Already in use");
+
+  _size = sizeof(E) * length;
+  _use_malloc = should_use_malloc(_size);
+  _addr = allocate_inner(_size, _use_malloc);
 
   return (E*)_addr;
 }
 
+template <class E, MEMFLAGS F>
+E* ArrayAllocator<E, F>::reallocate(size_t new_length) {
+  size_t new_size = sizeof(E) * new_length;
+  bool use_malloc = should_use_malloc(new_size);
+  char* new_addr = allocate_inner(new_size, use_malloc);
+
+  memcpy(new_addr, _addr, MIN2(new_size, _size));
+
+  free();
+  _size = new_size;
+  _use_malloc = use_malloc;
+  _addr = new_addr;
+  return (E*)new_addr;
+}
+
 template<class E, MEMFLAGS F>
 void ArrayAllocator<E, F>::free() {
   if (_addr != NULL) {

hotspot/src/share/vm/prims/jni.cpp

@@ -3878,6 +3878,7 @@ void TestMetachunk_test();
 void TestVirtualSpaceNode_test();
 void TestNewSize_test();
 void TestKlass_test();
+void TestBitMap_test();
 #if INCLUDE_ALL_GCS
 void TestOldFreeSpaceCalculation_test();
 void TestG1BiasedArray_test();
@@ -3903,6 +3904,7 @@ void execute_internal_vm_tests() {
     run_unit_test(test_loggc_filename());
     run_unit_test(TestNewSize_test());
     run_unit_test(TestKlass_test());
+    run_unit_test(TestBitMap_test());
 #if INCLUDE_VM_STRUCTS
     run_unit_test(VMStructs::test());
 #endif

hotspot/src/share/vm/utilities/bitMap.cpp

@@ -24,6 +24,7 @@
 
 #include "precompiled.hpp"
 #include "memory/allocation.inline.hpp"
+#include "memory/resourceArea.hpp"
 #include "utilities/bitMap.inline.hpp"
 #include "utilities/copy.hpp"
 #ifdef TARGET_OS_FAMILY_linux
@@ -67,16 +68,14 @@ void BitMap::resize(idx_t size_in_bits, bool in_resource_area) {
   idx_t new_size_in_words = size_in_words();
   if (in_resource_area) {
     _map = NEW_RESOURCE_ARRAY(bm_word_t, new_size_in_words);
+    Copy::disjoint_words((HeapWord*)old_map, (HeapWord*) _map,
+                         MIN2(old_size_in_words, new_size_in_words));
   } else {
-    if (old_map != NULL) {
-      _map_allocator.free();
-    }
-    _map = _map_allocator.allocate(new_size_in_words);
+    _map = _map_allocator.reallocate(new_size_in_words);
   }
-  Copy::disjoint_words((HeapWord*)old_map, (HeapWord*) _map,
-                       MIN2(old_size_in_words, new_size_in_words));
+
   if (new_size_in_words > old_size_in_words) {
-    clear_range_of_words(old_size_in_words, size_in_words());
+    clear_range_of_words(old_size_in_words, new_size_in_words);
   }
 }
 
@@ -536,6 +535,83 @@ void BitMap::print_on(outputStream* st) const {
   tty->cr();
 }
 
+class TestBitMap : public AllStatic {
+  const static BitMap::idx_t BITMAP_SIZE = 1024;
+  static void fillBitMap(BitMap& map) {
+    map.set_bit(1);
+    map.set_bit(3);
+    map.set_bit(17);
+    map.set_bit(512);
+  }
+
+  static void testResize(bool in_resource_area) {
+    {
+      BitMap map(0, in_resource_area);
+      map.resize(BITMAP_SIZE, in_resource_area);
+      fillBitMap(map);
+
+      BitMap map2(BITMAP_SIZE, in_resource_area);
+      fillBitMap(map2);
+      assert(map.is_same(map2), "could be");
+    }
+
+    {
+      BitMap map(128, in_resource_area);
+      map.resize(BITMAP_SIZE, in_resource_area);
+      fillBitMap(map);
+
+      BitMap map2(BITMAP_SIZE, in_resource_area);
+      fillBitMap(map2);
+      assert(map.is_same(map2), "could be");
+    }
+
+    {
+      BitMap map(BITMAP_SIZE, in_resource_area);
+      map.resize(BITMAP_SIZE, in_resource_area);
+      fillBitMap(map);
+
+      BitMap map2(BITMAP_SIZE, in_resource_area);
+      fillBitMap(map2);
+      assert(map.is_same(map2), "could be");
+    }
+  }
+
+  static void testResizeResource() {
+    ResourceMark rm;
+    testResize(true);
+  }
+
+  static void testResizeNonResource() {
+    const uintx bitmap_bytes = BITMAP_SIZE / BitsPerByte;
+
+    // Test the default behavior
+    testResize(false);
+
+    {
+      // Make sure that AllocatorMallocLimit is larger than our allocation request
+      // forcing it to call standard malloc()
+      UIntFlagSetting fs(ArrayAllocatorMallocLimit, bitmap_bytes * 4);
+      testResize(false);
+    }
+    {
+      // Make sure that AllocatorMallocLimit is smaller than our allocation request
+      // forcing it to call mmap() (or equivalent)
+      UIntFlagSetting fs(ArrayAllocatorMallocLimit, bitmap_bytes / 4);
+      testResize(false);
+    }
+  }
+
+ public:
+  static void test() {
+    testResizeResource();
+    testResizeNonResource();
+  }
+
+};
+
+void TestBitMap_test() {
+  TestBitMap::test();
+}
 #endif