8371465: Parallel: Revise asserts around heap expansion

Reviewed-by: aboldtch, tschatzl

src/hotspot/share/gc/parallel/mutableSpace.cpp

@@ -180,19 +180,6 @@ bool MutableSpace::cas_deallocate(HeapWord *obj, size_t size) {
   return AtomicAccess::cmpxchg(top_addr(), expected_top, obj) == expected_top;
 }
 
-// Only used by oldgen allocation.
-bool MutableSpace::needs_expand(size_t word_size) const {
-  // This method can be invoked either outside of safepoint by java threads or
-  // in safepoint by gc workers. Such accesses are synchronized by holding one
-  // of the following locks.
-  assert(Heap_lock->is_locked() || PSOldGenExpand_lock->is_locked(), "precondition");
-
-  // Holding the lock means end is stable.  So while top may be advancing
-  // via concurrent allocations, there is no need to order the reads of top
-  // and end here, unlike in cas_allocate.
-  return pointer_delta(end(), top()) < word_size;
-}
-
 void MutableSpace::oop_iterate(OopIterateClosure* cl) {
   HeapWord* obj_addr = bottom();
   HeapWord* t = top();

src/hotspot/share/gc/parallel/mutableSpace.hpp

@@ -127,11 +127,6 @@ public:
   virtual HeapWord* cas_allocate(size_t word_size);
   // Optional deallocation. Used in NUMA-allocator.
   bool cas_deallocate(HeapWord *obj, size_t size);
-  // Return true if this space needs to be expanded in order to satisfy an
-  // allocation request of the indicated size.  Concurrent allocations and
-  // resizes may change the result of a later call.  Used by oldgen allocator.
-  // precondition: holding PSOldGenExpand_lock if not VM thread
-  bool needs_expand(size_t word_size) const;
 
   // Iteration.
   void oop_iterate(OopIterateClosure* cl);

src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp

@@ -403,7 +403,16 @@ bool ParallelScavengeHeap::check_gc_overhead_limit() {
 }
 
 HeapWord* ParallelScavengeHeap::expand_heap_and_allocate(size_t size, bool is_tlab) {
+#ifdef ASSERT
   assert(Heap_lock->is_locked(), "precondition");
+  if (is_init_completed()) {
+    assert(SafepointSynchronize::is_at_safepoint(), "precondition");
+    assert(Thread::current()->is_VM_thread(), "precondition");
+  } else {
+    assert(Thread::current()->is_Java_thread(), "precondition");
+    assert(Heap_lock->owned_by_self(), "precondition");
+  }
+#endif
 
   HeapWord* result = young_gen()->expand_and_allocate(size);
 

src/hotspot/share/gc/parallel/psOldGen.cpp

@@ -33,6 +33,7 @@
 #include "gc/shared/spaceDecorator.hpp"
 #include "logging/log.hpp"
 #include "oops/oop.inline.hpp"
+#include "runtime/init.hpp"
 #include "runtime/java.hpp"
 #include "utilities/align.hpp"
 
@@ -118,13 +119,22 @@ void PSOldGen::initialize_performance_counters() {
 }
 
 HeapWord* PSOldGen::expand_and_allocate(size_t word_size) {
+#ifdef ASSERT
   assert(Heap_lock->is_locked(), "precondition");
+  if (is_init_completed()) {
+    assert(SafepointSynchronize::is_at_safepoint(), "precondition");
+    assert(Thread::current()->is_VM_thread(), "precondition");
+  } else {
+    assert(Thread::current()->is_Java_thread(), "precondition");
+    assert(Heap_lock->owned_by_self(), "precondition");
+  }
+#endif
 
-  if (object_space()->needs_expand(word_size)) {
+  if (pointer_delta(object_space()->end(), object_space()->top()) < word_size) {
     expand(word_size*HeapWordSize);
   }
 
-  // Reuse the CAS API even though this is VM thread in safepoint. This method
+  // Reuse the CAS API even though this is in a critical section. This method
   // is not invoked repeatedly, so the CAS overhead should be negligible.
   return cas_allocate_noexpand(word_size);
 }
@@ -168,7 +178,7 @@ bool PSOldGen::expand_for_allocate(size_t word_size) {
     // true until we expand, since we have the lock.  Other threads may take
     // the space we need before we can allocate it, regardless of whether we
     // expand.  That's okay, we'll just try expanding again.
-    if (object_space()->needs_expand(word_size)) {
+    if (pointer_delta(object_space()->end(), object_space()->top()) < word_size) {
       result = expand(word_size*HeapWordSize);
     }
   }
@@ -192,10 +202,21 @@ void PSOldGen::try_expand_till_size(size_t target_capacity_bytes) {
 
 bool PSOldGen::expand(size_t bytes) {
 #ifdef ASSERT
-  if (!Thread::current()->is_VM_thread()) {
-    assert_lock_strong(PSOldGenExpand_lock);
+  //  During startup (is_init_completed() == false), expansion can occur for
+  //    1. java-threads invoking heap-allocation (using Heap_lock)
+  //    2. CDS construction by a single thread (using PSOldGenExpand_lock but not needed)
+  //
+  //  After startup (is_init_completed() == true), expansion can occur for
+  //    1. GC workers for promoting to old-gen (using PSOldGenExpand_lock)
+  //    2. VM thread to satisfy the pending allocation
+  //    Both cases are inside safepoint pause, but are never overlapping.
+  //
+  if (is_init_completed()) {
+    assert(SafepointSynchronize::is_at_safepoint(), "precondition");
+    assert(Thread::current()->is_VM_thread() || PSOldGenExpand_lock->owned_by_self(), "precondition");
+  } else {
+    assert(Heap_lock->owned_by_self() || PSOldGenExpand_lock->owned_by_self(), "precondition");
   }
-  assert_locked_or_safepoint(Heap_lock);
   assert(bytes > 0, "precondition");
 #endif
   const size_t remaining_bytes = virtual_space()->uncommitted_size();