From 9803709df8d1d9ba80eefb4cdc72a6497452b7b3 Mon Sep 17 00:00:00 2001 From: Roland Conybeare Date: Thu, 2 Apr 2026 18:55:46 -0400 Subject: [PATCH] xo-gc: refactor: move X1 object storage to new cls GCObjectStore. --- include/xo/gc/DX1Collector.hpp | 33 ++--- include/xo/gc/GCObjectStore.hpp | 81 +++++++++++ include/xo/gc/MutationLogState.hpp | 4 +- src/gc/CMakeLists.txt | 1 + src/gc/DX1Collector.cpp | 208 ++--------------------------- src/gc/GCObjectStore.cpp | 108 +++++++++++++++ utest/Collector.test.cpp | 14 +- 7 files changed, 224 insertions(+), 225 deletions(-) create mode 100644 include/xo/gc/GCObjectStore.hpp create mode 100644 src/gc/GCObjectStore.cpp diff --git a/include/xo/gc/DX1Collector.hpp b/include/xo/gc/DX1Collector.hpp index 135ebc1..7992f47 100644 --- a/include/xo/gc/DX1Collector.hpp +++ b/include/xo/gc/DX1Collector.hpp @@ -7,6 +7,7 @@ #include "X1CollectorConfig.hpp" #include "GCObject.hpp" +#include "GCObjectStore.hpp" #include "MutationLogState.hpp" #include "X1VerifyStats.hpp" #include "generation.hpp" @@ -164,11 +165,11 @@ namespace xo { GCRunState runstate() const noexcept { return runstate_; } const ObjectTypeTable * get_object_types() const noexcept { return &object_types_; } const RootSet * get_root_set() const noexcept { return &root_set_; } - const DArena * get_space(role r, Generation g) const noexcept { return space_[r][g]; } - DArena * get_space(role r, Generation g) noexcept { return space_[r][g]; } - DArena * from_space(Generation g) noexcept { return get_space(role::from_space(), g); } - DArena * to_space(Generation g) noexcept { return get_space(role::to_space(), g); } - DArena * new_space() noexcept { return to_space(Generation{0}); } + const DArena * get_space(role r, Generation g) const noexcept { return gco_store_.get_space(r, g); } + DArena * get_space(role r, Generation g) noexcept { return gco_store_.get_space(r, g); } + DArena * from_space(Generation g) noexcept { return this->get_space(role::from_space(), g); } + DArena * to_space(Generation g) noexcept { return this->get_space(role::to_space(), g); } + DArena * new_space() noexcept { return this->to_space(Generation{0}); } // ----- basic statistics ----- @@ -397,8 +398,10 @@ namespace xo { // ----- book-keeping ----- +#ifdef OBSOLETE // see swap_roles() /** reverse to-space and from-space roles for generation g **/ void reverse_roles(Generation g) noexcept; +#endif /** discard all allocated memory **/ void clear() noexcept; @@ -410,10 +413,6 @@ namespace xo { void _init_gc_roots(const X1CollectorConfig & cfg, std::size_t page_z); /** aux init function: initialize @ref mlog_storage_[][] arenas **/ void _init_mlogs(const X1CollectorConfig & cfg, std::size_t page_z); -#ifdef MOVED - /** aux init function: create mutation log **/ - MutationLog _make_mlog(uint32_t igen, char tag_char, size_t mlog_z, std::size_t page_z); -#endif /** aux init function: initialize @ref space_storage_[][] arenas **/ void _init_space(const X1CollectorConfig & cfg); @@ -493,21 +492,9 @@ namespace xo { **/ MutationLogState mlog_state_; - /** collector-managed memory here. - * - space_[1] is from-space - * - space_[0] is to-space - * coordinates with role in gc/role.hpp, see also. + /** Collector-managed memory. **/ - - /** arena objects for collector managed memory - * 1:1 with roles, but polarity reverses for each collection - **/ - std::array space_storage_[c_n_role]; - - /** arena pointers. The roles of space_storage_[0][g] and space_storage_[1][g] - * are reversed each time generation g gets collected. - **/ - std::array space_[c_n_role]; + GCObjectStore gco_store_; /** counters collected during @ref verify_ok call **/ VerifyStats verify_stats_; diff --git a/include/xo/gc/GCObjectStore.hpp b/include/xo/gc/GCObjectStore.hpp new file mode 100644 index 0000000..35b15d4 --- /dev/null +++ b/include/xo/gc/GCObjectStore.hpp @@ -0,0 +1,81 @@ +/** @file GCObjectStore.hpp + * + * @author Roland Conybeare, Apr 2026 + **/ + +#pragma once + +#include "generation.hpp" +#include +#include +#include +#include + +namespace xo { + namespace mm { + + /** @brief container to hold gc-aware objects for X1 collector + **/ + class GCObjectStore { + public: + GCObjectStore(const ArenaConfig & arena_cfg, uint32_t ngen, bool debug_flag); + + const DArena * get_space(role r, Generation g) const noexcept { return space_[r][g]; } + DArena * get_space(role r, Generation g) noexcept { return space_[r][g]; } + DArena * from_space(Generation g) noexcept { return get_space(role::from_space(), g); } + DArena * to_space(Generation g) noexcept { return get_space(role::to_space(), g); } + DArena * new_space() noexcept { return to_space(Generation{0}); } + + /** Call @p visitor for each memory pool owned by this store **/ + void visit_pools(const MemorySizeVisitor & visitor) const; + + /** For each generation g in [0 ,.., upto) + * swap arenas assigned to {to-space, from-space}. + * Invoked once at the beginning of each gc cycle. + **/ + void swap_roles(Generation upto) noexcept; + + /** Cleanup at the end of a gc cycle. + * Reset from-space + * (current from-space is former to-space, + * relabeled at the beginning of collector cycle) + * for generations in [0 ,.., upto) + **/ + void cleanup_phase(Generation upto, + bool sanitize_flag); + + private: + /** auxiliary init function **/ + void _init_space(); + + private: + /** Configuration for collector spaces. + * Will have (2 x G) of these, + * where G is @ref n_generation_. + * Not using name_ member. + * + * REQUIRE: + * - arena_config_.store_header_flag_ must be true + **/ + ArenaConfig arena_config_; + /** number of generations in use. Same as @ref X1CollectorConfig::n_generation_ **/ + uint32_t n_generation_ = 0; + /** true to enable debug logging **/ + bool debug_flag_ = false; + + /** arena objects for collector managed memory + * 1:1 with roles, but polarity reverses for each collection + **/ + std::array space_storage_[c_n_role]; + + /** arena pointers. The roles of space_storage_[0][g] and space_storage_[1][g] + * are reversed each time generation g gets collected. + **/ + std::array space_[c_n_role]; + + }; + + } /*namespace mm*/ +} /*namespace xo*/ + +/* end GCObjectStore.hpp */ diff --git a/include/xo/gc/MutationLogState.hpp b/include/xo/gc/MutationLogState.hpp index ef7e96a..57c9314 100644 --- a/include/xo/gc/MutationLogState.hpp +++ b/include/xo/gc/MutationLogState.hpp @@ -16,6 +16,8 @@ namespace xo { class DX1Collector; class VerifyStats; + /** @brief container for X1 collector mutation logs + **/ class MutationLogState { public: using MutationLog = DArenaVector; @@ -133,7 +135,7 @@ namespace xo { public: - /** number of generations in use. Same as @ref X1CollectorConfig.n_generation_ **/ + /** number of generations in use. Same as @ref X1CollectorConfig::n_generation_ **/ uint32_t n_generation_ = 0; /** true to enable debug logging **/ bool debug_flag_ = false; diff --git a/src/gc/CMakeLists.txt b/src/gc/CMakeLists.txt index d30acf0..3f61335 100644 --- a/src/gc/CMakeLists.txt +++ b/src/gc/CMakeLists.txt @@ -15,6 +15,7 @@ set(SELF_SRCS DX1CollectorIterator.cpp X1CollectorConfig.cpp + GCObjectStore.cpp MutationLogState.cpp MutationLogEntry.cpp diff --git a/src/gc/DX1Collector.cpp b/src/gc/DX1Collector.cpp index 342da1c..de9af1e 100644 --- a/src/gc/DX1Collector.cpp +++ b/src/gc/DX1Collector.cpp @@ -66,7 +66,9 @@ namespace xo { using size_type = xo::mm::DX1Collector::size_type; DX1Collector::DX1Collector(const X1CollectorConfig & cfg) - : config_{cfg}, mlog_state_{cfg.n_generation_, cfg.debug_flag_} + : config_{cfg}, + mlog_state_{cfg.n_generation_, cfg.debug_flag_}, + gco_store_{cfg.arena_config_, cfg.n_generation_, cfg.debug_flag_} { assert(config_.arena_config_.header_.size_bits_ + config_.arena_config_.header_.age_bits_ + @@ -77,7 +79,9 @@ namespace xo { this->_init_object_types(cfg, page_z); this->_init_gc_roots(cfg, page_z); this->_init_mlogs(cfg, page_z); +#ifdef MOVED this->_init_space(cfg); +#endif } void @@ -108,86 +112,6 @@ namespace xo { DX1Collector::_init_mlogs(const X1CollectorConfig & cfg, std::size_t page_z) { this->mlog_state_.init_mlogs(cfg, page_z); - -#ifdef MOVED - for (uint32_t igen = 0, ngen = cfg.n_generation_; igen + 1 < ngen; ++igen) { - // special case: no use for mutation log for youngest generation, - // so don't trouble to allocate one - - if (igen + 1 < c_max_generation) { - this->mlog_storage_[0][igen] = _make_mlog(igen, 'a', cfg.mutation_log_z_, page_z); - this->mlog_storage_[1][igen] = _make_mlog(igen, 'b', cfg.mutation_log_z_, page_z); - this->mlog_storage_[2][igen] = _make_mlog(igen, 'c', cfg.mutation_log_z_, page_z); - - this->mlog_[0][igen] = &mlog_storage_[0][igen]; - this->mlog_[1][igen] = &mlog_storage_[1][igen]; - this->mlog_[2][igen] = &mlog_storage_[2][igen]; - } else { - assert(false); - } - } - - if (cfg.n_generation_ > 0) { - for (uint32_t igen = cfg.n_generation_ - 1; igen + 1 < c_max_generation; ++igen) { - this->mlog_[0][igen] = nullptr; - this->mlog_[1][igen] = nullptr; - this->mlog_[2][igen] = nullptr; - } - } else { - assert(false); - } -#endif - } - -#ifdef MOVED - auto - DX1Collector::_make_mlog(uint32_t igen, char tag_char, size_t mlog_z, size_t page_z) -> MutationLog - { - char buf[40]; - snprintf(buf, sizeof(buf), "x1-mlog-G%u-%c", igen, tag_char); - - return MutationLog::map(ArenaConfig{.name_ = std::string(buf), - .size_ = mlog_z, - .hugepage_z_ = page_z, - .store_header_flag_ = false}); - } -#endif - - void - DX1Collector::_init_space(const X1CollectorConfig & cfg) - { - assert(c_n_role == 2); - - for (uint32_t igen = 0, ngen = cfg.n_generation_; igen < ngen; ++igen) { - if (igen < c_max_generation) { - { - char buf[40]; - snprintf(buf, sizeof(buf), "x1-space-G%u-a", igen); - - this->space_storage_[0][igen] = DArena::map(cfg.arena_config_.with_name(std::string(buf))); - } - { - char buf[40]; - snprintf(buf, sizeof(buf), "x1-space-G%u-b", igen); - - this->space_storage_[1][igen] = DArena::map(cfg.arena_config_.with_name(std::string(buf))); - } - - this->space_[role::to_space()][igen] = &space_storage_[0][igen]; - this->space_[role::from_space()][igen] = &space_storage_[1][igen]; - } else { - assert(false); - } - } - - for (uint32_t igen = cfg.n_generation_; igen < c_max_generation; ++igen) { - this->space_[role::to_space()][igen] = nullptr; - this->space_[role::from_space()][igen] = nullptr; - } - - if (config_.n_generation_ == 2) { - assert(this->get_space(role::to_space(), Generation{2}) == nullptr); - } } void @@ -196,21 +120,8 @@ namespace xo { object_types_.visit_pools(visitor); root_set_.visit_pools(visitor); - for (uint32_t j = 0; j < config_.n_generation_; ++j) { - for (uint32_t i = 0; i < c_n_role; ++i) { - space_storage_[i][j].visit_pools(visitor); - } - } - + gco_store_.visit_pools(visitor); mlog_state_.visit_pools(visitor); - -#ifdef MOVED - for (uint32_t j = 0; j + 1 < config_.n_generation_; ++j) { - for (uint32_t i = 0; i < c_n_role + 1; ++i) { - mlog_storage_[i][j].visit_pools(visitor); - } - } -#endif } bool @@ -959,12 +870,7 @@ namespace xo { { scope log(XO_DEBUG(true), xtag("upto", upto)); - for (Generation g = Generation{0}; g < upto; ++g) { - log && log("swap roles", xtag("g", g)); - - std::swap(space_[role::to_space()][g], space_[role::from_space()][g]); - } - + gco_store_.swap_roles(upto); mlog_state_.swap_roles(upto); } @@ -974,108 +880,12 @@ namespace xo { mlog_state_.forward_mutation_log(this, upto); } -#ifdef MOVED - MutationLogStatistics - DX1Collector::_preserve_child_of_live_parent(Generation upto, - Generation parent_gen, - const MutationLogEntry & from_entry, - MutationLog * keep_mlog) - { - void * child_fr = *from_entry.p_data(); - AllocInfo child_info = this->alloc_info((std::byte *)(child_fr)); - - MutationLogStatistics counters; - - // if child collected: new child location in to-space - void * child_to = nullptr; - - // parent is alive: gc must ensure child remains alive - - ++counters.n_live_parent_; - - // Parent already recognized as alive. Either not subject to collection - // or already evacuated. - // (+ remember this need not be 1st pass over mlog entries) - - if (child_info.is_forwarding_tseq()) { - // [MLOG1] - - // child already forwarded. - // TODO: make this a method on AllocInfo - child_to = *(void **)child_fr; - - // assigning through address of P->C pointer - // also makes mlog entry current - - } else { - // [MLOG2] - - ++counters.n_rescue_; - - child_to = this->_deep_move_interior(child_fr, upto); - - // update child pointer in parent object - *from_entry.p_data() = child_to; - } - - // child_to generation in {gen, gen+1} - - this->_check_keep_mutation_aux(from_entry, parent_gen, child_to, keep_mlog); - - return counters; - } -#endif - -#ifdef MOVED - bool - DX1Collector::_check_keep_mutation_aux(const MutationLogEntry & from_entry, - Generation parent_gen_to, - void * child_to, - MutationLog * keep_mlog) - { - Generation child_gen_to - = this->generation_of(role::to_space(), child_to); - - bool need_mlog_entry - = ((child_gen_to + 1 < config_.n_generation_) - && (config_.promotion_threshold(parent_gen_to) - > 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 - void DX1Collector::_cleanup_phase(Generation upto) { scope log(XO_DEBUG(true), xtag("upto", upto)); - // everything live has been copied out of from-space - // -> now set to empty - // - for (Generation g = Generation{0}; g < upto; ++g) { - if (config_.sanitize_flag_) { - space_[role::from_space()][g]->scrub(); - } - - space_[role::from_space()][g]->clear(); - } - + this->gco_store_.cleanup_phase(upto, config_.sanitize_flag_); this->runstate_ = GCRunState::idle(); } @@ -1786,12 +1596,14 @@ namespace xo { arena_end); } +#ifdef MOVED void DX1Collector::reverse_roles(Generation g) noexcept { assert(g < config_.n_generation_); std::swap(space_[role::from_space()][g], space_[role::to_space()][g]); } +#endif void DX1Collector::clear() noexcept { diff --git a/src/gc/GCObjectStore.cpp b/src/gc/GCObjectStore.cpp new file mode 100644 index 0000000..7819f21 --- /dev/null +++ b/src/gc/GCObjectStore.cpp @@ -0,0 +1,108 @@ +/** @file GCObjectStore.cpp + * + * @author Roland Conybeare, Apr 2026 + **/ + +#include "GCObjectStore.hpp" +#include +#include + +namespace xo { + namespace mm { + + GCObjectStore::GCObjectStore(const ArenaConfig & arena_cfg, + uint32_t ngen, bool debug_flag) + : arena_config_{arena_cfg}, + n_generation_{ngen}, + debug_flag_{debug_flag} + { + assert(arena_config_.header_.size_bits_ + + arena_config_.header_.age_bits_ + + arena_config_.header_.tseq_bits_ <= 64); + + this->_init_space(); + } + + void + GCObjectStore::_init_space() + { + assert(c_n_role == 2); + + for (uint32_t igen = 0, ngen = n_generation_; igen < ngen; ++igen) { + if (igen < c_max_generation) { + { + char buf[40]; + snprintf(buf, sizeof(buf), "x1-space-G%u-a", igen); + + this->space_storage_[0][igen] + = DArena::map(arena_config_.with_name(std::string(buf))); + } + { + char buf[40]; + snprintf(buf, sizeof(buf), "x1-space-G%u-b", igen); + + this->space_storage_[1][igen] + = DArena::map(arena_config_.with_name(std::string(buf))); + } + + this->space_[role::to_space()][igen] = &space_storage_[0][igen]; + this->space_[role::from_space()][igen] = &space_storage_[1][igen]; + } else { + assert(false); + } + } + + for (uint32_t igen = n_generation_; igen < c_max_generation; ++igen) { + this->space_[role::to_space()][igen] = nullptr; + this->space_[role::from_space()][igen] = nullptr; + } + + if (n_generation_ == 2) { + assert(this->get_space(role::to_space(), Generation{2}) == nullptr); + } + } + + void + GCObjectStore::visit_pools(const MemorySizeVisitor & visitor) const + { + for (uint32_t j = 0; j < n_generation_; ++j) { + for (uint32_t i = 0; i < c_n_role; ++i) { + space_storage_[i][j].visit_pools(visitor); + } + } + } + + void + GCObjectStore::swap_roles(Generation upto) noexcept + { + scope log(XO_DEBUG(true), xtag("upto", upto)); + + for (Generation g = Generation{0}; g < upto; ++g) { + log && log("swap roles", xtag("g", g)); + + std::swap(space_[role::to_space()][g], space_[role::from_space()][g]); + } + } + + void + GCObjectStore::cleanup_phase(Generation upto, + bool sanitize_flag) + { + scope log(XO_DEBUG(true), xtag("upto", upto)); + + // everything live has been copied out of from-space + // -> now set to empty + // + for (Generation g = Generation{0}; g < upto; ++g) { + if (sanitize_flag) { + space_[role::from_space()][g]->scrub(); + } + + space_[role::from_space()][g]->clear(); + } + } + + } /*namespace mm*/ +} /*namespace xo*/ + +/* end GCObjectStore.cpp */ diff --git a/utest/Collector.test.cpp b/utest/Collector.test.cpp index 8b464a2..225a5ab 100644 --- a/utest/Collector.test.cpp +++ b/utest/Collector.test.cpp @@ -91,15 +91,23 @@ namespace xo { REQUIRE(gc.to_space(g1) != gc.to_space(g0)); REQUIRE(gc.from_space(g1) != gc.from_space(g0)); REQUIRE(gc.to_space(g0) != gc.from_space(g1)); + REQUIRE(gc.to_space(g1) != gc.from_space(g1)); + + for (Generation gi{0}; gi < 2; ++gi) { + INFO(xtag("gi", gi)); + + REQUIRE(gc.to_space(gi)); + REQUIRE(gc.from_space(gi)); + + REQUIRE(gc.from_space(gi)->is_mapped()); + REQUIRE(gc.to_space(gi)->is_mapped()); + } for (Generation gi = Generation(2); gi < c_max_generation; ++gi) { INFO(xtag("gi", gi)); REQUIRE(!gc.to_space(gi)); REQUIRE(!gc.from_space(gi)); - - REQUIRE(!gc.space_storage_[0][gi].is_mapped()); - REQUIRE(!gc.space_storage_[1][gi].is_mapped()); } }