From 3d06980b42f2b0feb077af533b2b87d4d471c2ef Mon Sep 17 00:00:00 2001 From: Roland Conybeare Date: Fri, 5 Dec 2025 18:38:29 -0500 Subject: [PATCH] xo-ordinaltree: expand unittest + debug logging --- xo-alloc/include/xo/alloc/GC.hpp | 4 +- xo-alloc/src/alloc/GC.cpp | 79 ++++++++++++++--- xo-alloc/src/alloc/Object.cpp | 14 ++- xo-alloc/utest/GC.test.cpp | 23 +++++ xo-allocutil/include/xo/allocutil/IAlloc.hpp | 32 +++++-- xo-allocutil/include/xo/allocutil/IObject.hpp | 4 +- .../xo/allocutil/gc_allocator_traits.hpp | 86 ++++++++++++++----- .../include/xo/ordinaltree/RedBlackTree.hpp | 12 ++- .../xo/ordinaltree/rbtree/RbTreeUtil.hpp | 69 +++++++++++++-- xo-ordinaltree/utest/RedBlackTree-gc.test.cpp | 42 +++++---- 10 files changed, 296 insertions(+), 69 deletions(-) diff --git a/xo-alloc/include/xo/alloc/GC.hpp b/xo-alloc/include/xo/alloc/GC.hpp index e5dd6781..d8acc08c 100644 --- a/xo-alloc/include/xo/alloc/GC.hpp +++ b/xo-alloc/include/xo/alloc/GC.hpp @@ -107,7 +107,7 @@ namespace xo { bool is_dead() const { return false; } MutationLogEntry update_parent_moved(IObject * parent_to) const; - void fixup_parent_child_moved(IObject * child_to) { *lhs_ = child_to; } + void fixup_parent_child_moved(IObject * child_to); private: IObject * parent_ = nullptr; @@ -341,7 +341,7 @@ namespace xo { * is recorded in mutation log, * given an object @p parent that contains object pointer @p lhs **/ - virtual bool check_write_barrier(IObject * parent, IObject ** lhs, bool may_throw) const final; + virtual bool check_write_barrier(const void * parent, const void * const * lhs, bool may_throw) const final; virtual std::byte * alloc(std::size_t z) final override; virtual std::byte * alloc_gc_copy(std::size_t z, const void * src) final override; diff --git a/xo-alloc/src/alloc/GC.cpp b/xo-alloc/src/alloc/GC.cpp index 5cfdd9fc..dd17fd3b 100644 --- a/xo-alloc/src/alloc/GC.cpp +++ b/xo-alloc/src/alloc/GC.cpp @@ -58,6 +58,12 @@ namespace xo { reinterpret_cast(lhs_to)); } + void + MutationLogEntry::fixup_parent_child_moved(IObject * child_to) + { + *(this->lhs_) = child_to; + } + GC::GC(const Config & config) : config_{config} { @@ -508,12 +514,17 @@ namespace xo { assert(retval); + log && log(xtag("retval", retval)); + return retval; } void GC::assign_member(IObject * parent, IObject ** lhs, IObject * rhs) { + scope log(XO_DEBUG(config_.debug_flag_), + xtag("parent", parent), xtag("lhs", lhs), xtag("rhs", rhs)); + ++gc_statistics_.n_mutation_; *lhs = rhs; @@ -532,23 +543,28 @@ namespace xo { { case generation_result::tenured: /* only need to log mutations that create tenured->nursery pointers */ + log && log(xtag("act", "any->T no log")); return; case generation_result::nursery: switch (tospace_generation_of(parent)) { case generation_result::nursery: if (is_before_checkpoint(parent)) { + log && log(xtag("act", "N1->N0 must mlog")); + // N1->N0, so must log this->mutation_log_[role2int(role::to_space)]->push_back(MutationLogEntry(parent, lhs)); ++(this->gc_statistics_.n_logged_mutation_); ++(this->gc_statistics_.n_xckp_mutation_); } else { // parent in N0, not an xckp mutation - return; + log && log(xtag("act", "N0->any no long")); } break; case generation_result::tenured: // T->N, so must log + log && log(xtag("act", "T->N must mlog")); + this->mutation_log_[role2int(role::to_space)]->push_back(MutationLogEntry(parent, lhs)); ++(this->gc_statistics_.n_logged_mutation_); ++(this->gc_statistics_.n_xgen_mutation_); @@ -556,11 +572,13 @@ namespace xo { case generation_result::not_found: // parent is global // This may be ok (provided lhs is a gc root) + log && log(xtag("act", "root->any no log")); break; } break; case generation_result::not_found: + log && log(xtag("act", "any->root no log")); // child is global; // logging not required @@ -571,6 +589,8 @@ namespace xo { void GC::forward_inplace(IObject ** lhs) { + scope log(XO_DEBUG(config_.debug_flag_), xtag("lhs", lhs)); + Object::_forward_inplace(lhs, this); } @@ -588,10 +608,13 @@ namespace xo { } bool - GC::check_write_barrier(IObject * parent, - IObject ** lhs, + GC::check_write_barrier(const void * parent, + const void * const * lhs, bool may_throw_flag) const { + scope log(XO_DEBUG(config_.debug_flag_), + xtag("P", parent), xtag("L", lhs)); + if (!this->contains(parent)) { if (may_throw_flag) { throw std::runtime_error(tostr("GC::check_write_barrier", @@ -601,10 +624,11 @@ namespace xo { return false; } +#ifdef NOPE // don't want to assume IObject* std::size_t parent_z = parent->_shallow_size(); - std::byte * parent_addr = reinterpret_cast(parent); - std::byte * lhs_addr = reinterpret_cast(lhs); + const std::byte * parent_addr = reinterpret_cast(parent); + const std::byte * lhs_addr = reinterpret_cast(lhs); if ((lhs_addr < parent_addr) || (parent_addr + parent_z < lhs_addr)) { if (may_throw_flag) { @@ -617,12 +641,21 @@ namespace xo { } return false; } +#endif - IObject * rhs = *lhs; + const void * rhs = *lhs; + + if (!rhs) + return true; auto parent_gen = tospace_generation_of(parent); auto rhs_gen = tospace_generation_of(rhs); + log && log(xtag("C", rhs), + xtag("gen(P)", parent_gen), xtag("gen(C)", rhs_gen), + xtag("P.before-ckp", is_before_checkpoint(parent)), + xtag("C.before-ckp", is_before_checkpoint(rhs))); + switch(parent_gen) { case generation_result::nursery: if (is_before_checkpoint(parent)) { @@ -630,21 +663,25 @@ namespace xo { case generation_result::nursery: if (is_before_checkpoint(rhs)) { /* no mlog entry needed */ + log && log(xtag("msg", "N1->N1 - trivial")); return true; } else { /* need to check mlog */ - ; + log && log(xtag("msg", "N1->N0 - xgen")); } break; case generation_result::tenured: /* no mlog entry needed */ + log && log(xtag("msg", "N1->T - trivial")); return true; case generation_result::not_found: /* possible non-gc rhs */ + log && log(xtag("msg", "non-gc rhs - trivial")); return true; } } else { /* no mlog entry needed */ + log && log(xtag("msg", "N0->any - trivial")); return true; } break; @@ -652,16 +689,21 @@ namespace xo { switch(rhs_gen) { case generation_result::nursery: /* need to check mlog */ + log && log(xtag("msg", "T->N - xgen")); break; case generation_result::tenured: /* no mlog entry needed */ + log && log(xtag("msg", "T->T - trivial")); return true; case generation_result::not_found: /* possible non-gc rhs */ + log && log(xtag("msg", "non-gc rhs - trivial")); return true; } + break; case generation_result::not_found: /* already excluded -> impossible */ + log && log(xtag("msg", "assert")); assert(false && "already verified parent owned by GC"); } @@ -669,7 +711,7 @@ namespace xo { * search mutation log + verify such entry exists */ for (MutationLogEntry & mlog : *(mutation_log_[role2int(role::to_space)])) { - if ((mlog.parent() == parent) && (mlog.lhs() == lhs)) { + if ((mlog.parent() == parent) && ((const void * const *)mlog.lhs() == lhs)) { return true; } mlog.lhs(); @@ -872,7 +914,7 @@ namespace xo { for (MutationLogEntry & from_entry : *from_mlog) { if (log) { - if (i_from % 10000 == 0) + if (i_from % 10000 == 0 || true) log(xtag("i_from", i_from)); } @@ -905,8 +947,12 @@ namespace xo { ++n_rescue; + log && log(xtag("parent", parent), xtag("act", "move child"), xtag("child.from", child_from)); + Object::_deep_move(child_from, this, per_type_stats); + log && log(xtag("child.to", child_from->_destination())); + // C forwards to C', fall thru to parent fixup below // (a) T->N1' // (c) T->T @@ -924,13 +970,24 @@ namespace xo { IObject * child_to = child_from->_destination(); + log && log(xtag("act", "fixup parent"), xtag("parent", parent), xtag("lhs", from_entry.lhs()), xtag("child.from", child_from), xtag("child.to", child_to)); + from_entry.fixup_parent_child_moved(child_to); + { + // verify fixup was effective + IObject * child_from2 = from_entry.child(); + assert(child_from2 == child_to); + } + + // P->C', loc(C') in {N1', T'} if (tospace_generation_of(child_to) == generation_result::nursery) { // (b) loc(P)=T, loc(C')=N1'; also case (a) + log && log(xtag("act", "still xgen -> keep mlog entry")); + // still have xgen pointer, so need mlog for it to_mlog->push_back(from_entry); } else { @@ -972,6 +1029,8 @@ namespace xo { } } else { + log && log("defer"); + // loc(P) = N1, loc(C) = N0, P may be garbage // Includes cases: // (e) P->C, C not moved @@ -1347,7 +1406,7 @@ namespace xo { void GC::execute_gc(generation upto) { - scope log(XO_DEBUG(config_.stats_flag_)); + scope log(XO_DEBUG(config_.stats_flag_ || config_.debug_flag_)); auto t0 = std::chrono::steady_clock::now(); diff --git a/xo-alloc/src/alloc/Object.cpp b/xo-alloc/src/alloc/Object.cpp index b0be501b..b0a52f59 100644 --- a/xo-alloc/src/alloc/Object.cpp +++ b/xo-alloc/src/alloc/Object.cpp @@ -42,18 +42,24 @@ namespace xo { Object::_forward(IObject * src, gc::IAlloc * gc) { + scope log(XO_DEBUG(gc->debug_flag()), xtag("src", src)); + if (!src) return src; - if (src->_is_forwarded()) + if (src->_is_forwarded()) { + log && log("already forwarded", xtag("dest", src->_offset_destination(src))); return src->_offset_destination(src); + } if (gc->check_move(src)) { + log && log("needs forwarding"); Object::_shallow_move(src, gc); /* *src is now a forwarding pointer to a copy in to-space */ return src->_offset_destination(src); } else { + log && log("already tenured + incr collection"); /* don't move tenured objects during incremental collection */ return src; } @@ -62,6 +68,8 @@ namespace xo { IObject * Object::_deep_move(IObject * from_src, gc::GC * gc, gc::ObjectStatistics * /*stats*/) { + scope log(XO_DEBUG(gc->config().debug_flag_)); + using gc::generation; if (!from_src) @@ -146,13 +154,15 @@ namespace xo { do { fixup_work = 0; - auto fixup_generation = [gc, &gray_lo_v](generation gen) { + auto fixup_generation = [gc, &log, &gray_lo_v](generation gen) { std::size_t work = 0; while(gray_lo_v[gen2int(gen)] < gc->free_ptr(gen)) { Object * x = reinterpret_cast(gray_lo_v[gen2int(gen)]); // update per-class stats here + log && log("fwd children", xtag("x", x)); + std::size_t xz = x->_forward_children(gc); // must pad xz to multiple of word size, diff --git a/xo-alloc/utest/GC.test.cpp b/xo-alloc/utest/GC.test.cpp index 90e22dfa..f178f4a8 100644 --- a/xo-alloc/utest/GC.test.cpp +++ b/xo-alloc/utest/GC.test.cpp @@ -202,6 +202,29 @@ namespace xo { vector_type member2_; bool ctor_ran_ = false; }; + +#ifdef NOT_YET + struct MemberType2 { + public: + MemberType2() = default; + /** GC hooks rely on copy constructor. But can't write it without allocator state. + * Therefore: need copy-like constructor that takes allocator argument + **/ + + template + explicit MemberType2(Allocator & alloc, uint64 payload) { + using traits = gc_allocator_traits; + + uint64_t * ptr = traits::allocate(alloc, 1); + + this->payload_ = payload; + this->ctor_ran_ = true; + } + + uint64_t * payload_ = nullptr; + bool ctor_ran_ = false; + } +#endif } TEST_CASE("vector_custom_allocator", "[alloc][vector]") diff --git a/xo-allocutil/include/xo/allocutil/IAlloc.hpp b/xo-allocutil/include/xo/allocutil/IAlloc.hpp index 2c78b131..b38272a4 100644 --- a/xo-allocutil/include/xo/allocutil/IAlloc.hpp +++ b/xo-allocutil/include/xo/allocutil/IAlloc.hpp @@ -143,8 +143,8 @@ namespace xo { /** check write barrier (if impl has write barrier) * given an object @p parent that contains object pointer @p lhs. **/ - virtual bool check_write_barrier(IObject * /*parent*/, - IObject ** /*lhs*/, + virtual bool check_write_barrier(const void * /*parent*/, + const void * const * /*lhs*/, bool /*may_throw*/) const { return true; }; /** write barrier for collector. perform assignment * @code @@ -169,6 +169,16 @@ namespace xo { } }; + /** for gc_allocator_traits, want an allocator pointer we can inherit from **/ + struct IAllocPtr { + public: + inline bool check_write_barrier(const void * parent, const void * const * lhs, bool may_throw) const { + return mm_->check_write_barrier(parent, lhs, may_throw); + } + + IAlloc * mm_ = nullptr; + }; + /** allocator wrapper (in the style of std::allocator) **/ template @@ -183,10 +193,11 @@ namespace xo { using difference_type = std::ptrdiff_t; using gc_object_interface = IAlloc::gc_object_interface; + using gc_interface = IAllocPtr; using has_incremental_gc_interface = IAlloc::has_incremental_gc_interface; /** rebind is for typed allocators. since IAlloc is untyped, - * we want degenerate version + * rebind is almost trivial **/ template struct rebind { @@ -194,16 +205,16 @@ namespace xo { }; public: - explicit allocator(IAlloc * mm) : mm_{mm} {} + explicit allocator(IAlloc * mm) : impl_{mm} {} allocator(const allocator &) = default; allocator & operator=(const allocator &) = default; template - allocator(const allocator & other) : mm_{other.mm_} {} + allocator(const allocator & other) : impl_{other.impl_.mm_} {} pointer allocate(size_type n) { - std::byte * raw = mm_->allocate(n * sizeof(T)); + std::byte * raw = impl_.mm_->allocate(n * sizeof(T)); return reinterpret_cast(raw); } @@ -211,7 +222,7 @@ namespace xo { void deallocate(pointer p, size_type n) { std::byte * raw = reinterpret_cast(p); - mm_->deallocate(raw, n * sizeof(T)); + impl_.mm_->deallocate(raw, n * sizeof(T)); } // optional construct, destroy (but allocator_traits provides defaults) @@ -221,11 +232,14 @@ namespace xo { **/ template bool operator==(const allocator & other) const noexcept { - return mm_ == other.mm_; + return impl_.mm_ == other.impl_.mm_; } + /** gc_interface=IAlloc **/ + operator gc_interface () const { return impl_; } + public: - IAlloc * mm_ = nullptr; + IAllocPtr impl_; }; } /*namespace gc*/ diff --git a/xo-allocutil/include/xo/allocutil/IObject.hpp b/xo-allocutil/include/xo/allocutil/IObject.hpp index 086d4135..6cad9295 100644 --- a/xo-allocutil/include/xo/allocutil/IObject.hpp +++ b/xo-allocutil/include/xo/allocutil/IObject.hpp @@ -29,6 +29,8 @@ namespace xo { /** GC write barrier: * assign value @p rhs to member @p *lhs of @p parent. * Identifiy and remember cross-generational pointers. + * + * Expect Allocator is xo::gc::allocator (see IAlloc.hpp) **/ template void _gc_assign_member(T ** lhs, @@ -37,7 +39,7 @@ namespace xo { { static_assert(std::is_convertible_v); - alloc.mm_->assign_member(this, reinterpret_cast(lhs), rhs); + alloc.impl_.mm_->assign_member(this, reinterpret_cast(lhs), rhs); } /** true iff this object represents a forwarding pointer. diff --git a/xo-allocutil/include/xo/allocutil/gc_allocator_traits.hpp b/xo-allocutil/include/xo/allocutil/gc_allocator_traits.hpp index 88ab33ca..20568689 100644 --- a/xo-allocutil/include/xo/allocutil/gc_allocator_traits.hpp +++ b/xo-allocutil/include/xo/allocutil/gc_allocator_traits.hpp @@ -40,6 +40,19 @@ namespace xo { virtual std::size_t _forward_children(gc::IAlloc *) { assert(false); return 0; } }; + /** dummy GC interface. + * non-empty intersection with IAlloc + **/ + template + struct FallbackGcInterface { + template + FallbackGcInterface(Allocator & alloc) {} + + bool check_write_barrier(const void * parent, + const void * const * lhs, + bool may_throw) { return true; }; + }; + /** Extended version of * std::allocator_traits * Introduces additional i/face methods @@ -136,6 +149,8 @@ namespace xo { using super::allocate; using super::deallocate; + // ---------------------------------------------------------------- + // default: allocator A fallback to standard non-gc allocator behavior template struct has_incremental_gc_interface : std::false_type {}; @@ -148,6 +163,19 @@ namespace xo { struct has_incremental_gc_interface> : A::has_incremental_gc_interface {}; + /** true iff this allocator advertises itself as an incremental collector. + * Allocator will include: + * + * struct IAlloc { + * using has_incremental_gc_interface = std::true_type; + * }; + **/ + static inline constexpr + bool + has_incremental_gc_interface_v = has_incremental_gc_interface::value; + + // ---------------------------------------------------------------- + // default: allocate A fallback to standard non-GC allocator behavior template struct has_trivial_deallocate : std::false_type {}; @@ -160,6 +188,19 @@ namespace xo { struct has_trivial_deallocate> : A::has_trivial_deallocate {}; + /** true iff this allocator advertises trivial deallocate + * Allocate will include: + * + * struct IAlloc { + * using has_trivial_deallocate = std::true_type; + * }; + **/ + static inline constexpr + bool + has_trivial_deallocate_v = has_trivial_deallocate::value; + + // ---------------------------------------------------------------- + // default: empty object interface. // // classes that want to conditionally support GC @@ -187,27 +228,32 @@ namespace xo { // using object_interface_type = object_interface; - /** true iff this allocator advertises itself as an incremental collector. - * Allocator will include: - * - * struct IAlloc { - * using has_incremental_gc_interface = std::true_type; - * }; - **/ - static inline constexpr - bool - has_incremental_gc_interface_v = has_incremental_gc_interface::value; + // ---------------------------------------------------------------- + + // default: minimal garbage collector interface. + // + // Use in allocator-aware components that need conditionally + // to engage with GC functionality. + // For example in RedBlackTree::verify_ok() want to check + // cross-generational pointers. + // + // gc_interface gc + // - gc_interface(A & alloc) + // - gc.check_write_barrier(const object_interface_type * p, + // const object_interface_type * const * lhs, + // bool may_throw) + // + template + struct gc_interface : public FallbackGcInterface {}; + + // allocator opt-in by providing a gc_interface type + template + struct gc_interface> : public A::gc_interface {}; + + // interface for (narrow) GC interaction. + // Construct from allocator + using gc_interface_type = gc_interface; - /** true iff this allocator advertises trivial deallocate - * Allocate will include: - * - * struct IAlloc { - * using has_trivial_deallocate = std::true_type; - * }; - **/ - static inline constexpr - bool - has_trivial_deallocate_v = has_trivial_deallocate::value; }; } /*namespace gc*/ } /*namespace xo*/ diff --git a/xo-ordinaltree/include/xo/ordinaltree/RedBlackTree.hpp b/xo-ordinaltree/include/xo/ordinaltree/RedBlackTree.hpp index 78b1555b..7c9347fb 100644 --- a/xo-ordinaltree/include/xo/ordinaltree/RedBlackTree.hpp +++ b/xo-ordinaltree/include/xo/ordinaltree/RedBlackTree.hpp @@ -580,14 +580,19 @@ namespace xo { * R is reduced-value for right child * RB8. RedBlackTree.size() equals the #of nodes in tree */ - bool verify_ok(bool /*throw_flag_not_implemented*/ = true) const { + bool verify_ok(bool /*throw_flag_not_implemented*/ = true) const + { using xo::scope; using xo::tostr; using xo::xtag; constexpr const char *c_self = "RedBlackTree::verify_ok"; - scope log(XO_DEBUG(debug_flag_)); + scope log(XO_DEBUG(debug_flag_), xtag("size", size_)); + if (debug_flag_) { + // look forward to upgrading this to pp + this->display_to_log(); + } /* RB0. */ if (root_ == nullptr) { @@ -610,7 +615,8 @@ namespace xo { int32_t black_height = 0; /* n_node: #of nodes in this->root_ */ - size_t n_node = RbUtil::verify_subtree_ok(this->reduce_fn_, + size_t n_node = RbUtil::verify_subtree_ok(this->node_alloc_, + this->reduce_fn_, this->root_, &black_height); diff --git a/xo-ordinaltree/include/xo/ordinaltree/rbtree/RbTreeUtil.hpp b/xo-ordinaltree/include/xo/ordinaltree/rbtree/RbTreeUtil.hpp index f263b7b4..998b512f 100644 --- a/xo-ordinaltree/include/xo/ordinaltree/rbtree/RbTreeUtil.hpp +++ b/xo-ordinaltree/include/xo/ordinaltree/rbtree/RbTreeUtil.hpp @@ -572,7 +572,7 @@ namespace xo { for (uint32_t iter = 0;; ++iter) { if (c_excessive_verify_enabled) - RbTreeUtil::verify_subtree_ok(reduce_fn, G, nullptr /*&black_height*/); + RbTreeUtil::verify_subtree_ok(alloc, reduce_fn, G, nullptr /*&black_height*/); if (log.enabled()) { if (G) { @@ -720,7 +720,7 @@ namespace xo { RbTreeUtil::rotate(alloc, d, P, reduce_fn, debug_flag, pp_root); if (c_excessive_verify_enabled) - RbTreeUtil::verify_subtree_ok(reduce_fn, S, nullptr /*&black_height*/); + RbTreeUtil::verify_subtree_ok(alloc, reduce_fn, S, nullptr /*&black_height*/); /* (relabel S->P etc. for merged control flow below) */ R = P; @@ -755,7 +755,7 @@ namespace xo { log("verify subtree at GG", xtag("GG", GG), xtag("GG.key", GG->key())); - RbTreeUtil::verify_subtree_ok(reduce_fn, GG, nullptr /*&black_height*/); + RbTreeUtil::verify_subtree_ok(alloc, reduce_fn, GG, nullptr /*&black_height*/); RbTreeUtil::display_aux(D_Invalid, GG, 0 /*depth*/, &log); log("fixup complete"); @@ -841,11 +841,28 @@ namespace xo { RbNode * new_node = RbNode::make_leaf(alloc, kv_pair, reduce_fn.leaf(kv_pair.second)); + log && log(xtag("act", "N gets new leaf"), + xtag("N", N), + xtag("d", d), + xtag("new_node", new_node)); N->assign_child_reparent(alloc, d, new_node); + { + using allocator_traits = xo::gc::gc_allocator_traits; + typename allocator_traits::gc_interface_type gc(alloc); + + const void * src = N; + const void * const * lhs + = reinterpret_cast(&(N->child_v_[0])); + + XO_EXPECT(gc.check_write_barrier(src, lhs, false), + tostr("RbTreeUtil::insert_aux", + ": expect mlog entry for xgen child pointer")); + } + assert(is_red(N->child(d))); /* recalculate Node sizes on path [root .. N] */ @@ -1679,13 +1696,16 @@ namespace xo { * * returns the #of nodes in subtree rooted at N. */ - static size_t verify_subtree_ok(Reduce const & reduce_fn, + template + static size_t verify_subtree_ok(NodeAllocator & alloc, + Reduce const & reduce_fn, RbNode const * N, int32_t * p_black_height) { using xo::scope; using xo::xtag; using xo::print::ccs; + using allocator_traits = xo::gc::gc_allocator_traits; constexpr char const *c_self = "RbTreeUtil::verify_subtree_ok"; @@ -1699,12 +1719,15 @@ namespace xo { /* establish on first leaf node encountered */ uint32_t black_height = 0; + typename allocator_traits::gc_interface_type gc(alloc); + auto verify_fn = [c_self, + &gc, &reduce_fn, &i_node, &last_key, &i_black_height, - &black_height] (RbNode const *x, + &black_height] (RbNode const * x, uint32_t bd) { XO_EXPECT(x->_is_forwarded() == false, @@ -1712,6 +1735,17 @@ namespace xo { xtag("i", i_node), xtag("node[i]", x) )); + if (x->parent()) { + const void * src = x; + const void * const * lhs = reinterpret_cast(&(x->parent_)); + + XO_EXPECT(gc.check_write_barrier(src, lhs, false), + tostr(c_self, (": expect mlog entry for xgen parent pointer"), + xtag("i", i_node), xtag("node[i]", x), + xtag("key[i]", x->key()), + xtag("parent", x->parent()))); + } + /* RB2. if c=x->child(d), then c->parent()=x */ if (x->left_child()) { @@ -1721,6 +1755,17 @@ namespace xo { xtag("key[i]", x->key()), xtag("child", x->left_child()) )); + + { + const void * parent = x; + const void * const * lhs = reinterpret_cast(&(x->child_v_[0])); + + XO_EXPECT(gc.check_write_barrier(parent, lhs, false), + tostr(c_self, (": expect mlog entry for xgen left child pointer"), + xtag("i", i_node), xtag("node[i]", x), + xtag("key[i]", x->key()), + xtag("child", x->left_child()))); + } XO_EXPECT(x == x->left_child()->parent(), tostr(c_self, (": expect symmetric child/parent pointers"), @@ -1731,6 +1776,7 @@ namespace xo { xtag("child.parent", x->left_child()->parent_), xtag("child.parent._is_forwarded", x->left_child()->parent_->_is_forwarded()) )); + } if (x->right_child()) { @@ -1741,6 +1787,17 @@ namespace xo { xtag("child", x->right_child()) )); + { + const void * parent = x; + const void * const * lhs = reinterpret_cast(&(x->child_v_[1])); + + XO_EXPECT(gc.check_write_barrier(parent, lhs, false), + tostr(c_self, (": expect mlog entry for xgen right child pointer"), + xtag("i", i_node), xtag("node[i]", x), + xtag("key[i]", x->key()), + xtag("child", x->right_child()))); + } + XO_EXPECT(x == x->right_child()->parent(), tostr(c_self, ": expect symmetric child/parent pointers", xtag("i", i_node), @@ -1897,6 +1954,8 @@ namespace xo { scope log(XO_DEBUG(true /*debug_flag*/)); display_aux(D_Invalid, N, d, &log); + + assert(false); } /*display*/ }; /*RbTreeUtil*/ } /*namespace detail*/ diff --git a/xo-ordinaltree/utest/RedBlackTree-gc.test.cpp b/xo-ordinaltree/utest/RedBlackTree-gc.test.cpp index b888ddbc..152dc4b1 100644 --- a/xo-ordinaltree/utest/RedBlackTree-gc.test.cpp +++ b/xo-ordinaltree/utest/RedBlackTree-gc.test.cpp @@ -1,4 +1,6 @@ /** @file RedBlackTree-gc.test.cpp + * + * @author Roland Conybeare, Dec 2025 **/ #include "random_tree_ops.hpp" @@ -38,7 +40,7 @@ namespace xo { std::vector s_testcase_v = { - //Testcase_RbTree(1024, 4096, 512, 512, false), + Testcase_RbTree(1024, 4096, 512, 512, false), Testcase_RbTree(1024, 4096, 512, 512, true), }; } @@ -129,8 +131,7 @@ namespace xo { std::uint64_t seed = 8813374093428528487ULL; auto rgen = xo::rng::xoshiro256ss(seed); - //for (std::uint32_t n=0; n<1024; ++n) - for (std::uint32_t n=0; n<=128;) { + for (std::uint32_t n=0; n<=1024;) { bool ok_flag = false; for (std::uint32_t attention = 0; !ok_flag && (attention < 2); ++attention) { @@ -191,26 +192,29 @@ namespace xo { REQUIRE(gc->enable_gc_once()); REQUIRE(gc->gc_in_progress() == false); } + + REQUIRE(rbtree->verify_ok(debug_flag)); + } if (n > 0) { - { - INFO("insert phase B - random_inserts(1, n+1, ..)"); + INFO("insert phase B - random_inserts(1, n+1, ..)"); - /* insert odd integers in [1, n+1), in random order **/ - ok_flag &= TreeUtil::random_inserts(1, n+1, 2, - debug_flag, - &rgen, - rbtree.get()); + /* insert odd integers in [1, n+1), in random order **/ + ok_flag &= TreeUtil::random_inserts(1, n+1, 2, + debug_flag, + &rgen, + rbtree.get()); - if (tc.do_extra_gc_) { - REQUIRE(gc->gc_in_progress() == false); - gc->request_gc(gc::generation::nursery); - REQUIRE(gc->is_gc_pending()); - REQUIRE(gc->enable_gc_once()); - REQUIRE(gc->gc_in_progress() == false); - } + if (tc.do_extra_gc_) { + REQUIRE(gc->gc_in_progress() == false); + gc->request_gc(gc::generation::nursery); + REQUIRE(gc->is_gc_pending()); + REQUIRE(gc->enable_gc_once()); + REQUIRE(gc->gc_in_progress() == false); } + + REQUIRE(rbtree->verify_ok(debug_flag)); } /* check iterator traverses [0..n-1] in both directions */ @@ -250,7 +254,9 @@ namespace xo { debug_flag, rbtree.get(), &rgen); + REQUIRE(rbtree->verify_ok(debug_flag)); + if (tc.do_extra_gc_) { REQUIRE(gc->gc_in_progress() == false); gc->request_gc(gc::generation::nursery); @@ -259,6 +265,8 @@ namespace xo { REQUIRE(gc->gc_in_progress() == false); } + REQUIRE(rbtree->verify_ok(debug_flag)); + /* verify that updates changed tree contents in expected way */ ok_flag &= TreeUtil::check_ordinal_lookup(10000 /*dvalue*/, debug_flag,