changeset 274:818a18cd69a8

6730514: assertion failure in mangling code when expanding by 0 bytes Summary: An expansion by 0 bytes was not anticipated when the assertion was composed. Reviewed-by: jjh, jcoomes, apetrusenko
author jmasa
date Wed, 30 Jul 2008 11:54:00 -0700
parents 850fdf70db2b
children e8cf9b1f7c93
files make/windows/makefiles/defs.make src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.hpp src/share/vm/gc_implementation/parallelScavenge/psOldGen.cpp src/share/vm/gc_implementation/shared/spaceDecorator.cpp src/share/vm/memory/compactingPermGenGen.cpp src/share/vm/memory/compactingPermGenGen.hpp src/share/vm/memory/generation.cpp src/share/vm/memory/generation.hpp
diffstat 9 files changed, 88 insertions(+), 54 deletions(-) [+]
line wrap: on
line diff
--- a/make/windows/makefiles/defs.make	Mon Jul 28 15:30:23 2008 -0700
+++ b/make/windows/makefiles/defs.make	Wed Jul 30 11:54:00 2008 -0700
@@ -107,7 +107,7 @@
   ABS_OUTPUTDIR   := $(subst /,\\,$(shell /bin/cygpath -m -a "$(OUTPUTDIR)"))
   ABS_BOOTDIR     := $(subst /,\\,$(shell /bin/cygpath -m -a "$(BOOTDIR)"))
   ABS_GAMMADIR    := $(subst /,\\,$(shell /bin/cygpath -m -a "$(GAMMADIR)"))
-  ABS_OS_MAKEFILE := $(shell /bin/cygpath -m -a "$(HS_BUILD_DIR)/$(OSNAME)")/build.make
+  ABS_OS_MAKEFILE := $(shell /bin/cygpath -m -a "$(HS_MAKE_DIR)/$(OSNAME)")/build.make
 else
   ABS_OUTPUTDIR   := $(subst /,\\,$(shell $(CD) $(OUTPUTDIR);$(PWD)))
   ABS_BOOTDIR     := $(subst /,\\,$(shell $(CD) $(BOOTDIR);$(PWD)))
--- a/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp	Mon Jul 28 15:30:23 2008 -0700
+++ b/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp	Wed Jul 30 11:54:00 2008 -0700
@@ -3195,31 +3195,16 @@
 // YSR: All of this generation expansion/shrinking stuff is an exact copy of
 // OneContigSpaceCardGeneration, which makes me wonder if we should move this
 // to CardGeneration and share it...
+bool ConcurrentMarkSweepGeneration::expand(size_t bytes, size_t expand_bytes) {
+  return CardGeneration::expand(bytes, expand_bytes);
+}
+
 void ConcurrentMarkSweepGeneration::expand(size_t bytes, size_t expand_bytes,
   CMSExpansionCause::Cause cause)
 {
-  assert_locked_or_safepoint(Heap_lock);
-
-  size_t aligned_bytes  = ReservedSpace::page_align_size_up(bytes);
-  size_t aligned_expand_bytes = ReservedSpace::page_align_size_up(expand_bytes);
-  bool success = false;
-  if (aligned_expand_bytes > aligned_bytes) {
-    success = grow_by(aligned_expand_bytes);
-  }
-  if (!success) {
-    success = grow_by(aligned_bytes);
-  }
-  if (!success) {
-    size_t remaining_bytes = _virtual_space.uncommitted_size();
-    if (remaining_bytes > 0) {
-      success = grow_by(remaining_bytes);
-    }
-  }
-  if (GC_locker::is_active()) {
-    if (PrintGC && Verbose) {
-      gclog_or_tty->print_cr("Garbage collection disabled, expanded heap instead");
-    }
-  }
+
+  bool success = expand(bytes, expand_bytes);
+
   // remember why we expanded; this information is used
   // by shouldConcurrentCollect() when making decisions on whether to start
   // a new CMS cycle.
--- a/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.hpp	Mon Jul 28 15:30:23 2008 -0700
+++ b/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.hpp	Wed Jul 30 11:54:00 2008 -0700
@@ -1048,10 +1048,6 @@
   double _initiating_occupancy;
 
  protected:
-  // Grow generation by specified size (returns false if unable to grow)
-  bool grow_by(size_t bytes);
-  // Grow generation to reserved size.
-  bool grow_to_reserved();
   // Shrink generation by specified size (returns false if unable to shrink)
   virtual void shrink_by(size_t bytes);
 
@@ -1103,6 +1099,11 @@
   // Override
   virtual void ref_processor_init();
 
+  // Grow generation by specified size (returns false if unable to grow)
+  bool grow_by(size_t bytes);
+  // Grow generation to reserved size.
+  bool grow_to_reserved();
+
   void clear_expansion_cause() { _expansion_cause = CMSExpansionCause::_no_expansion; }
 
   // Space enquiries
@@ -1193,6 +1194,7 @@
   // Allocation failure
   void expand(size_t bytes, size_t expand_bytes,
     CMSExpansionCause::Cause cause);
+  virtual bool expand(size_t bytes, size_t expand_bytes);
   void shrink(size_t bytes);
   HeapWord* expand_and_par_lab_allocate(CMSParGCThreadState* ps, size_t word_sz);
   bool expand_and_ensure_spooling_space(PromotionInfo* promo);
--- a/src/share/vm/gc_implementation/parallelScavenge/psOldGen.cpp	Mon Jul 28 15:30:23 2008 -0700
+++ b/src/share/vm/gc_implementation/parallelScavenge/psOldGen.cpp	Wed Jul 30 11:54:00 2008 -0700
@@ -215,10 +215,22 @@
 }
 
 void PSOldGen::expand(size_t bytes) {
+  if (bytes == 0) {
+    return;
+  }
   MutexLocker x(ExpandHeap_lock);
   const size_t alignment = virtual_space()->alignment();
   size_t aligned_bytes  = align_size_up(bytes, alignment);
   size_t aligned_expand_bytes = align_size_up(MinHeapDeltaBytes, alignment);
+  if (aligned_bytes == 0){
+    // The alignment caused the number of bytes to wrap.  An expand_by(0) will
+    // return true with the implication that and expansion was done when it
+    // was not.  A call to expand implies a best effort to expand by "bytes"
+    // but not a guarantee.  Align down to give a best effort.  This is likely
+    // the most that the generation can expand since it has some capacity to
+    // start with.
+    aligned_bytes = align_size_down(bytes, alignment);
+  }
 
   bool success = false;
   if (aligned_expand_bytes > aligned_bytes) {
@@ -231,8 +243,8 @@
     success = expand_to_reserved();
   }
 
-  if (GC_locker::is_active()) {
-    if (PrintGC && Verbose) {
+  if (PrintGC && Verbose) {
+    if (success && GC_locker::is_active()) {
       gclog_or_tty->print_cr("Garbage collection disabled, expanded heap instead");
     }
   }
@@ -241,6 +253,9 @@
 bool PSOldGen::expand_by(size_t bytes) {
   assert_lock_strong(ExpandHeap_lock);
   assert_locked_or_safepoint(Heap_lock);
+  if (bytes == 0) {
+    return true;  // That's what virtual_space()->expand_by(0) would return
+  }
   bool result = virtual_space()->expand_by(bytes);
   if (result) {
     if (ZapUnusedHeapArea) {
--- a/src/share/vm/gc_implementation/shared/spaceDecorator.cpp	Mon Jul 28 15:30:23 2008 -0700
+++ b/src/share/vm/gc_implementation/shared/spaceDecorator.cpp	Wed Jul 30 11:54:00 2008 -0700
@@ -39,7 +39,8 @@
 
 void SpaceMangler::set_top_for_allocations(HeapWord* v)  {
   if (v < end()) {
-    assert(is_mangled(v), "The high water mark is not mangled");
+    assert(!CheckZapUnusedHeapArea || is_mangled(v),
+      "The high water mark is not mangled");
   }
   _top_for_allocations = v;
 }
--- a/src/share/vm/memory/compactingPermGenGen.cpp	Mon Jul 28 15:30:23 2008 -0700
+++ b/src/share/vm/memory/compactingPermGenGen.cpp	Wed Jul 30 11:54:00 2008 -0700
@@ -432,14 +432,16 @@
 }
 
 
-void CompactingPermGenGen::grow_to_reserved() {
+bool CompactingPermGenGen::grow_to_reserved() {
   // Don't allow _virtual_size to expand into shared spaces.
+  bool success = false;
   if (_virtual_space.uncommitted_size() > _shared_space_size) {
     size_t remaining_bytes =
       _virtual_space.uncommitted_size() - _shared_space_size;
-    bool success = OneContigSpaceCardGeneration::grow_by(remaining_bytes);
+    success = OneContigSpaceCardGeneration::grow_by(remaining_bytes);
     DEBUG_ONLY(if (!success) warning("grow to reserved failed");)
   }
+  return success;
 }
 
 
--- a/src/share/vm/memory/compactingPermGenGen.hpp	Mon Jul 28 15:30:23 2008 -0700
+++ b/src/share/vm/memory/compactingPermGenGen.hpp	Wed Jul 30 11:54:00 2008 -0700
@@ -184,7 +184,7 @@
   void post_compact();
   size_t contiguous_available() const;
   bool grow_by(size_t bytes);
-  void grow_to_reserved();
+  virtual bool grow_to_reserved();
 
   void clear_remembered_set();
   void invalidate_remembered_set();
--- a/src/share/vm/memory/generation.cpp	Mon Jul 28 15:30:23 2008 -0700
+++ b/src/share/vm/memory/generation.cpp	Wed Jul 30 11:54:00 2008 -0700
@@ -379,6 +379,41 @@
   }
 }
 
+bool CardGeneration::expand(size_t bytes, size_t expand_bytes) {
+  assert_locked_or_safepoint(Heap_lock);
+  if (bytes == 0) {
+    return true;  // That's what grow_by(0) would return
+  }
+  size_t aligned_bytes  = ReservedSpace::page_align_size_up(bytes);
+  if (aligned_bytes == 0){
+    // The alignment caused the number of bytes to wrap.  An expand_by(0) will
+    // return true with the implication that an expansion was done when it
+    // was not.  A call to expand implies a best effort to expand by "bytes"
+    // but not a guarantee.  Align down to give a best effort.  This is likely
+    // the most that the generation can expand since it has some capacity to
+    // start with.
+    aligned_bytes = ReservedSpace::page_align_size_down(bytes);
+  }
+  size_t aligned_expand_bytes = ReservedSpace::page_align_size_up(expand_bytes);
+  bool success = false;
+  if (aligned_expand_bytes > aligned_bytes) {
+    success = grow_by(aligned_expand_bytes);
+  }
+  if (!success) {
+    success = grow_by(aligned_bytes);
+  }
+  if (!success) {
+    success = grow_to_reserved();
+  }
+  if (PrintGC && Verbose) {
+    if (success && GC_locker::is_active()) {
+      gclog_or_tty->print_cr("Garbage collection disabled, expanded heap instead");
+    }
+  }
+
+  return success;
+}
+
 
 // No young generation references, clear this generation's cards.
 void CardGeneration::clear_remembered_set() {
@@ -441,25 +476,9 @@
   }
 }
 
-void OneContigSpaceCardGeneration::expand(size_t bytes, size_t expand_bytes) {
+bool OneContigSpaceCardGeneration::expand(size_t bytes, size_t expand_bytes) {
   GCMutexLocker x(ExpandHeap_lock);
-  size_t aligned_bytes  = ReservedSpace::page_align_size_up(bytes);
-  size_t aligned_expand_bytes = ReservedSpace::page_align_size_up(expand_bytes);
-  bool success = false;
-  if (aligned_expand_bytes > aligned_bytes) {
-    success = grow_by(aligned_expand_bytes);
-  }
-  if (!success) {
-    success = grow_by(aligned_bytes);
-  }
-  if (!success) {
-    grow_to_reserved();
-  }
-  if (GC_locker::is_active()) {
-    if (PrintGC && Verbose) {
-      gclog_or_tty->print_cr("Garbage collection disabled, expanded heap instead");
-    }
-  }
+  return CardGeneration::expand(bytes, expand_bytes);
 }
 
 
--- a/src/share/vm/memory/generation.hpp	Mon Jul 28 15:30:23 2008 -0700
+++ b/src/share/vm/memory/generation.hpp	Wed Jul 30 11:54:00 2008 -0700
@@ -606,11 +606,21 @@
 
  public:
 
+  // Attempt to expand the generation by "bytes".  Expand by at a
+  // minimum "expand_bytes".  Return true if some amount (not
+  // necessarily the full "bytes") was done.
+  virtual bool expand(size_t bytes, size_t expand_bytes);
+
   virtual void clear_remembered_set();
 
   virtual void invalidate_remembered_set();
 
   virtual void prepare_for_verify();
+
+  // Grow generation with specified size (returns false if unable to grow)
+  virtual bool grow_by(size_t bytes) = 0;
+  // Grow generation to reserved size.
+  virtual bool grow_to_reserved() = 0;
 };
 
 // OneContigSpaceCardGeneration models a heap of old objects contained in a single
@@ -631,14 +641,14 @@
                                       // and after last GC.
 
   // Grow generation with specified size (returns false if unable to grow)
-  bool grow_by(size_t bytes);
+  virtual bool grow_by(size_t bytes);
   // Grow generation to reserved size.
-  bool grow_to_reserved();
+  virtual bool grow_to_reserved();
   // Shrink generation with specified size (returns false if unable to shrink)
   void shrink_by(size_t bytes);
 
   // Allocation failure
-  void expand(size_t bytes, size_t expand_bytes);
+  virtual bool expand(size_t bytes, size_t expand_bytes);
   void shrink(size_t bytes);
 
   // Accessing spaces