From 53d9ab77471032a165c34e2ffa963f35c05808f0 Mon Sep 17 00:00:00 2001 From: Roland Conybeare Date: Fri, 10 Apr 2026 20:32:55 -0400 Subject: [PATCH] xo-gc: refactor: migrate verify impl DX1Collector -> GCObjectStore --- include/xo/gc/DX1Collector.hpp | 2 + include/xo/gc/GCObjectStore.hpp | 12 +++++ src/gc/DX1Collector.cpp | 4 +- src/gc/GCObjectStore.cpp | 77 ++++++++++++++++++++++++++------- utest/GCObjectStore.test.cpp | 77 ++++++++++++++++++++++++++++++--- 5 files changed, 149 insertions(+), 23 deletions(-) diff --git a/include/xo/gc/DX1Collector.hpp b/include/xo/gc/DX1Collector.hpp index 98fc254..b0feefc 100644 --- a/include/xo/gc/DX1Collector.hpp +++ b/include/xo/gc/DX1Collector.hpp @@ -358,10 +358,12 @@ namespace xo { /** cleanup after gc **/ void _cleanup_phase(Generation upto); +#ifdef OBSOLETE /** Verify that pointer {@p iface, @p data} is valid: * destination either in to-space, or somewhere outside this collector **/ void _verify_aux(AGCObject * iface, void * data); +#endif public: /** garbage collector configuration **/ diff --git a/include/xo/gc/GCObjectStore.hpp b/include/xo/gc/GCObjectStore.hpp index dbb591c..e1acf0c 100644 --- a/include/xo/gc/GCObjectStore.hpp +++ b/include/xo/gc/GCObjectStore.hpp @@ -55,6 +55,9 @@ namespace xo { **/ Generation generation_of(Role r, const void * addr) const noexcept; + /** return details from last error (i.e. from g0 to-space) **/ + AllocError last_error() const noexcept; + /** get allocation size from header **/ std::size_t header2size(header_type hdr) const noexcept; /** get generation counter from alloc header **/ @@ -167,6 +170,15 @@ namespace xo { void ** lhs_data, Generation upto); + /** categorize fop {@p lhs_iface, @p lhs_data} + * based on location of @p lhs_data. + * Update @p *p_verify_stats based on the result: + * increment exactly one of {n_from_, n_to_, n_ext_} + **/ + void verify_aux(AGCObject * lhs_iface, + void * lhs_data, + X1VerifyStats * p_verify_stats); + /** Cleanup at the end of a gc cycle. * Reset from-space * (current from-space is former to-space, diff --git a/src/gc/DX1Collector.cpp b/src/gc/DX1Collector.cpp index 63b6ef5..a358e68 100644 --- a/src/gc/DX1Collector.cpp +++ b/src/gc/DX1Collector.cpp @@ -604,7 +604,7 @@ namespace xo { } case VisitReason::code::verify: // called during verify_ok - this->_verify_aux(lhs_iface, *lhs_data); + gco_store_.verify_aux(lhs_iface, *lhs_data, &verify_stats_); break; default: // should be unreachable @@ -612,6 +612,7 @@ namespace xo { } } +#ifdef OBSOLETE void DX1Collector::_verify_aux(AGCObject * iface, void * data) { @@ -640,6 +641,7 @@ namespace xo { ++(verify_stats_.n_to_); } } +#endif auto DX1Collector::alloc(typeseq t, size_type z) noexcept -> value_type diff --git a/src/gc/GCObjectStore.cpp b/src/gc/GCObjectStore.cpp index 1f54a4c..e20f92a 100644 --- a/src/gc/GCObjectStore.cpp +++ b/src/gc/GCObjectStore.cpp @@ -164,6 +164,12 @@ namespace xo { return Generation::sentinel(); } + AllocError + GCObjectStore::last_error() const noexcept + { + return this->get_space(Role::to_space(), Generation{0})->last_error(); + } + auto GCObjectStore::header2size(header_type hdr) const noexcept -> size_type { @@ -364,16 +370,35 @@ namespace xo { (void)error_mm; - std::uint64_t n_age = config_.arena_config_.header_.max_age() + 1; + std::uint32_t hard_n_age = config_.arena_config_.header_.max_age() + 1; + // maximum age of a still-existing object + std::uint32_t soft_max_age = 0; + + // first pass, establish max age + for (Generation g{0}; g < config_.n_generation_; ++g) { + const DArena * arena = this->get_space(Role::to_space(), g); + + for (AllocInfo info : *arena) { + if (info.is_forwarding_tseq()) { + assert(false); + return false; + } + + uint32_t age = info.age(); + + assert(age < hard_n_age); + + soft_max_age = std::max(soft_max_age, age); + } + } // stats, indexed by age - DArray * stats_v = DArray::empty(mm, n_age); + DArray * stats_v = DArray::empty(mm, soft_max_age + 1); if (!stats_v) return false; - // pre-populate with empty dictionaries for each age bucket - for (std::uint64_t a = 0; a < n_age; ++a) { + for (std::uint32_t a = 0; a <= soft_max_age; ++a) { DDictionary * recd = DDictionary::make(mm); if (!recd) @@ -386,14 +411,12 @@ namespace xo { stats_v->push_back(obj(recd)); } - log && log(xtag("n_age", n_age), + log && log(xtag("soft_max_age", soft_max_age), xtag("stats_v.size", stats_v->size())); + // second pass, populate // scan to-space, count objects by age - - // track largest age with at least one object - std::int64_t max_age_present = 0; - + // for (Generation g{0}; g < config_.n_generation_; ++g) { const DArena * arena = this->get_space(Role::to_space(), g); @@ -406,9 +429,6 @@ namespace xo { uint32_t age = info.age(); size_t z = info.size(); - if (static_cast(age) > max_age_present) - max_age_present = age; - auto recd = obj::from(stats_v->at(age)); assert(recd); @@ -428,9 +448,6 @@ namespace xo { } } - // trim to only report ages up to max observed - stats_v->resize(max_age_present + 1); - *p_output = obj(stats_v); return true; @@ -642,6 +659,36 @@ namespace xo { } } /*_forward_inplace_aux*/ + void + GCObjectStore::verify_aux(AGCObject * iface, + void * data, + X1VerifyStats * p_verify_stats) + { + //scope log(XO_DEBUG(config_.debug_flag_), xtag("data", data)); + + (void)iface; + + Generation g1 = this->generation_of(Role::to_space(), data); + + if (g1.is_sentinel()) { + assert(this->contains(Role::to_space(), data) == false); + + Generation g2 = this->generation_of(Role::from_space(), data); + + if (!g2.is_sentinel()) { + // verify failure - live pointer still refers to from-space + + ++(p_verify_stats->n_from_); + } else { + ++(p_verify_stats->n_ext_); + } + } else { + assert(this->contains(Role::to_space(), data)); + + ++(p_verify_stats->n_to_); + } + } + void GCObjectStore::swap_roles(Generation upto) noexcept { diff --git a/utest/GCObjectStore.test.cpp b/utest/GCObjectStore.test.cpp index 7576656..198c021 100644 --- a/utest/GCObjectStore.test.cpp +++ b/utest/GCObjectStore.test.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -53,6 +54,8 @@ namespace ut { explicit Testcase(uint32_t n_gen, uint32_t n_survive, size_t gc_z, uint32_t type_z, bool do_type_registration, + size_t report_z, + size_t error_z, uint32_t n_test_obj, uint32_t n_test_assign) : n_gen_{n_gen}, @@ -60,6 +63,8 @@ namespace ut { gc_size_{gc_z}, object_type_z_{type_z}, do_type_registration_{do_type_registration}, + report_size_{report_z}, + error_size_{error_z}, n_test_obj_{n_test_obj}, n_test_assign_{n_test_assign} {} @@ -80,20 +85,29 @@ namespace ut { * (i.e. DBoolean) **/ bool do_type_registration_ = false; + /** size for report-output arena **/ + size_t report_size_ = 0; + /** size for error-output arena **/ + size_t error_size_ = 0; /** #of cells in random object graph **/ uint32_t n_test_obj_ = 0; /** #of random assignments to attempt (these may create cycles, for example) **/ uint32_t n_test_assign_ = 0; }; + constexpr uint32_t c_report_z1 = 64 * 1024; + constexpr uint32_t c_error_z1 = 16 * 1024; + static std::vector s_testcase_v = { - /** n_gen, n_survive, gc_size, object_type_z, do_type_registration, n_obj **/ - Testcase(2, 4, 16 * 1024, 8 * 128, false, 0, 0), - Testcase(2, 4, 16 * 1024, 8 * 128, true, 1, 0), - Testcase(2, 4, 16 * 1024, 8 * 128, true, 2, 0), - Testcase(2, 4, 16 * 1024, 8 * 128, true, 4, 0), - Testcase(2, 4, 16 * 1024, 8 * 128, true, 8, 4), - Testcase(2, 4, 16 * 1024, 8 * 128, true, 16, 7), + // note: report_z: 64k not sufficient for report_object_ages() + + /** n_gen, n_survive, gc_size, object_type_z, do_type_registration, report_z, error_z, n_obj, n_test_assign **/ + Testcase(2, 4, 16 * 1024, 8 * 128, false, c_report_z1, c_error_z1, 0, 0), + Testcase(2, 4, 16 * 1024, 8 * 128, true, c_report_z1, c_error_z1, 1, 0), + Testcase(2, 4, 16 * 1024, 8 * 128, true, c_report_z1, c_error_z1, 2, 0), + Testcase(2, 4, 16 * 1024, 8 * 128, true, c_report_z1, c_error_z1, 4, 0), + Testcase(2, 4, 16 * 1024, 8 * 128, true, c_report_z1, c_error_z1, 8, 4), + Testcase(2, 4, 16 * 1024, 8 * 128, true, c_report_z1, c_error_z1, 16, 7), }; /** record capturing some stats for a (randomly created) gc-aware object **/ @@ -300,6 +314,22 @@ namespace ut { .with_size(tc.gc_size_ * tc.n_gen_) .with_store_header_flag(true)); + /** Arena for holding report output: + * See GCObjectStore methods .report_object_types(), .report_object_ages() + **/ + DArena report_arena + = DArena::map(ArenaConfig().with_name("report-arena") + .with_size(tc.report_size_) + .with_store_header_flag(true)); + obj report_mm(&report_arena); + + /** Arena for holding error messages **/ + DArena error_arena + = DArena::map(ArenaConfig().with_name("error-arena") + .with_size(tc.error_size_) + .with_store_header_flag(true)); + obj error_mm(&error_arena); + // object type storage will be empty unless we install a type. GCObjectStore gcos(gcos_config); @@ -611,6 +641,39 @@ namespace ut { #endif // - verify_ok + + { + obj report_gco; + bool ok = gcos.report_object_types(report_mm, error_mm, &report_gco); + + REQUIRE(ok); + REQUIRE(report_gco); + + // TODO: print report_gco, verify output + + // discard report + + report_gco.reset(); + report_mm->clear(); + } + + { + obj report_gco; + bool ok = gcos.report_object_ages(report_mm, error_mm, &report_gco); + + if (!ok) { + log.retroactively_enable(); + log && log(xtag("error", report_mm.last_error())); + } + + REQUIRE(ok); + REQUIRE(report_gco); + + // TODO: print report_gco, verify output + + report_gco.reset(); + report_mm->clear(); + } } }