SUGGESTED FIX
The following delta is under testing and review:-
--- old/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp Tue Dec 14 15:36:51 2010
+++ new/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp Tue Dec 14 15:36:50 2010
@@ -3950,8 +3950,7 @@
// 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.");
+ // XXX Change to a plain old assert ?
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++) {
@@ -4046,7 +4045,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 =
--- old/src/share/vm/gc_implementation/parNew/parNewGeneration.cpp Tue Dec 14 15:36:55 2010
+++ new/src/share/vm/gc_implementation/parNew/parNewGeneration.cpp Tue Dec 14 15:36:54 2010
@@ -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);
}
}
--- old/src/share/vm/gc_implementation/parallelScavenge/psScavenge.cpp Tue Dec 14 15:36:57 2010
+++ new/src/share/vm/gc_implementation/parallelScavenge/psScavenge.cpp Tue Dec 14 15:36:56 2010
@@ -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);
--- old/src/share/vm/memory/defNewGeneration.cpp Tue Dec 14 15:36:59 2010
+++ new/src/share/vm/memory/defNewGeneration.cpp Tue Dec 14 15:36:58 2010
@@ -684,14 +684,21 @@
_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) {
+ _promotion_failed = true;
preserve_mark_if_necessary(old, old->mark());
if (!_promotion_failed && PrintPromotionFailure) {
gclog_or_tty->print(" (promotion failure size = " SIZE_FORMAT ") ",
@@ -700,7 +707,6 @@
// forward to self
old->forward_to(old);
- _promotion_failed = true;
_promo_failure_scan_stack.push(old);
--- old/src/share/vm/memory/defNewGeneration.hpp Tue Dec 14 15:37:01 2010
+++ new/src/share/vm/memory/defNewGeneration.hpp Tue Dec 14 15:37:00 2010
@@ -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.
--- old/src/share/vm/oops/markOop.inline.hpp Tue Dec 14 15:37:02 2010
+++ new/src/share/vm/oops/markOop.inline.hpp Tue Dec 14 15:37:02 2010
@@ -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,6 +47,7 @@
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());
@@ -53,8 +54,8 @@
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);
}
|