8170395: Metaspace initialization queries the wrong chunk freelist
authorstefank
Tue, 29 Nov 2016 19:25:36 +0100
changeset 42615 86c4562176fa
parent 42613 29c73ac33ad3
child 42616 62e6f7e7fde2
8170395: Metaspace initialization queries the wrong chunk freelist Reviewed-by: mgerdin, pliden, stuefe
hotspot/src/share/vm/memory/metaspace.cpp
hotspot/src/share/vm/memory/metaspace.hpp
hotspot/test/native/memory/test_spaceManager.cpp
--- a/hotspot/src/share/vm/memory/metaspace.cpp	Mon Nov 28 18:54:30 2016 +0300
+++ b/hotspot/src/share/vm/memory/metaspace.cpp	Tue Nov 29 19:25:36 2016 +0100
@@ -596,9 +596,8 @@
 
   size_t free_bytes();
 
-  Metachunk* get_new_chunk(size_t word_size,
-                           size_t grow_chunks_by_words,
-                           size_t medium_chunk_bunch);
+  Metachunk* get_new_chunk(size_t chunk_word_size,
+                           size_t suggested_commit_granularity);
 
   bool expand_node_by(VirtualSpaceNode* node,
                       size_t min_words,
@@ -749,15 +748,22 @@
     MediumChunkMultiple = 4
   };
 
-  bool is_class() { return _mdtype == Metaspace::ClassType; }
+  static size_t specialized_chunk_size(bool is_class) { return is_class ? ClassSpecializedChunk : SpecializedChunk; }
+  static size_t small_chunk_size(bool is_class)       { return is_class ? ClassSmallChunk : SmallChunk; }
+  static size_t medium_chunk_size(bool is_class)      { return is_class ? ClassMediumChunk : MediumChunk; }
+
+  static size_t smallest_chunk_size(bool is_class)    { return specialized_chunk_size(is_class); }
 
   // Accessors
-  size_t specialized_chunk_size() { return (size_t) is_class() ? ClassSpecializedChunk : SpecializedChunk; }
-  size_t small_chunk_size()       { return (size_t) is_class() ? ClassSmallChunk : SmallChunk; }
-  size_t medium_chunk_size()      { return (size_t) is_class() ? ClassMediumChunk : MediumChunk; }
-  size_t medium_chunk_bunch()     { return medium_chunk_size() * MediumChunkMultiple; }
-
-  size_t smallest_chunk_size()  { return specialized_chunk_size(); }
+  bool is_class() const { return _mdtype == Metaspace::ClassType; }
+
+  size_t specialized_chunk_size() const { return specialized_chunk_size(is_class()); }
+  size_t small_chunk_size()       const { return small_chunk_size(is_class()); }
+  size_t medium_chunk_size()      const { return medium_chunk_size(is_class()); }
+
+  size_t smallest_chunk_size()    const { return smallest_chunk_size(is_class()); }
+
+  size_t medium_chunk_bunch()     const { return medium_chunk_size() * MediumChunkMultiple; }
 
   size_t allocated_blocks_words() const { return _allocated_blocks_words; }
   size_t allocated_blocks_bytes() const { return _allocated_blocks_words * BytesPerWord; }
@@ -781,10 +787,13 @@
   // decremented for all the Metachunks in-use by this SpaceManager.
   void dec_total_from_size_metrics();
 
-  // Set the sizes for the initial chunks.
-  void get_initial_chunk_sizes(Metaspace::MetaspaceType type,
-                               size_t* chunk_word_size,
-                               size_t* class_chunk_word_size);
+  // Adjust the initial chunk size to match one of the fixed chunk list sizes,
+  // or return the unadjusted size if the requested size is humongous.
+  static size_t adjust_initial_chunk_size(size_t requested, bool is_class_space);
+  size_t adjust_initial_chunk_size(size_t requested) const;
+
+  // Get the initial chunks size for this metaspace type.
+  size_t get_initial_chunk_size(Metaspace::MetaspaceType type) const;
 
   size_t sum_capacity_in_chunks_in_use() const;
   size_t sum_used_in_chunks_in_use() const;
@@ -795,7 +804,7 @@
   size_t sum_count_in_chunks_in_use();
   size_t sum_count_in_chunks_in_use(ChunkIndex i);
 
-  Metachunk* get_new_chunk(size_t word_size, size_t grow_chunks_by_words);
+  Metachunk* get_new_chunk(size_t chunk_word_size);
 
   // Block allocation and deallocation.
   // Allocates a block from the current chunk
@@ -1400,12 +1409,10 @@
   return false;
 }
 
-Metachunk* VirtualSpaceList::get_new_chunk(size_t word_size,
-                                           size_t grow_chunks_by_words,
-                                           size_t medium_chunk_bunch) {
+Metachunk* VirtualSpaceList::get_new_chunk(size_t chunk_word_size, size_t suggested_commit_granularity) {
 
   // Allocate a chunk out of the current virtual space.
-  Metachunk* next = current_virtual_space()->get_chunk_vs(grow_chunks_by_words);
+  Metachunk* next = current_virtual_space()->get_chunk_vs(chunk_word_size);
 
   if (next != NULL) {
     return next;
@@ -1414,8 +1421,8 @@
   // The expand amount is currently only determined by the requested sizes
   // and not how much committed memory is left in the current virtual space.
 
-  size_t min_word_size       = align_size_up(grow_chunks_by_words, Metaspace::commit_alignment_words());
-  size_t preferred_word_size = align_size_up(medium_chunk_bunch,   Metaspace::commit_alignment_words());
+  size_t min_word_size       = align_size_up(chunk_word_size,              Metaspace::commit_alignment_words());
+  size_t preferred_word_size = align_size_up(suggested_commit_granularity, Metaspace::commit_alignment_words());
   if (min_word_size >= preferred_word_size) {
     // Can happen when humongous chunks are allocated.
     preferred_word_size = min_word_size;
@@ -1423,7 +1430,7 @@
 
   bool expanded = expand_by(min_word_size, preferred_word_size);
   if (expanded) {
-    next = current_virtual_space()->get_chunk_vs(grow_chunks_by_words);
+    next = current_virtual_space()->get_chunk_vs(chunk_word_size);
     assert(next != NULL, "The allocation was expected to succeed after the expansion");
   }
 
@@ -1917,36 +1924,58 @@
 
 // SpaceManager methods
 
-void SpaceManager::get_initial_chunk_sizes(Metaspace::MetaspaceType type,
-                                           size_t* chunk_word_size,
-                                           size_t* class_chunk_word_size) {
-  switch (type) {
-  case Metaspace::BootMetaspaceType:
-    *chunk_word_size = Metaspace::first_chunk_word_size();
-    *class_chunk_word_size = Metaspace::first_class_chunk_word_size();
-    break;
-  case Metaspace::ROMetaspaceType:
-    *chunk_word_size = SharedReadOnlySize / wordSize;
-    *class_chunk_word_size = ClassSpecializedChunk;
-    break;
-  case Metaspace::ReadWriteMetaspaceType:
-    *chunk_word_size = SharedReadWriteSize / wordSize;
-    *class_chunk_word_size = ClassSpecializedChunk;
-    break;
-  case Metaspace::AnonymousMetaspaceType:
-  case Metaspace::ReflectionMetaspaceType:
-    *chunk_word_size = SpecializedChunk;
-    *class_chunk_word_size = ClassSpecializedChunk;
-    break;
-  default:
-    *chunk_word_size = SmallChunk;
-    *class_chunk_word_size = ClassSmallChunk;
-    break;
+size_t SpaceManager::adjust_initial_chunk_size(size_t requested, bool is_class_space) {
+  size_t chunk_sizes[] = {
+      specialized_chunk_size(is_class_space),
+      small_chunk_size(is_class_space),
+      medium_chunk_size(is_class_space)
+  };
+
+  // Adjust up to one of the fixed chunk sizes ...
+  for (size_t i = 0; i < ARRAY_SIZE(chunk_sizes); i++) {
+    if (requested <= chunk_sizes[i]) {
+      return chunk_sizes[i];
+    }
   }
-  assert(*chunk_word_size != 0 && *class_chunk_word_size != 0,
-         "Initial chunks sizes bad: data  " SIZE_FORMAT
-         " class " SIZE_FORMAT,
-         *chunk_word_size, *class_chunk_word_size);
+
+  // ... or return the size as a humongous chunk.
+  return requested;
+}
+
+size_t SpaceManager::adjust_initial_chunk_size(size_t requested) const {
+  return adjust_initial_chunk_size(requested, is_class());
+}
+
+size_t SpaceManager::get_initial_chunk_size(Metaspace::MetaspaceType type) const {
+  size_t requested;
+
+  if (is_class()) {
+    switch (type) {
+    case Metaspace::BootMetaspaceType:       requested = Metaspace::first_class_chunk_word_size(); break;
+    case Metaspace::ROMetaspaceType:         requested = ClassSpecializedChunk; break;
+    case Metaspace::ReadWriteMetaspaceType:  requested = ClassSpecializedChunk; break;
+    case Metaspace::AnonymousMetaspaceType:  requested = ClassSpecializedChunk; break;
+    case Metaspace::ReflectionMetaspaceType: requested = ClassSpecializedChunk; break;
+    default:                                 requested = ClassSmallChunk; break;
+    }
+  } else {
+    switch (type) {
+    case Metaspace::BootMetaspaceType:       requested = Metaspace::first_chunk_word_size(); break;
+    case Metaspace::ROMetaspaceType:         requested = SharedReadOnlySize / wordSize; break;
+    case Metaspace::ReadWriteMetaspaceType:  requested = SharedReadWriteSize / wordSize; break;
+    case Metaspace::AnonymousMetaspaceType:  requested = SpecializedChunk; break;
+    case Metaspace::ReflectionMetaspaceType: requested = SpecializedChunk; break;
+    default:                                 requested = SmallChunk; break;
+    }
+  }
+
+  // Adjust to one of the fixed chunk sizes (unless humongous)
+  const size_t adjusted = adjust_initial_chunk_size(requested);
+
+  assert(adjusted != 0, "Incorrect initial chunk size. Requested: "
+         SIZE_FORMAT " adjusted: " SIZE_FORMAT, requested, adjusted);
+
+  return adjusted;
 }
 
 size_t SpaceManager::sum_free_in_chunks_in_use() const {
@@ -2131,8 +2160,8 @@
   }
 
   // Get another chunk
-  size_t grow_chunks_by_words = calc_chunk_size(word_size);
-  Metachunk* next = get_new_chunk(word_size, grow_chunks_by_words);
+  size_t chunk_word_size = calc_chunk_size(word_size);
+  Metachunk* next = get_new_chunk(chunk_word_size);
 
   MetaWord* mem = NULL;
 
@@ -2431,14 +2460,12 @@
   }
 }
 
-Metachunk* SpaceManager::get_new_chunk(size_t word_size,
-                                       size_t grow_chunks_by_words) {
+Metachunk* SpaceManager::get_new_chunk(size_t chunk_word_size) {
   // Get a chunk from the chunk freelist
-  Metachunk* next = chunk_manager()->chunk_freelist_allocate(grow_chunks_by_words);
+  Metachunk* next = chunk_manager()->chunk_freelist_allocate(chunk_word_size);
 
   if (next == NULL) {
-    next = vs_list()->get_new_chunk(word_size,
-                                    grow_chunks_by_words,
+    next = vs_list()->get_new_chunk(chunk_word_size,
                                     medium_chunk_bunch());
   }
 
@@ -3176,7 +3203,7 @@
          SIZE_FORMAT " != " SIZE_FORMAT, rs.size(), CompressedClassSpaceSize);
   assert(using_class_space(), "Must be using class space");
   _class_space_list = new VirtualSpaceList(rs);
-  _chunk_manager_class = new ChunkManager(SpecializedChunk, ClassSmallChunk, ClassMediumChunk);
+  _chunk_manager_class = new ChunkManager(ClassSpecializedChunk, ClassSmallChunk, ClassMediumChunk);
 
   if (!_class_space_list->initialization_succeeded()) {
     vm_exit_during_initialization("Failed to setup compressed class space virtual space list.");
@@ -3346,75 +3373,62 @@
   MetaspaceGC::post_initialize();
 }
 
-Metachunk* Metaspace::get_initialization_chunk(MetadataType mdtype,
-                                               size_t chunk_word_size,
-                                               size_t chunk_bunch) {
+void Metaspace::initialize_first_chunk(MetaspaceType type, MetadataType mdtype) {
+  Metachunk* chunk = get_initialization_chunk(type, mdtype);
+  if (chunk != NULL) {
+    // Add to this manager's list of chunks in use and current_chunk().
+    get_space_manager(mdtype)->add_chunk(chunk, true);
+  }
+}
+
+Metachunk* Metaspace::get_initialization_chunk(MetaspaceType type, MetadataType mdtype) {
+  size_t chunk_word_size = get_space_manager(mdtype)->get_initial_chunk_size(type);
+
   // Get a chunk from the chunk freelist
   Metachunk* chunk = get_chunk_manager(mdtype)->chunk_freelist_allocate(chunk_word_size);
-  if (chunk != NULL) {
-    return chunk;
+
+  if (chunk == NULL) {
+    chunk = get_space_list(mdtype)->get_new_chunk(chunk_word_size,
+                                                  get_space_manager(mdtype)->medium_chunk_bunch());
+  }
+
+  // For dumping shared archive, report error if allocation has failed.
+  if (DumpSharedSpaces && chunk == NULL) {
+    report_insufficient_metaspace(MetaspaceAux::committed_bytes() + chunk_word_size * BytesPerWord);
   }
 
-  return get_space_list(mdtype)->get_new_chunk(chunk_word_size, chunk_word_size, chunk_bunch);
+  return chunk;
+}
+
+void Metaspace::verify_global_initialization() {
+  assert(space_list() != NULL, "Metadata VirtualSpaceList has not been initialized");
+  assert(chunk_manager_metadata() != NULL, "Metadata ChunkManager has not been initialized");
+
+  if (using_class_space()) {
+    assert(class_space_list() != NULL, "Class VirtualSpaceList has not been initialized");
+    assert(chunk_manager_class() != NULL, "Class ChunkManager has not been initialized");
+  }
 }
 
 void Metaspace::initialize(Mutex* lock, MetaspaceType type) {
-
-  assert(space_list() != NULL,
-    "Metadata VirtualSpaceList has not been initialized");
-  assert(chunk_manager_metadata() != NULL,
-    "Metadata ChunkManager has not been initialized");
-
+  verify_global_initialization();
+
+  // Allocate SpaceManager for metadata objects.
   _vsm = new SpaceManager(NonClassType, lock);
-  if (_vsm == NULL) {
-    return;
-  }
-  size_t word_size;
-  size_t class_word_size;
-  vsm()->get_initial_chunk_sizes(type, &word_size, &class_word_size);
 
   if (using_class_space()) {
-  assert(class_space_list() != NULL,
-    "Class VirtualSpaceList has not been initialized");
-  assert(chunk_manager_class() != NULL,
-    "Class ChunkManager has not been initialized");
-
     // Allocate SpaceManager for classes.
     _class_vsm = new SpaceManager(ClassType, lock);
-    if (_class_vsm == NULL) {
-      return;
-    }
   }
 
   MutexLockerEx cl(SpaceManager::expand_lock(), Mutex::_no_safepoint_check_flag);
 
   // Allocate chunk for metadata objects
-  Metachunk* new_chunk = get_initialization_chunk(NonClassType,
-                                                  word_size,
-                                                  vsm()->medium_chunk_bunch());
-  // For dumping shared archive, report error if allocation has failed.
-  if (DumpSharedSpaces && new_chunk == NULL) {
-    report_insufficient_metaspace(MetaspaceAux::committed_bytes() + word_size * BytesPerWord);
-  }
-  assert(!DumpSharedSpaces || new_chunk != NULL, "should have enough space for both chunks");
-  if (new_chunk != NULL) {
-    // Add to this manager's list of chunks in use and current_chunk().
-    vsm()->add_chunk(new_chunk, true);
-  }
+  initialize_first_chunk(type, NonClassType);
 
   // Allocate chunk for class metadata objects
   if (using_class_space()) {
-    Metachunk* class_chunk = get_initialization_chunk(ClassType,
-                                                      class_word_size,
-                                                      class_vsm()->medium_chunk_bunch());
-    if (class_chunk != NULL) {
-      class_vsm()->add_chunk(class_chunk, true);
-    } else {
-      // For dumping shared archive, report error if allocation has failed.
-      if (DumpSharedSpaces) {
-        report_insufficient_metaspace(MetaspaceAux::committed_bytes() + class_word_size * BytesPerWord);
-      }
-    }
+    initialize_first_chunk(type, ClassType);
   }
 
   _alloc_record_head = NULL;
@@ -3840,7 +3854,7 @@
     // vm_allocation_granularity aligned on Windows.
     size_t large_size = (size_t)(2*256*K + (os::vm_page_size()/BytesPerWord));
     large_size += (os::vm_page_size()/BytesPerWord);
-    vs_list->get_new_chunk(large_size, large_size, 0);
+    vs_list->get_new_chunk(large_size, 0);
   }
 
   static void test() {
@@ -4017,4 +4031,54 @@
   TestVirtualSpaceNodeTest::test();
   TestVirtualSpaceNodeTest::test_is_available();
 }
+
+// The following test is placed here instead of a gtest / unittest file
+// because the ChunkManager class is only available in this file.
+class SpaceManagerTest : AllStatic {
+  friend void SpaceManager_test_adjust_initial_chunk_size();
+
+  static void test_adjust_initial_chunk_size(bool is_class) {
+    const size_t smallest = SpaceManager::smallest_chunk_size(is_class);
+    const size_t normal   = SpaceManager::small_chunk_size(is_class);
+    const size_t medium   = SpaceManager::medium_chunk_size(is_class);
+
+#define test_adjust_initial_chunk_size(value, expected, is_class_value)          \
+    do {                                                                         \
+      size_t v = value;                                                          \
+      size_t e = expected;                                                       \
+      assert(SpaceManager::adjust_initial_chunk_size(v, (is_class_value)) == e,  \
+             "Expected: " SIZE_FORMAT " got: " SIZE_FORMAT, e, v);               \
+    } while (0)
+
+    // Smallest (specialized)
+    test_adjust_initial_chunk_size(1,            smallest, is_class);
+    test_adjust_initial_chunk_size(smallest - 1, smallest, is_class);
+    test_adjust_initial_chunk_size(smallest,     smallest, is_class);
+
+    // Small
+    test_adjust_initial_chunk_size(smallest + 1, normal, is_class);
+    test_adjust_initial_chunk_size(normal - 1,   normal, is_class);
+    test_adjust_initial_chunk_size(normal,       normal, is_class);
+
+    // Medium
+    test_adjust_initial_chunk_size(normal + 1, medium, is_class);
+    test_adjust_initial_chunk_size(medium - 1, medium, is_class);
+    test_adjust_initial_chunk_size(medium,     medium, is_class);
+
+    // Humongous
+    test_adjust_initial_chunk_size(medium + 1, medium + 1, is_class);
+
+#undef test_adjust_initial_chunk_size
+  }
+
+  static void test_adjust_initial_chunk_size() {
+    test_adjust_initial_chunk_size(false);
+    test_adjust_initial_chunk_size(true);
+  }
+};
+
+void SpaceManager_test_adjust_initial_chunk_size() {
+  SpaceManagerTest::test_adjust_initial_chunk_size();
+}
+
 #endif
--- a/hotspot/src/share/vm/memory/metaspace.hpp	Mon Nov 28 18:54:30 2016 +0300
+++ b/hotspot/src/share/vm/memory/metaspace.hpp	Tue Nov 29 19:25:36 2016 +0100
@@ -105,14 +105,15 @@
   };
 
  private:
+  static void verify_global_initialization();
+
   void initialize(Mutex* lock, MetaspaceType type);
 
-  // Get the first chunk for a Metaspace.  Used for
+  // Initialize the first chunk for a Metaspace.  Used for
   // special cases such as the boot class loader, reflection
   // class loader and anonymous class loader.
-  Metachunk* get_initialization_chunk(MetadataType mdtype,
-                                      size_t chunk_word_size,
-                                      size_t chunk_bunch);
+  void initialize_first_chunk(MetaspaceType type, MetadataType mdtype);
+  Metachunk* get_initialization_chunk(MetaspaceType type, MetadataType mdtype);
 
   // Align up the word size to the allocation word size
   static size_t align_word_size_up(size_t);
@@ -139,6 +140,10 @@
 
   SpaceManager* _class_vsm;
   SpaceManager* class_vsm() const { return _class_vsm; }
+  SpaceManager* get_space_manager(MetadataType mdtype) {
+    assert(mdtype != MetadataTypeCount, "MetadaTypeCount can't be used as mdtype");
+    return mdtype == ClassType ? class_vsm() : vsm();
+  }
 
   // Allocate space for metadata of type mdtype. This is space
   // within a Metachunk and is used by
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/native/memory/test_spaceManager.cpp	Tue Nov 29 19:25:36 2016 +0100
@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2016, 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"
+
+// The test function is only available in debug builds
+#ifdef ASSERT
+
+#include "unittest.hpp"
+
+void SpaceManager_test_adjust_initial_chunk_size();
+
+TEST(SpaceManager, adjust_initial_chunk_size) {
+  // The SpaceManager is only available in metaspace.cpp,
+  // so the test code is located in that file.
+  SpaceManager_test_adjust_initial_chunk_size();
+}
+
+#endif // ASSERT