8296785: Use realloc for CHeap-allocated BitMaps

Reviewed-by: stuefe, aboldtch

src/hotspot/share/memory/allocation.hpp

@@ -627,6 +627,8 @@ class ArrayAllocator : public AllStatic {
   static E* allocate_malloc(size_t length, MEMFLAGS flags);
   static E* allocate_mmap(size_t length, MEMFLAGS flags);
 
+  static E* reallocate_malloc(E* addr, size_t new_length, MEMFLAGS flags);
+
   static void free_malloc(E* addr, size_t length);
   static void free_mmap(E* addr, size_t length);
 
@@ -656,6 +658,7 @@ class MallocArrayAllocator : public AllStatic {
   static size_t size_for(size_t length);
 
   static E* allocate(size_t length, MEMFLAGS flags);
+  static E* reallocate(E* addr, size_t new_length, MEMFLAGS flags);
   static void free(E* addr);
 };
 

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

@@ -101,7 +101,12 @@ E* MallocArrayAllocator<E>::allocate(size_t length, MEMFLAGS flags) {
   return (E*)AllocateHeap(size_for(length), flags);
 }
 
-template<class E>
+template <class E>
+E* MallocArrayAllocator<E>::reallocate(E* addr, size_t new_length, MEMFLAGS flags) {
+  return (E*)ReallocateHeap((char*)addr, size_for(new_length), flags);
+}
+
+template <class E>
 void MallocArrayAllocator<E>::free(E* addr) {
   FreeHeap(addr);
 }
@@ -130,8 +135,17 @@ E* ArrayAllocator<E>::allocate(size_t length, MEMFLAGS flags) {
   return allocate_mmap(length, flags);
 }
 
+template <class E>
+E* ArrayAllocator<E>::reallocate_malloc(E* addr, size_t new_length, MEMFLAGS flags) {
+  return MallocArrayAllocator<E>::reallocate(addr, new_length, flags);
+}
+
 template <class E>
 E* ArrayAllocator<E>::reallocate(E* old_addr, size_t old_length, size_t new_length, MEMFLAGS flags) {
+  if (should_use_malloc(old_length) && should_use_malloc(new_length)) {
+    return reallocate_malloc(old_addr, new_length, flags);
+  }
+
   E* new_addr = (new_length > 0)
       ? allocate(new_length, flags)
       : NULL;
@@ -147,17 +161,17 @@ E* ArrayAllocator<E>::reallocate(E* old_addr, size_t old_length, size_t new_leng
   return new_addr;
 }
 
-template<class E>
+template <class E>
 void ArrayAllocator<E>::free_malloc(E* addr, size_t length) {
   MallocArrayAllocator<E>::free(addr);
 }
 
-template<class E>
+template <class E>
 void ArrayAllocator<E>::free_mmap(E* addr, size_t length) {
   MmapArrayAllocator<E>::free(addr, length);
 }
 
-template<class E>
+template <class E>
 void ArrayAllocator<E>::free(E* addr, size_t length) {
   if (addr != NULL) {
     if (should_use_malloc(length)) {

src/hotspot/share/utilities/bitMap.cpp

@@ -36,38 +36,67 @@ using idx_t = BitMap::idx_t;
 
 STATIC_ASSERT(sizeof(bm_word_t) == BytesPerWord); // "Implementation assumption."
 
+// For the BitMaps with allocators that don't support reallocate
 template <class BitMapWithAllocator>
-bm_word_t* GrowableBitMap<BitMapWithAllocator>::reallocate(bm_word_t* old_map, idx_t old_size_in_bits, idx_t new_size_in_bits, bool clear) {
-  size_t old_size_in_words = calc_size_in_words(old_size_in_bits);
-  size_t new_size_in_words = calc_size_in_words(new_size_in_bits);
+static bm_word_t* pseudo_reallocate(const BitMapWithAllocator& derived, bm_word_t* old_map, size_t old_size_in_words, size_t new_size_in_words) {
+  assert(new_size_in_words > 0, "precondition");
 
-  bm_word_t* map = NULL;
-  BitMapWithAllocator* derived = static_cast<BitMapWithAllocator*>(this);
+  bm_word_t* map = derived.allocate(new_size_in_words);
+  if (old_map != NULL) {
+    Copy::disjoint_words((HeapWord*)old_map, (HeapWord*) map,
+        MIN2(old_size_in_words, new_size_in_words));
+  }
 
-  if (new_size_in_words > 0) {
-    map = derived->allocate(new_size_in_words);
+  derived.free(old_map, old_size_in_words);
 
-    if (old_map != NULL) {
-      Copy::disjoint_words((HeapWord*)old_map, (HeapWord*) map,
-                           MIN2(old_size_in_words, new_size_in_words));
-    }
+  return map;
+}
 
-    if (clear && (new_size_in_bits > old_size_in_bits)) {
-      // If old_size_in_bits is not word-aligned, then the preceding
-      // copy can include some trailing bits in the final copied word
-      // that also need to be cleared.  See clear_range_within_word.
-      bm_word_t mask = bit_mask(old_size_in_bits) - 1;
-      map[raw_to_words_align_down(old_size_in_bits)] &= mask;
-      // Clear the remaining full words.
-      clear_range_of_words(map, old_size_in_words, new_size_in_words);
-    }
-  }
+template <class BitMapWithAllocator>
+void GrowableBitMap<BitMapWithAllocator>::initialize(idx_t size_in_bits, bool clear) {
+  assert(map() == NULL, "precondition");
+  assert(size() == 0,   "precondition");
 
-  if (old_map != NULL) {
+  resize(size_in_bits, clear);
+}
+
+template <class BitMapWithAllocator>
+void GrowableBitMap<BitMapWithAllocator>::reinitialize(idx_t new_size_in_bits, bool clear) {
+  // Remove previous bits - no need to clear
+  resize(0, false /* clear */);
+
+  initialize(new_size_in_bits, clear);
+}
+
+template <class BitMapWithAllocator>
+void GrowableBitMap<BitMapWithAllocator>::resize(idx_t new_size_in_bits, bool clear) {
+  const size_t old_size_in_bits = size();
+  bm_word_t* const old_map = map();
+
+  const size_t old_size_in_words = calc_size_in_words(size());
+  const size_t new_size_in_words = calc_size_in_words(new_size_in_bits);
+
+  BitMapWithAllocator* derived = static_cast<BitMapWithAllocator*>(this);
+
+  if (new_size_in_words == 0) {
     derived->free(old_map, old_size_in_words);
+    update(NULL, 0);
+    return;
   }
 
-  return map;
+
+  bm_word_t* map = derived->reallocate(old_map, old_size_in_words, new_size_in_words);
+  if (clear && (new_size_in_bits > old_size_in_bits)) {
+    // If old_size_in_bits is not word-aligned, then the preceding
+    // copy can include some trailing bits in the final copied word
+    // that also need to be cleared.  See clear_range_within_word.
+    bm_word_t mask = bit_mask(old_size_in_bits) - 1;
+    map[raw_to_words_align_down(old_size_in_bits)] &= mask;
+    // Clear the remaining full words.
+    clear_range_of_words(map, old_size_in_words, new_size_in_words);
+  }
+
+  update(map, new_size_in_bits);
 }
 
 ArenaBitMap::ArenaBitMap(Arena* arena, idx_t size_in_bits, bool clear)
@@ -79,6 +108,10 @@ bm_word_t* ArenaBitMap::allocate(idx_t size_in_words) const {
   return (bm_word_t*)_arena->Amalloc(size_in_words * BytesPerWord);
 }
 
+bm_word_t* ArenaBitMap::reallocate(bm_word_t* old_map, size_t old_size_in_words, size_t new_size_in_words) const {
+  return pseudo_reallocate(*this, old_map, old_size_in_words, new_size_in_words);
+}
+
 ResourceBitMap::ResourceBitMap(idx_t size_in_bits, bool clear)
   : GrowableBitMap<ResourceBitMap>() {
   initialize(size_in_bits, clear);
@@ -88,6 +121,10 @@ bm_word_t* ResourceBitMap::allocate(idx_t size_in_words) const {
   return (bm_word_t*)NEW_RESOURCE_ARRAY(bm_word_t, size_in_words);
 }
 
+bm_word_t* ResourceBitMap::reallocate(bm_word_t* old_map, size_t old_size_in_words, size_t new_size_in_words) const {
+  return pseudo_reallocate(*this, old_map, old_size_in_words, new_size_in_words);
+}
+
 CHeapBitMap::CHeapBitMap(idx_t size_in_bits, MEMFLAGS flags, bool clear)
   : GrowableBitMap<CHeapBitMap>(), _flags(flags) {
   initialize(size_in_bits, clear);
@@ -105,6 +142,10 @@ void CHeapBitMap::free(bm_word_t* map, idx_t size_in_words) const {
   ArrayAllocator<bm_word_t>::free(map, size_in_words);
 }
 
+bm_word_t* CHeapBitMap::reallocate(bm_word_t* map, size_t old_size_in_words, size_t new_size_in_words) const {
+  return ArrayAllocator<bm_word_t>::reallocate(map, old_size_in_words, new_size_in_words, _flags);
+}
+
 #ifdef ASSERT
 void BitMap::verify_size(idx_t size_in_bits) {
   assert(size_in_bits <= max_size_in_bits(),

src/hotspot/share/utilities/bitMap.hpp

@@ -326,43 +326,28 @@ class BitMap {
 template <class BitMapWithAllocator>
 class GrowableBitMap : public BitMap {
  protected:
-  bm_word_t* reallocate(bm_word_t* map, idx_t old_size_in_bits, idx_t new_size_in_bits, bool clear);
-
   GrowableBitMap() : GrowableBitMap(nullptr, 0) {}
   GrowableBitMap(bm_word_t* map, idx_t size_in_bits) : BitMap(map, size_in_bits) {}
 
  public:
-  // Set up and clear the bitmap memory.
+  // Set up and optionally clear the bitmap memory.
   //
   // Precondition: The bitmap was default constructed and has
   // not yet had memory allocated via resize or (re)initialize.
-  void initialize(idx_t size_in_bits, bool clear = true) {
-    assert(map() == NULL, "precondition");
-    assert(size() == 0,   "precondition");
-
-    resize(size_in_bits, clear);
-  }
+  void initialize(idx_t size_in_bits, bool clear = true);
 
-  // Set up and clear the bitmap memory.
+  // Set up and optionally clear the bitmap memory.
   //
   // Can be called on previously initialized bitmaps.
-  void reinitialize(idx_t new_size_in_bits, bool clear = true) {
-    // Remove previous bits - no need to clear
-    resize(0, false /* clear */);
-
-    initialize(new_size_in_bits, clear);
-  }
+  void reinitialize(idx_t new_size_in_bits, bool clear = true);
 
   // Protected functions, that are used by BitMap sub-classes that support them.
 
   // Resize the backing bitmap memory.
   //
   // Old bits are transferred to the new memory
-  // and the extended memory is cleared.
-  void resize(idx_t new_size_in_bits, bool clear = true) {
-    bm_word_t* new_map = reallocate(map(), size(), new_size_in_bits, clear);
-    update(new_map, new_size_in_bits);
-  }
+  // and the extended memory is optionally cleared.
+  void resize(idx_t new_size_in_bits, bool clear = true);
 };
 
 // A concrete implementation of the "abstract" BitMap class.
@@ -381,12 +366,12 @@ class ArenaBitMap : public GrowableBitMap<ArenaBitMap> {
   NONCOPYABLE(ArenaBitMap);
 
  public:
-  // Clears the bitmap memory.
   ArenaBitMap(Arena* arena, idx_t size_in_bits, bool clear = true);
 
   bm_word_t* allocate(idx_t size_in_words) const;
+  bm_word_t* reallocate(bm_word_t* old_map, size_t old_size_in_words, size_t new_size_in_words) const;
   void free(bm_word_t* map, idx_t size_in_words) const {
-    // ArenaBitMaps currently don't free memory.
+    // ArenaBitMaps don't free memory.
   }
 };
 
@@ -397,8 +382,9 @@ class ResourceBitMap : public GrowableBitMap<ResourceBitMap> {
   explicit ResourceBitMap(idx_t size_in_bits, bool clear = true);
 
   bm_word_t* allocate(idx_t size_in_words) const;
+  bm_word_t* reallocate(bm_word_t* old_map, size_t old_size_in_words, size_t new_size_in_words) const;
   void free(bm_word_t* map, idx_t size_in_words) const {
-    // ResourceBitMaps currently don't free memory.
+    // ResourceBitMaps don't free memory.
   }
 };
 
@@ -417,6 +403,7 @@ class CHeapBitMap : public GrowableBitMap<CHeapBitMap> {
   ~CHeapBitMap();
 
   bm_word_t* allocate(idx_t size_in_words) const;
+  bm_word_t* reallocate(bm_word_t* old_map, size_t old_size_in_words, size_t new_size_in_words) const;
   void free(bm_word_t* map, idx_t size_in_words) const;
 };
 

test/hotspot/gtest/memory/test_arrayAllocator.cpp

@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2022, 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.
+ */
+
+#include "precompiled.hpp"
+#include "memory/allocation.inline.hpp"
+#include "unittest.hpp"
+
+using Element = struct TestArrayAllocatorElement {
+  double a;
+  int b;
+};
+
+static void fill(Element* elements, int start, int size) {
+  for (int i = 0; i < size; i++) {
+    new (&elements[start + i]) Element{0.0, start + i};
+  }
+}
+
+static Element* allocate_and_fill(int size) {
+  Element* const elements = MallocArrayAllocator<Element>::allocate(size, mtTest);
+
+  fill(elements, 0, size);
+
+  return elements;
+}
+
+TEST_VM(ArrayAllocator, allocate) {
+  const int size = 10;
+
+  Element* const elements = allocate_and_fill(size);
+
+  for (int i = 0; i < size; i++) {
+    ASSERT_EQ(elements[i].b, i);
+  }
+
+  MallocArrayAllocator<Element>::free(elements);
+}
+
+TEST_VM(ArrayAllocator, reallocate_0) {
+  const int size = 10;
+
+  Element* const elements = allocate_and_fill(size);
+
+  Element* const ret = MallocArrayAllocator<Element>::reallocate(elements, 0, mtTest);
+  ASSERT_NE(ret, nullptr) << "We've chosen to NOT return nullptr when reallcting with 0";
+
+  MallocArrayAllocator<Element>::free(ret);
+}
+
+TEST_VM(ArrayAllocator, reallocate_shrink) {
+  const int size = 10;
+
+  Element* const elements = allocate_and_fill(size);
+
+  Element* const ret = MallocArrayAllocator<Element>::reallocate(elements, size / 2, mtTest);
+
+  for (int i = 0; i < size / 2; i++) {
+    ASSERT_EQ(ret[i].b, i);
+  }
+
+  MallocArrayAllocator<Element>::free(ret);
+}
+
+TEST_VM(ArrayAllocator, reallocate_grow) {
+  const int size = 10;
+
+  Element* const elements = allocate_and_fill(size);
+
+  Element* const ret = MallocArrayAllocator<Element>::reallocate(elements, size * 2, mtTest);
+
+  fill(ret, size, size);
+
+  for (int i = 0; i < size * 2; i++) {
+    ASSERT_EQ(ret[i].b, i);
+  }
+
+  MallocArrayAllocator<Element>::free(ret);
+}