diff --git a/include/xo/gc/MutationLogEntry.hpp b/include/xo/gc/MutationLogEntry.hpp index 5141ba3..9702834 100644 --- a/include/xo/gc/MutationLogEntry.hpp +++ b/include/xo/gc/MutationLogEntry.hpp @@ -10,6 +10,7 @@ namespace xo { namespace mm { class GCObjectStore; // see xo-gc/ GCObjectStore.hpp + class MutationLogStatistics; // see xo-gc/ MutationLogStatistics.hpp /** @brief Track a cross-generational pointer * @@ -38,9 +39,25 @@ namespace xo { /** true iff child pointer matches value when this mlog entry created **/ bool is_active() const noexcept { return *p_data_ == snap_.data(); } - /** true iff child pointer has been altered since this mlog entry created **/ + /** true iff child pointer has been altered since this mlog entry created + * WARNING: extra care needed around forwarding pointers + **/ bool is_superseded() const noexcept { return *p_data_ != snap_.data(); } + /** Update parent and snapshot if either has been forwarded. + * Do not try to correct *p_data_: it might now point outside + * gc-owned space, in which case need not be intelligible + **/ + bool check_forward_inplace(GCObjectStore * gcos, + MutationLogStatistics * p_counters) noexcept; + + /** update @ref parent_, @ref p_data_ iff parent has been forwarded + * @return true iff encountered forwarded parent + * Require: parent must be gc-owned, since we rely on AllocInfo existing + **/ + bool check_forward_parent_inplace(GCObjectStore * gcos, + MutationLogStatistics * p_counters) noexcept; + /** Refresh snapshot when *p_data_ does not match snap_.data_ * Get updated facet information from destination alloc header. * It's possible that *p_data_ no longer points to gc-owned space diff --git a/include/xo/gc/MutationLogStore.hpp b/include/xo/gc/MutationLogStore.hpp index b1b2787..018b286 100644 --- a/include/xo/gc/MutationLogStore.hpp +++ b/include/xo/gc/MutationLogStore.hpp @@ -135,6 +135,8 @@ namespace xo { * with parent P in generation @p parent_gen: * ensure child C is evacuated, and append @p from_entry to * @p keep_mlog. + * + * Require: child is gc-owned **/ MutationLogStatistics _preserve_child_of_live_parent(obj gc, Generation upto, diff --git a/src/gc/MutationLogEntry.cpp b/src/gc/MutationLogEntry.cpp index 56c34ad..39bf802 100644 --- a/src/gc/MutationLogEntry.cpp +++ b/src/gc/MutationLogEntry.cpp @@ -4,9 +4,11 @@ **/ #include "MutationLogEntry.hpp" -#include +#include "GCObjectStore.hpp" +#include "MutationLogStatistics.hpp" namespace xo { + using xo::mm::MutationLogStatistics; using xo::reflect::typeseq; namespace mm { @@ -19,39 +21,110 @@ namespace xo { snap_{snap} {} + bool + MutationLogEntry::check_forward_inplace(GCObjectStore * gcos, + MutationLogStatistics * p_counters) noexcept + { + /** Several cases here based on state of {mlog entry, parent}. + * + * 1. gc already updated P->P', C->C', + * still have snap->P, snap->C + * update P'->C', snap->P', snap->C' + * 2. gc already updated P->C to P->C'. + * still have snap->C. mlog entry is current. + * update snap->C to snap->C' + * 3. gc has not updated P->C, + * but it has (independently) evacuated C to C'. + * still have snap->C, P->C. mlog entry is current. + * update P->C' and snap->C' + * 4. gc has not moved P or C. mlog entry is up to date. + * no update. P,C may yet be rescued, or may be garbage. + * 5. P->D, D not in {C,C'} + * mlog entry has been superseded. + * Be careful of confusing this case with 1..4 + **/ + + // if either of {parent, snapshot} has been forwarded, + // update with destination. + // + // Don't do this with child, since child might now point + // outside gc-owned space + + bool upd_flag = this->check_forward_parent_inplace(gcos, p_counters); + + AllocInfo snap_info = gcos->alloc_info((std::byte *)snap_.data_); + + if (snap_info.is_forwarding_tseq()) { + void * snap_from = snap_.data_; + void * snap_to = *(void **)snap_.data_; + + if (snap_from == *p_data_) { + // parent still refers to forwarding pointer, needs fix + *p_data_ = snap_to; + // also fix snapshot + this->snap_.reset_opaque(snap_to); + + upd_flag = true; + } else if (snap_to == *p_data_) { + // parent updated, but snapshot stale + this->snap_.reset_opaque(snap_to); + + upd_flag = true; + } else { + // superseded mlog entry + } + } + + return upd_flag; + } + + bool + MutationLogEntry::check_forward_parent_inplace(GCObjectStore * gcos, + MutationLogStatistics * p_counters) noexcept + { + AllocInfo parent_info = gcos->alloc_info((std::byte *)parent_); + + if (parent_info.is_forwarding_tseq()) { + void * parent_to = *(void **)parent_; + + std::size_t offset + = (std::byte *)p_data_ - (std::byte *)parent_; + + void ** p_data_to = (void **)((std::byte *)parent_to + offset); + + this->parent_ = parent_to; + this->p_data_ = p_data_to; + + ++(p_counters->n_live_parent_); + + return true; + } else { + return false; + } + } + bool MutationLogEntry::refresh_snapshot(Generation parent_gen, GCObjectStore * gcos) noexcept { + scope log(XO_DEBUG(gcos->config().debug_flag_)); + void * child_data = *p_data_; - // note: never the same child_info as computed at the top of - // MutationLogEntry._preserve_child_of_live_parent() - // Child pointer was either forwarding pointer or moved. - // In either case must pickup info for new location. - // - AllocInfo child_info = gcos->alloc_info((std::byte*)child_data); - - if (child_info.is_forwarding_tseq()) { - // code salvaged from MutationLogStore._check_keep_mutation_aux() - // as reminder. But if caller is gc will have to know this anyway, - // so it can move child - - // if (info.is_forwarding_tseq()) { - // child_data = *(void **)child_data; - // info = gcos->alloc_info((std::byte *)child_data); - //} - - assert(false); // for now assuming caller forward child - } Generation child_gen_to = gcos->generation_of(Role::to_space(), child_data); if (child_gen_to.is_sentinel()) { - // child no longer points to gc-owned space. - // 1. may not have an alloc header (could be a static global for example), - // so AllocInfo not available - // 2. doesn't need a mutation log entry since this gc can't move destination + log && log("child not in to-space"); + + // unreachable, since: + // 1. not in from-space, if it were forwarded, + // caller would already have forwarded this mlog entry. + // 2. caller already confirmed mlog entry current. + // 3. would not create mlog entry for non-gc-owned child + // in any case, would be legal to discard mlog entry here. + + assert(false); snap_.data_ = nullptr; // hygiene @@ -66,6 +139,12 @@ namespace xo { > config.promotion_threshold(child_gen_to))); if (need_mlog_entry) { + AllocInfo child_info = gcos->alloc_info((std::byte*)child_data); + + assert(!child_info.is_forwarding_tseq()); + + log && log("need mlog entry", xtag("tseq", child_info.tseq())); + AGCObject * iface = gcos->lookup_type(typeseq(child_info.tseq())); if (iface) { @@ -74,11 +153,17 @@ namespace xo { // snapshot updated, keep mlog entry return true; } else { + log && log("facet install error!"); + + // facet install error + assert(false); return false; } } else { + log && log("retire mlog entry"); + // retire this entry. return false; } diff --git a/src/gc/MutationLogStore.cpp b/src/gc/MutationLogStore.cpp index de5d0e6..e57f40d 100644 --- a/src/gc/MutationLogStore.cpp +++ b/src/gc/MutationLogStore.cpp @@ -258,12 +258,21 @@ namespace xo { MutationLogStore::forward_mutation_log(obj gc, Generation upto) { + scope log0(XO_DEBUG(config_.debug_flag_)); + /** non-zero if at least one object was rescued (from any generation) * by mutation log scan **/ std::size_t work = 0; + /** count outer loop iterations */ + std::size_t i_fixpoint_loop = 0; + do { + scope log1(XO_DEBUG(log0), "fixpoint", xtag("i", i_fixpoint_loop)); + + work = 0; + // on 1st iteration, for all generations: // - to_mlog, triage_mlog are empty @@ -271,7 +280,10 @@ namespace xo { child_gen + 1 < config_.n_generation_; ++child_gen) { - MutationLog * from_mlog = this->mlog_[Role::from_space()][child_gen]; + scope log2(XO_DEBUG(log1), xtag("gen", child_gen)); + + MutationLog * from_mlog + = this->mlog_[Role::from_space()][child_gen]; if (!from_mlog->empty()) { MutationLog * to_mlog = this->mlog_[Role::to_space()][child_gen]; @@ -294,10 +306,14 @@ namespace xo { work += stats.n_rescue_; } } + + ++i_fixpoint_loop; } while (work > 0); // here: reached fixpoints, any remaining triaged mlogs can be discarded for (Generation child_gen{0}; child_gen + 2 < config_.n_generation_; ++child_gen) { + log0 && log0("dismiss unelected triage", xtag("gen", child_gen)); + MutationLog * triage_mlog = this->mlog_[c_n_role][child_gen]; triage_mlog->clear(); @@ -372,89 +388,85 @@ namespace xo { for (MutationLogEntry & from_entry : *from_mlog) { if (log) { - log(xtag("i_from", i_from)); + log(xtag("i_from", i_from), + xtag("parent", from_entry.parent()), + xtag("snap", from_entry.snap().data())); } - if (from_entry.is_superseded()) { - // there must be a second mlog entry that refers to - // the new child. Rely on that second entry, - // skipping this one. - - // [MLOG0] obsolete mutation -> skip - ++counters.n_stale_; - continue; - } - - /* here: mlog current */ + // fixup gc-owned forwarded pointers in from_entry. + // May update from_entry.p_data_, but not *(from_entry.p_data_), since: + // 1. *p_data_ may not be gc-owned + // 2. want to preserve ability to superseded mlog entry + // + // load-bearing at least for [MLOG3] + // + from_entry.check_forward_inplace(gco_store_, &counters); + // Two possibiities for parent in to-space: + // 1. belongs to generation not subject to collection this cycle. + // 2. from_entry updated above for new location + // Generation parent_gen_to = gc.generation_of(Role::to_space(), from_entry.parent()); if (parent_gen_to.is_sentinel()) { - // parent is not in to-space + // parent is not in to-space. + // Only gc-owned parents eligible for mlog entries. + // Therefore parent must be in from-space + // Since not rescued, it may be garbage. - void * parent_fr = from_entry.parent(); - AllocInfo parent_info = gc.alloc_info((std::byte *)parent_fr); + log && log("parent not in to-space -> must be in from-space"); - if (parent_info.is_forwarding_tseq()) { - /* [MLOG3] */ + Generation parent_gen_from = gc.generation_of(Role::from_space(), + from_entry.parent()); + assert(!parent_gen_from.is_sentinel()); - ++counters.n_live_parent_; - - // new parent location in to-space - // TODO: method on AllocInfo to streamline this - void * parent_to = *(void **)parent_fr; - - parent_gen_to = gc.generation_of(Role::to_space(), - parent_to); - parent_info = gc.alloc_info((std::byte *)parent_to); - - assert(!parent_gen_to.is_sentinel()); - - // Since parent already forwarded, we don't have to preserve child - // or update parent object. GC already guaranteed to have visited - // parent's child pointers - // - // Do need to replace mlog entry to reflect new parent location. - - std::size_t offset - = ((std::byte *)from_entry.p_data() - - (std::byte *)from_entry.parent()); - - void ** p_data_to = (void **)((std::byte *)(parent_to) + offset); - //void * child_to = *p_data_to; - - MutationLogEntry to_entry(parent_to, p_data_to, from_entry.snap()); - - if (to_entry.refresh_snapshot(parent_gen_to, gco_store_)) - keep_mlog->push_back(to_entry); - -#ifdef OBSOLETE - this->_check_keep_mutation_aux(to_entry, - parent_gen_to, - child_to, - keep_mlog); -#endif + if (from_entry.is_superseded()) { + log && log("entry superseded -> discard"); + // parent mutated again after from_entry. + // If new child needs rescue, that will rely on mlog + // entry for that second mutation + ++counters.n_stale_; } else { - ++counters.n_triage_; + log && log("entry current -> triage"); - // parent hasn't been collected and may be garbage. - // However this is only provisional, since - // parent could turn out to be reachable via some other mutation. + // although parent appears to be garbage, + // it may get rescued via some other mlog entry. + // Keep mlog entry while considering other mutations. + + ++counters.n_triage_; triage_mlog->push_back(from_entry); } } else { - /* [MLOG1, MLOG2] */ + // parent in to-space: p_data_ is valid, can check superseded - counters - += this->_preserve_child_of_live_parent(gc, - upto, - parent_gen_to, - from_entry, - keep_mlog); + log && log("parent in to-space"); + + if (from_entry.is_superseded()) { + log && log("entry superseded -> discard"); + // there must be a second mlog entry that refers to + // the new child. Rely on that second entry, + // skipping this one. + + // [MLOG0] obsolete mutation -> skip + ++counters.n_stale_; + } else { + log && log("entry current -> preserve child"); + + /* [MLOG1, MLOG2] */ + + log && log(xtag("case", "MLOG1|MLOG2")); + + counters + += this->_preserve_child_of_live_parent(gc, + upto, + parent_gen_to, + from_entry, + keep_mlog); + } } } @@ -468,6 +480,8 @@ namespace xo { MutationLogEntry & from_entry, MutationLog * keep_mlog) { + scope log(XO_DEBUG(config_.debug_flag_)); + void * child_fr = *from_entry.p_data(); AllocInfo child_info = gc.alloc_info((std::byte *)(child_fr)); @@ -489,6 +503,8 @@ namespace xo { if (child_info.is_forwarding_tseq()) { // [MLOG1] + log && log(xtag("case", "MLOG1"), xtag("msg", "child forwarded")); + // child already forwarded. // TODO: make this a method on AllocInfo child_to = *(void **)child_fr; @@ -496,6 +512,8 @@ namespace xo { } else { // [MLOG2] + log && log(xtag("case", "MLOG2"), xtag("msg", "rescue child")); + ++counters.n_rescue_; child_to = gco_store_->deep_move_interior(gc, child_fr, upto); @@ -503,7 +521,7 @@ namespace xo { // update child pointer in parent object. // either forwarded or moved - *from_entry.p_data() = child_to; + *(from_entry.p_data()) = child_to; // TODO: pass statistics object if (from_entry.refresh_snapshot(parent_gen, gco_store_)) { @@ -515,41 +533,6 @@ namespace xo { return counters; } -#ifdef OBSOLETE - void - MutationLogStore::_check_keep_mutation_aux(MutationLogEntry & from_entry, - Generation parent_gen_to, - void * child_to, - MutationLog * keep_mlog) - { - Generation child_gen_to - = gco_store_->generation_of(Role::to_space(), child_to); - - bool need_mlog_entry - = ((child_gen_to + 1 < config_.n_generation_) - && (gco_store_->config().promotion_threshold(parent_gen_to) - > gco_store_->config().promotion_threshold(child_gen_to))); - - if (need_mlog_entry) { - // 1. P->C pointer is still cross-age (xage), and - // 2. this matters; in future P will promote before C - // - // Need to keep entry because parent will be eligible for promotion - // before child - - keep_mlog->push_back(from_entry); - - return true; - } else { - // child now in final generation, - // no longer need to track incoming mutations. - - return false; - } - } -#endif - - } /*namespace mm*/ } /*namespace xo*/ diff --git a/utest/MutationLogStore.test.cpp b/utest/MutationLogStore.test.cpp index 6bf6fb9..c9f5a3d 100644 --- a/utest/MutationLogStore.test.cpp +++ b/utest/MutationLogStore.test.cpp @@ -285,7 +285,7 @@ namespace ut { Testcase(2, 4, 16 * 1024, 8 * 128, T, seq_1, 0, F, c_fixed, 1, 0, 0, 0, 0, F), Testcase(2, 1, 16 * 1024, 8 * 128, T, seq_2, 128, T, c_fixed, 3, 0, 0, 0, 0, F), Testcase(2, 2, 16 * 1024, 8 * 128, T, seq_3, 128, T, c_fixed, 4, 0, 0, 0, 0, F), - Testcase(2, 2, 16 * 1024, 8 * 128, T, seq_4, 128, T, c_fixed, 4, 0, 0, 0, 0, T), + Testcase(2, 2, 16 * 1024, 8 * 128, T, seq_4, 128, T, c_fixed, 4, 0, 0, 0, 0, F), }; # undef T