# HG changeset patch # User ysr # Date 1292658091 28800 # Node ID 74ee0db180fa7788c7f41b4db39b8b9e9bc39bd4 # Parent b03260081e9bae5a109752871b338b37daa69bb5 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 diff -r b03260081e9b -r 74ee0db180fa src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp --- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp Fri Dec 17 11:26:53 2010 -0800 +++ b/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 = diff -r b03260081e9b -r 74ee0db180fa src/share/vm/gc_implementation/parNew/parNewGeneration.cpp --- a/src/share/vm/gc_implementation/parNew/parNewGeneration.cpp Fri Dec 17 11:26:53 2010 -0800 +++ b/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); } } diff -r b03260081e9b -r 74ee0db180fa src/share/vm/gc_implementation/parallelScavenge/psScavenge.cpp --- a/src/share/vm/gc_implementation/parallelScavenge/psScavenge.cpp Fri Dec 17 11:26:53 2010 -0800 +++ b/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); diff -r b03260081e9b -r 74ee0db180fa src/share/vm/memory/defNewGeneration.cpp --- a/src/share/vm/memory/defNewGeneration.cpp Fri Dec 17 11:26:53 2010 -0800 +++ b/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); diff -r b03260081e9b -r 74ee0db180fa src/share/vm/memory/defNewGeneration.hpp --- a/src/share/vm/memory/defNewGeneration.hpp Fri Dec 17 11:26:53 2010 -0800 +++ b/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 pairs. // They should always contain the same number of elements. diff -r b03260081e9b -r 74ee0db180fa src/share/vm/oops/markOop.inline.hpp --- a/src/share/vm/oops/markOop.inline.hpp Fri Dec 17 11:26:53 2010 -0800 +++ b/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); }