6807801: CMS: could save/restore fewer header words during scavenge
authorysr
Fri, 17 Dec 2010 23:41:31 -0800
changeset 7658 b970e410547a
parent 7657 a80e571c3d96
child 7659 1d92613a1e8a
6807801: CMS: could save/restore fewer header words during scavenge Summary: Age bits need not enter the mark-word preservation calculus; also affected, in addition to CMS, per CR synopsis above, were ParNew (but not DefNew), ParallelScavenge and G1, albeit to a lesser degree than CMS. Reviewed-by: tonyp, johnc
hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
hotspot/src/share/vm/gc_implementation/parNew/parNewGeneration.cpp
hotspot/src/share/vm/gc_implementation/parallelScavenge/psScavenge.cpp
hotspot/src/share/vm/memory/defNewGeneration.cpp
hotspot/src/share/vm/memory/defNewGeneration.hpp
hotspot/src/share/vm/oops/markOop.inline.hpp
--- a/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Fri Dec 17 11:26:53 2010 -0800
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Fri Dec 17 23:41:31 2010 -0800
@@ -3958,8 +3958,6 @@
   // Now restore saved marks, if any.
   if (_objs_with_preserved_marks != NULL) {
     assert(_preserved_marks_of_objs != NULL, "Both or none.");
-    assert(_objs_with_preserved_marks->length() ==
-           _preserved_marks_of_objs->length(), "Both or none.");
     guarantee(_objs_with_preserved_marks->length() ==
               _preserved_marks_of_objs->length(), "Both or none.");
     for (int i = 0; i < _objs_with_preserved_marks->length(); i++) {
@@ -4054,7 +4052,10 @@
 }
 
 void G1CollectedHeap::preserve_mark_if_necessary(oop obj, markOop m) {
-  if (m != markOopDesc::prototype()) {
+  assert(evacuation_failed(), "Oversaving!");
+  // We want to call the "for_promotion_failure" version only in the
+  // case of a promotion failure.
+  if (m->must_be_preserved_for_promotion_failure(obj)) {
     if (_objs_with_preserved_marks == NULL) {
       assert(_preserved_marks_of_objs == NULL, "Both or none.");
       _objs_with_preserved_marks =
--- a/hotspot/src/share/vm/gc_implementation/parNew/parNewGeneration.cpp	Fri Dec 17 11:26:53 2010 -0800
+++ b/hotspot/src/share/vm/gc_implementation/parNew/parNewGeneration.cpp	Fri Dec 17 23:41:31 2010 -0800
@@ -1058,10 +1058,11 @@
 #endif
 
 void ParNewGeneration::preserve_mark_if_necessary(oop obj, markOop m) {
-  if ((m != markOopDesc::prototype()) &&
-      (!UseBiasedLocking || (m != markOopDesc::biased_locking_prototype()))) {
+  if (m->must_be_preserved_for_promotion_failure(obj)) {
+    // We should really have separate per-worker stacks, rather
+    // than use locking of a common pair of stacks.
     MutexLocker ml(ParGCRareEvent_lock);
-    DefNewGeneration::preserve_mark_if_necessary(obj, m);
+    preserve_mark(obj, m);
   }
 }
 
--- a/hotspot/src/share/vm/gc_implementation/parallelScavenge/psScavenge.cpp	Fri Dec 17 11:26:53 2010 -0800
+++ b/hotspot/src/share/vm/gc_implementation/parallelScavenge/psScavenge.cpp	Fri Dec 17 23:41:31 2010 -0800
@@ -694,6 +694,8 @@
 void PSScavenge::oop_promotion_failed(oop obj, markOop obj_mark) {
   _promotion_failed = true;
   if (obj_mark->must_be_preserved_for_promotion_failure(obj)) {
+    // Should use per-worker private stakcs hetre rather than
+    // locking a common pair of stacks.
     ThreadCritical tc;
     _preserved_oop_stack.push(obj);
     _preserved_mark_stack.push(obj_mark);
--- a/hotspot/src/share/vm/memory/defNewGeneration.cpp	Fri Dec 17 11:26:53 2010 -0800
+++ b/hotspot/src/share/vm/memory/defNewGeneration.cpp	Fri Dec 17 23:41:31 2010 -0800
@@ -684,23 +684,28 @@
   _preserved_marks_of_objs.clear(true);
 }
 
+void DefNewGeneration::preserve_mark(oop obj, markOop m) {
+  assert(promotion_failed() && m->must_be_preserved_for_promotion_failure(obj),
+         "Oversaving!");
+  _objs_with_preserved_marks.push(obj);
+  _preserved_marks_of_objs.push(m);
+}
+
 void DefNewGeneration::preserve_mark_if_necessary(oop obj, markOop m) {
   if (m->must_be_preserved_for_promotion_failure(obj)) {
-    _objs_with_preserved_marks.push(obj);
-    _preserved_marks_of_objs.push(m);
+    preserve_mark(obj, m);
   }
 }
 
 void DefNewGeneration::handle_promotion_failure(oop old) {
-  preserve_mark_if_necessary(old, old->mark());
-  if (!_promotion_failed && PrintPromotionFailure) {
+  if (PrintPromotionFailure && !_promotion_failed) {
     gclog_or_tty->print(" (promotion failure size = " SIZE_FORMAT ") ",
                         old->size());
   }
-
+  _promotion_failed = true;
+  preserve_mark_if_necessary(old, old->mark());
   // forward to self
   old->forward_to(old);
-  _promotion_failed = true;
 
   _promo_failure_scan_stack.push(old);
 
--- a/hotspot/src/share/vm/memory/defNewGeneration.hpp	Fri Dec 17 11:26:53 2010 -0800
+++ b/hotspot/src/share/vm/memory/defNewGeneration.hpp	Fri Dec 17 23:41:31 2010 -0800
@@ -85,6 +85,7 @@
   // Preserve the mark of "obj", if necessary, in preparation for its mark
   // word being overwritten with a self-forwarding-pointer.
   void   preserve_mark_if_necessary(oop obj, markOop m);
+  void   preserve_mark(oop obj, markOop m);    // work routine used by the above
 
   // Together, these keep <object with a preserved mark, mark value> pairs.
   // They should always contain the same number of elements.
--- a/hotspot/src/share/vm/oops/markOop.inline.hpp	Fri Dec 17 11:26:53 2010 -0800
+++ b/hotspot/src/share/vm/oops/markOop.inline.hpp	Fri Dec 17 23:41:31 2010 -0800
@@ -30,7 +30,7 @@
 #include "oops/markOop.hpp"
 #include "runtime/globals.hpp"
 
-// Should this header be preserved during GC?
+// Should this header be preserved during GC (when biased locking is enabled)?
 inline bool markOopDesc::must_be_preserved_with_bias(oop obj_containing_mark) const {
   assert(UseBiasedLocking, "unexpected");
   if (has_bias_pattern()) {
@@ -47,14 +47,15 @@
   return (!is_unlocked() || !has_no_hash());
 }
 
+// Should this header be preserved during GC?
 inline bool markOopDesc::must_be_preserved(oop obj_containing_mark) const {
   if (!UseBiasedLocking)
     return (!is_unlocked() || !has_no_hash());
   return must_be_preserved_with_bias(obj_containing_mark);
 }
 
-// Should this header (including its age bits) be preserved in the
-// case of a promotion failure during scavenge?
+// Should this header be preserved in the case of a promotion failure
+// during scavenge (when biased locking is enabled)?
 inline bool markOopDesc::must_be_preserved_with_bias_for_promotion_failure(oop obj_containing_mark) const {
   assert(UseBiasedLocking, "unexpected");
   // We don't explicitly save off the mark words of biased and
@@ -70,18 +71,20 @@
       prototype_for_object(obj_containing_mark)->has_bias_pattern()) {
     return true;
   }
-  return (this != prototype());
+  return (!is_unlocked() || !has_no_hash());
 }
 
+// Should this header be preserved in the case of a promotion failure
+// during scavenge?
 inline bool markOopDesc::must_be_preserved_for_promotion_failure(oop obj_containing_mark) const {
   if (!UseBiasedLocking)
-    return (this != prototype());
+    return (!is_unlocked() || !has_no_hash());
   return must_be_preserved_with_bias_for_promotion_failure(obj_containing_mark);
 }
 
 
-// Should this header (including its age bits) be preserved in the
-// case of a scavenge in which CMS is the old generation?
+// Same as must_be_preserved_with_bias_for_promotion_failure() except that
+// it takes a klassOop argument, instead of the object of which this is the mark word.
 inline bool markOopDesc::must_be_preserved_with_bias_for_cms_scavenge(klassOop klass_of_obj_containing_mark) const {
   assert(UseBiasedLocking, "unexpected");
   // CMS scavenges preserve mark words in similar fashion to promotion failures; see above
@@ -89,11 +92,14 @@
       klass_of_obj_containing_mark->klass_part()->prototype_header()->has_bias_pattern()) {
     return true;
   }
-  return (this != prototype());
+  return (!is_unlocked() || !has_no_hash());
 }
+
+// Same as must_be_preserved_for_promotion_failure() except that
+// it takes a klassOop argument, instead of the object of which this is the mark word.
 inline bool markOopDesc::must_be_preserved_for_cms_scavenge(klassOop klass_of_obj_containing_mark) const {
   if (!UseBiasedLocking)
-    return (this != prototype());
+    return (!is_unlocked() || !has_no_hash());
   return must_be_preserved_with_bias_for_cms_scavenge(klass_of_obj_containing_mark);
 }