xo-gc: bugfixes. xo-gc utests pass.

This commit is contained in:
Roland Conybeare 2026-04-25 15:50:08 -04:00
commit 866ab0503d
5 changed files with 209 additions and 122 deletions

View file

@ -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

View file

@ -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<AGCObjectVisitor> gc,
Generation upto,

View file

@ -4,9 +4,11 @@
**/
#include "MutationLogEntry.hpp"
#include <xo/gc/GCObjectStore.hpp>
#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;
}

View file

@ -258,12 +258,21 @@ namespace xo {
MutationLogStore::forward_mutation_log(obj<AGCObjectVisitor> 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,83 +388,78 @@ 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()));
}
// 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.
// Only gc-owned parents eligible for mlog entries.
// Therefore parent must be in from-space
// Since not rescued, it may be garbage.
log && log("parent not in to-space -> must be in from-space");
Generation parent_gen_from = gc.generation_of(Role::from_space(),
from_entry.parent());
assert(!parent_gen_from.is_sentinel());
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 {
log && log("entry current -> triage");
// 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 {
// parent in to-space: p_data_ is valid, can check superseded
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_;
continue;
}
/* here: mlog current */
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
void * parent_fr = from_entry.parent();
AllocInfo parent_info = gc.alloc_info((std::byte *)parent_fr);
if (parent_info.is_forwarding_tseq()) {
/* [MLOG3] */
++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
} else {
++counters.n_triage_;
log && log("entry current -> preserve child");
// 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.
triage_mlog->push_back(from_entry);
}
} else {
/* [MLOG1, MLOG2] */
log && log(xtag("case", "MLOG1|MLOG2"));
counters
+= this->_preserve_child_of_live_parent(gc,
upto,
@ -457,6 +468,7 @@ namespace xo {
keep_mlog);
}
}
}
return counters;
} /*forward_mutation_log_phase*/
@ -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*/

View file

@ -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