Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unconditionally preserve all marks during GC #6

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -3061,7 +3061,7 @@ void G1CollectedHeap::do_collection_pause_at_safepoint_helper(double target_paus

void G1CollectedHeap::preserve_mark_during_evac_failure(uint worker_id, oop obj, markWord m) {
_evacuation_failed_info_array[worker_id].register_copy_failure(obj->size());
_preserved_marks_set.get(worker_id)->push_if_necessary(obj, m);
_preserved_marks_set.get(worker_id)->push(obj, m);
}

bool G1ParEvacuateFollowersClosure::offer_termination() {
@@ -113,14 +113,6 @@ void G1FullGCCompactionPoint::forward(oop object, size_t size) {
// even if the mark-word is used. This is no problem since
// forwardee() will return NULL in the compaction phase as well.
object->init_mark();
} else {
// Make sure object has the correct mark-word set or that it will be
// fixed when restoring the preserved marks.
assert(object->mark() == markWord::prototype_for_klass(object->klass()) || // Correct mark
object->mark_must_be_preserved() || // Will be restored by PreservedMarksSet
shipilev marked this conversation as resolved.
Show resolved Hide resolved
(UseBiasedLocking && object->has_bias_pattern()), // Will be restored by BiasedLocking
"should have correct prototype obj: " PTR_FORMAT " mark: " PTR_FORMAT " prototype: " PTR_FORMAT,
p2i(object), object->mark().value(), markWord::prototype_for_klass(object->klass()).value());
}
assert(object->forwardee() == NULL, "should be forwarded to NULL");
}
@@ -54,10 +54,9 @@ inline bool G1FullGCMarker::mark_object(oop obj) {

// Marked by us, preserve if needed.
markWord mark = obj->mark();
if (obj->mark_must_be_preserved(mark) &&
// It is not necessary to preserve marks for objects in regions we do not
// compact because we do not change their headers (i.e. forward them).
_collector->is_compacting(obj)) {
// It is not necessary to preserve marks for objects in regions we do not
// compact because we do not change their headers (i.e. forward them).
if (_collector->is_compacting(obj)) {
preserved_stack()->push(obj, mark);
}

@@ -79,11 +79,6 @@ template <class T> inline void G1AdjustClosure::adjust_pointer(T* p) {
oop forwardee = obj->forwardee();
if (forwardee == NULL) {
// Not forwarded, return current reference.
assert(obj->mark() == markWord::prototype_for_klass(obj->klass()) || // Correct mark
obj->mark_must_be_preserved() || // Will be restored by PreservedMarksSet
shipilev marked this conversation as resolved.
Show resolved Hide resolved
(UseBiasedLocking && obj->has_bias_pattern()), // Will be restored by BiasedLocking
"Must have correct prototype or be preserved, obj: " PTR_FORMAT ", mark: " PTR_FORMAT ", prototype: " PTR_FORMAT,
p2i(obj), obj->mark().value(), markWord::prototype_for_klass(obj->klass()).value());
return;
}

@@ -360,7 +360,7 @@ oop PSPromotionManager::oop_promotion_failed(oop obj, markWord obj_mark) {

push_contents(obj);

_preserved_marks->push_if_necessary(obj, obj_mark);
_preserved_marks->push(obj, obj_mark);
} else {
// We lost, someone else "owns" this object
guarantee(obj->is_forwarded(), "Object must be forwarded if the cas failed.");
@@ -680,7 +680,7 @@ void DefNewGeneration::handle_promotion_failure(oop old) {

_promotion_failed = true;
_promotion_failed_info.register_copy_failure(old->size());
_preserved_marks_set.get()->push_if_necessary(old, old->mark());
_preserved_marks_set.get()->push(old, old->mark());
// forward to self
old->forward_to(old);

@@ -41,9 +41,7 @@ inline void MarkSweep::mark_object(oop obj) {
markWord mark = obj->mark();
obj->set_mark(markWord::prototype().set_marked());

if (obj->mark_must_be_preserved(mark)) {
preserve_mark(obj, mark);
}
preserve_mark(obj, mark);
}

template <class T> inline void MarkSweep::mark_and_push(T* p) {
@@ -52,12 +52,9 @@ class PreservedMarks {

OopAndMarkWordStack _stack;

inline bool should_preserve_mark(oop obj, markWord m) const;

public:
size_t size() const { return _stack.size(); }
inline void push(oop obj, markWord m);
inline void push_if_necessary(oop obj, markWord m);
// Iterate over the stack, restore all preserved marks, and
// reclaim the memory taken up by the stack segments.
void restore();
@@ -30,22 +30,11 @@
#include "oops/oop.inline.hpp"
#include "utilities/stack.inline.hpp"

inline bool PreservedMarks::should_preserve_mark(oop obj, markWord m) const {
return obj->mark_must_be_preserved_for_promotion_failure(m);
}

inline void PreservedMarks::push(oop obj, markWord m) {
assert(should_preserve_mark(obj, m), "pre-condition");
OopAndMarkWord elem(obj, m);
_stack.push(elem);
}

inline void PreservedMarks::push_if_necessary(oop obj, markWord m) {
if (should_preserve_mark(obj, m)) {
push(obj, m);
}
}

inline void PreservedMarks::init_forwarded_mark(oop obj) {
obj->init_mark();
}
@@ -360,7 +360,7 @@ class ShenandoahPrepareForCompactionObjectClosure : public ObjectClosure {
// Object fits into current region, record new location:
assert(_compact_point + obj_size <= _to_region->end(), "must fit");
shenandoah_assert_not_forwarded(NULL, p);
_preserved_marks->push_if_necessary(p, p->mark());
_preserved_marks->push(p, p->mark());
p->forward_to(cast_to_oop(_compact_point));
_compact_point += obj_size;
}
@@ -468,7 +468,7 @@ void ShenandoahFullGC::calculate_target_humongous_objects() {

if (start >= to_begin && start != r->index()) {
// Fits into current window, and the move is non-trivial. Record the move then, and continue scan.
_preserved_marks->get(0)->push_if_necessary(old_obj, old_obj->mark());
_preserved_marks->get(0)->push(old_obj, old_obj->mark());
old_obj->forward_to(cast_to_oop(heap->get_region(start)->bottom()));
to_end = start;
continue;
@@ -236,30 +236,6 @@ class markWord {
// is transient and *should* be short-lived).
static markWord INFLATING() { return zero(); } // inflate-in-progress

// Should this header be preserved during GC?
template <typename KlassProxy>
inline bool must_be_preserved(KlassProxy klass) const;

// Should this header (including its age bits) be preserved in the
// case of a promotion failure during scavenge?
// Note that we special case this situation. We want to avoid
// calling BiasedLocking::preserve_marks()/restore_marks() (which
// decrease the number of mark words that need to be preserved
// during GC) during each scavenge. During scavenges in which there
// is no promotion failure, we actually don't need to call the above
// routines at all, since we don't mutate and re-initialize the
// marks of promoted objects using init_mark(). However, during
// scavenges which result in promotion failure, we do re-initialize
// the mark words of objects, meaning that we should have called
// these mark word preservation routines. Currently there's no good
// place in which to call them in any of the scavengers (although
// guarded by appropriate locks we could make one), but the
// observation is that promotion failures are quite rare and
// reducing the number of mark words preserved during them isn't a
// high priority.
template <typename KlassProxy>
inline bool must_be_preserved_for_promotion_failure(KlassProxy klass) const;

// WARNING: The following routines are used EXCLUSIVELY by
// synchronization functions. They are not really gc safe.
// They must get updated if markWord layout get changed.
@@ -29,45 +29,6 @@
#include "oops/markWord.hpp"
#include "runtime/globals.hpp"

// Should this header be preserved during GC?
template <typename KlassProxy>
inline bool markWord::must_be_preserved(KlassProxy klass) const {
if (UseBiasedLocking) {
if (has_bias_pattern()) {
// Will reset bias at end of collection
// Mark words of biased and currently locked objects are preserved separately
return false;
}
markWord prototype_header = prototype_for_klass(klass);
if (prototype_header.has_bias_pattern()) {
// Individual instance which has its bias revoked; must return
// true for correctness
return true;
}
}
return (!is_unlocked() || !has_no_hash());
}

// Should this header be preserved in the case of a promotion failure during scavenge?
template <typename KlassProxy>
inline bool markWord::must_be_preserved_for_promotion_failure(KlassProxy klass) const {
if (UseBiasedLocking) {
// We don't explicitly save off the mark words of biased and
// currently-locked objects during scavenges, so if during a
// promotion failure we encounter either a biased mark word or a
// klass which still has a biasable prototype header, we have to
// preserve the mark word. This results in oversaving, but promotion
// failures are rare, and this avoids adding more complex logic to
// the scavengers to call new variants of
// BiasedLocking::preserve_marks() / restore_marks() in the middle
// of a scavenge when a promotion failure has first been detected.
if (has_bias_pattern() || prototype_for_klass(klass).has_bias_pattern()) {
return true;
}
}
return (!is_unlocked() || !has_no_hash());
}

inline markWord markWord::prototype_for_klass(const Klass* klass) {
markWord prototype_header = klass->prototype_header();
assert(prototype_header == prototype() || prototype_header.has_bias_pattern(), "corrupt prototype header");
@@ -294,11 +294,6 @@ class oopDesc {
inline markWord displaced_mark() const;
inline void set_displaced_mark(markWord m);

// Checks if the mark word needs to be preserved
inline bool mark_must_be_preserved() const;
inline bool mark_must_be_preserved(markWord m) const;
inline bool mark_must_be_preserved_for_promotion_failure(markWord m) const;

static bool has_klass_gap();

// for code generation
@@ -409,21 +409,4 @@ class DeferredObjectToKlass {
}
};

bool oopDesc::mark_must_be_preserved() const {
return mark_must_be_preserved(mark());
}

bool oopDesc::mark_must_be_preserved(markWord m) const {
// There's a circular dependency between oop.inline.hpp and
// markWord.inline.hpp because markWord::must_be_preserved wants to call
// oopDesc::klass(). This could be solved by calling klass() here. However,
// not all paths inside must_be_preserved calls klass(). Defer the call until
// the klass is actually needed.
return m.must_be_preserved(DeferredObjectToKlass(this));
}

bool oopDesc::mark_must_be_preserved_for_promotion_failure(markWord m) const {
return m.must_be_preserved_for_promotion_failure(DeferredObjectToKlass(this));
}

#endif // SHARE_OOPS_OOP_INLINE_HPP
@@ -1441,10 +1441,8 @@ inline void ObjectMarker::mark(oop o) {
// object's mark word
markWord mark = o->mark();

if (o->mark_must_be_preserved(mark)) {
_saved_mark_stack->push(mark);
_saved_oop_stack->push(o);
}
_saved_mark_stack->push(mark);
_saved_oop_stack->push(o);

// mark the object
o->set_mark(markWord::prototype().set_marked());