From e454bee6afaea69704b07255d99cfcea33872f62 Mon Sep 17 00:00:00 2001 From: Roland Conybeare Date: Tue, 2 Dec 2025 17:07:19 -0500 Subject: [PATCH] xo-alloc: UT for allocator interation + misc improvements --- xo-alloc/include/xo/alloc/ArenaAllocT.hpp | 74 -------- xo-alloc/utest/ArenaAllocT.test.cpp | 67 ------- xo-alloc/utest/CMakeLists.txt | 1 - xo-alloc/utest/Forwarding1.test.cpp | 10 +- xo-alloc/utest/GC.test.cpp | 179 +++++++++++++++++- xo-alloc/utest/IAlloc.test.cpp | 118 +++++++++++- xo-allocutil/include/xo/allocutil/IAlloc.hpp | 6 +- .../include/xo/allocutil/ObjectVisitor.hpp | 53 ++++++ .../include/xo/ordinaltree/rbtree/Node.hpp | 25 ++- xo-ordinaltree/utest/RedBlackTree-gc.test.cpp | 21 +- 10 files changed, 383 insertions(+), 171 deletions(-) delete mode 100644 xo-alloc/include/xo/alloc/ArenaAllocT.hpp delete mode 100644 xo-alloc/utest/ArenaAllocT.test.cpp create mode 100644 xo-allocutil/include/xo/allocutil/ObjectVisitor.hpp diff --git a/xo-alloc/include/xo/alloc/ArenaAllocT.hpp b/xo-alloc/include/xo/alloc/ArenaAllocT.hpp deleted file mode 100644 index 6c8a9b41..00000000 --- a/xo-alloc/include/xo/alloc/ArenaAllocT.hpp +++ /dev/null @@ -1,74 +0,0 @@ -/** @file Allocator.hpp - * - * @author Roland Conybeare, Nov 2025 - **/ - -#pragma once - -#include "xo/alloc/ArenaAlloc.hpp" - -namespace xo { - namespace gc { - /** @class allocator - * @brief c++ allocator with allocator traits - * - * Can use ArenaAllocT with std::map etc. - **/ - template - class ArenaAllocT { - public: - using value_type = T; - /** copy assignment: leave lhs allocator in place **/ - using propagate_on_container_copy_assignment = std::false_type; - /** move assignment: adopt rhs allocator - * (Forced: cannot mix allocations from different allocators - * within a container) - **/ - using propagate_on_container_move_assignment = std::true_type; - /** swap: also swap allocators - * (Forced: cannot mix allocations from different allocators - * within a containers) - **/ - using propagate_on_container_swap = std::true_type; - /** An ArenaAlloc instance is unique owner of its own memory: - * no other instance can dealloc - **/ - using is_always_equal = std::false_type; - - public: - explicit ArenaAllocT(ArenaAlloc * mm) : mm_{mm} {} - ArenaAllocT(const ArenaAllocT & other) = default; - - /** rebind ctor. Allows container to use supplied allocator - * for multiple types - **/ - template - ArenaAllocT(const ArenaAllocT & other) noexcept : mm_{other.mm_} {} - - T * allocate(size_t n) { - void * mem = mm_->alloc(n * sizeof(T)); - - return reinterpret_cast(mem); - } - - void deallocate(T * p, size_t n) noexcept { - assert(mm_->contains(p)); - assert(n == 0 || mm_->contains(p + n - 1)); - - //arena_->deallocate(p, n * sizeof(T)); - } - - bool operator==(const ArenaAllocT & other) const { - return mm_ == other.mm_; - } - - bool operator!=(const ArenaAllocT & other) const { - return mm_ != other.mm_; - } - - ArenaAlloc * mm_ = nullptr; - }; - } /*namespace gc*/ -} /*namespace xo*/ - -/* end Allocator.hpp */ diff --git a/xo-alloc/utest/ArenaAllocT.test.cpp b/xo-alloc/utest/ArenaAllocT.test.cpp deleted file mode 100644 index a9257173..00000000 --- a/xo-alloc/utest/ArenaAllocT.test.cpp +++ /dev/null @@ -1,67 +0,0 @@ -/** @file ArenaAllocT.test.cpp - * - * @author Roland Conybeare, Nov 2025 - **/ - -#include "xo/alloc/ArenaAllocT.hpp" -#include -#include - -namespace xo { - using xo::gc::ArenaAllocT; - using xo::gc::ArenaAlloc; - - namespace ut { - - namespace { - struct testcase_ArenaAllocT { - std::size_t arena_z_; - std::vector> kv_pairs_; - }; - - std::vector - s_testcase_v = { - { 4096, {} }, - { 4096, {{"a", "apple"}} }, - { 4096, {{"a", "apple"}, {"b", "banana"}, {"c", "carrot"}} }, - { 4096, {{"a", "apple"}, {"b", "banana"}, {"c", "carrot"}, {"e", "eggplant"}} }, - }; - } - - TEST_CASE("ArenaAllocT", "[alloc][traits]") - { - for (std::size_t i_tc = 0, n_tc = s_testcase_v.size(); i_tc < n_tc; ++i_tc) { - const testcase_ArenaAllocT & tc = s_testcase_v[i_tc]; - - constexpr bool c_debug_flag = false; - - auto arena = ArenaAlloc::make("arena", tc.arena_z_, c_debug_flag); - auto alloc = ArenaAllocT>(arena.get()); - - using TestMapType = std::map, - ArenaAllocT>>; - - TestMapType test_map(alloc); - - size_t n = 0; - for (const auto & kv_ix : tc.kv_pairs_) { - test_map[kv_ix.first] = kv_ix.second; - ++n; - - REQUIRE(test_map.size() == n); - - for (const auto & map_ix : test_map) { - // verify alloc was used for both Key + Value. - REQUIRE(arena->contains(&map_ix.first)); - REQUIRE(arena->contains(&map_ix.second)); - } - } - - } - } - } -} /*namespace xo*/ - -/* end ArenaAllocT.test.cpp */ diff --git a/xo-alloc/utest/CMakeLists.txt b/xo-alloc/utest/CMakeLists.txt index 6c644f51..366cf664 100644 --- a/xo-alloc/utest/CMakeLists.txt +++ b/xo-alloc/utest/CMakeLists.txt @@ -7,7 +7,6 @@ set(UTEST_SRCS alloc_utest_main.cpp IAlloc.test.cpp ArenaAlloc.test.cpp - ArenaAllocT.test.cpp ListAlloc.test.cpp GC.test.cpp GcStatistics.test.cpp diff --git a/xo-alloc/utest/Forwarding1.test.cpp b/xo-alloc/utest/Forwarding1.test.cpp index f0a81c14..64c9f3f8 100644 --- a/xo-alloc/utest/Forwarding1.test.cpp +++ b/xo-alloc/utest/Forwarding1.test.cpp @@ -7,6 +7,7 @@ #include "ArenaAlloc.hpp" #include "xo/reflect/Reflect.hpp" #include +#include #include namespace xo { @@ -58,7 +59,14 @@ namespace xo { std::stringstream ss; ss << fwd; - REQUIRE(ss.str() == ""); + // forwarding printer looks like + // "" + // + + std::regex pattern(R"()"); + REQUIRE(std::regex_match(ss.str(), pattern)); + + //REQUIRE(ss.str() == ""); tag_config::tag_color_enabled = saved; } diff --git a/xo-alloc/utest/GC.test.cpp b/xo-alloc/utest/GC.test.cpp index 9e6c17e3..c5245243 100644 --- a/xo-alloc/utest/GC.test.cpp +++ b/xo-alloc/utest/GC.test.cpp @@ -4,12 +4,16 @@ */ #include "xo/alloc/GC.hpp" +#include "xo/allocutil/gc_allocator_traits.hpp" #include namespace xo { + using xo::gc::IAlloc; using xo::gc::GC; + using xo::gc::gc_allocator_traits; using xo::gc::generation; using xo::gc::Config; + using xo::reflect::TaggedPtr; namespace ut { @@ -80,13 +84,186 @@ namespace xo { REQUIRE(gc->gc_in_progress() == false); REQUIRE(gc->native_gc_statistics().gen_v_[gen2int(generation::nursery)].n_gc_ == 1); REQUIRE(gc->native_gc_statistics().gen_v_[gen2int(generation::tenured)].n_gc_ == 1); - } catch (std::exception &ex) { + } catch (std::exception & ex) { std::cerr << "caught exception: " << ex.what() << std::endl; REQUIRE(false); } } } + /** gc-enabled allocator **/ + namespace { + /** Setup test with custom allocator + * + **/ + template + struct TestClass : public GcObjectInterface { + TestClass() = default; + explicit TestClass(const Nested & member1) : member1_{member1} {} + + // using allocator_type = Allocator; + // using allocator_traits = xo::gc::gc_allocator_traits; + + /** stage1 - just allocates some memory using allocator **/ + template + static TestClass * make_0(Allocator & alloc) { + TestClass * mem = alloc.allocate(sizeof(TestClass)); + + /* but ctor will not have run, so ub to visit object */ + + return mem; + } + + /** stage2 - use allocator_traits construct **/ + template + static TestClass * make_1(Allocator & alloc) { + using traits = gc_allocator_traits; + + TestClass * mem = traits::allocate(alloc, 1); + + /* ctor will not have run here either */ + return mem; + } + + /** stage3 - invoke construct **/ + template + static TestClass * make_2(Allocator & alloc) { + using traits = gc_allocator_traits; + + TestClass * obj = traits::allocate(alloc, 1); + try { + // placement new + traits::construct(alloc, obj); + + return obj; + } catch(...) { + traits::deallocate(alloc, obj, 1); + throw; + } + } + + /** stage4 - init nested type **/ + template + static TestClass * make_3(Allocator & alloc) { + using traits = gc_allocator_traits; + + TestClass * obj = traits::allocate(alloc, 1); + try { + Nested nested; + + // placemenet new + traits::construct(alloc, obj); + + return obj; + } catch(...) { + traits::deallocate(alloc, obj, 1); + throw; + } + } + + // ----- inherited from Object ----- + + virtual TaggedPtr self_tp() const final override { + assert(false); return TaggedPtr::universal_null(); + } + virtual void display(std::ostream & os) const final override { + os << ""; + } + virtual std::size_t _shallow_size() const final override { + assert(false); return sizeof(*this); + } + virtual IObject * _shallow_copy(IAlloc * gc) const final override { + assert(false); return nullptr; + } + virtual std::size_t _forward_children(IAlloc * gc) final override { + assert(false); return _shallow_size(); + } + + Nested member1_; + }; + + struct MemberType { + MemberType() : ctor_ran_{true} {} + explicit MemberType(const std::vector> & mem2) : member2_{mem2} {} + + std::vector> member2_; + std::size_t ctor_ran_ = false; + }; + } + + TEST_CASE("gc_allocator_traits", "[alloc][gc]") + { + for (std::size_t i_tc = 0, n_tc = s_testcase_v.size(); i_tc < n_tc; ++i_tc) { + try { + const testcase_gc & tc = s_testcase_v[i_tc]; + + up gc = GC::make( + {.initial_nursery_z_ = tc.nursery_z_, + .initial_tenured_z_ = tc.tenured_z_, + .incr_gc_threshold_ = tc.incr_gc_threshold_, + .full_gc_threshold_ = tc.full_gc_threshold_, + }); + + REQUIRE(gc.get()); + REQUIRE(gc->name() == "GC"); + + using ex_allocator = xo::gc::allocator; + using MyObjectInterface = gc_allocator_traits::template object_interface; + using NestedElementAllocator = xo::gc::allocator>; + using NestedType = MemberType; + using MyType = TestClass; + using MyAllocator = xo::gc::allocator; + + MyAllocator alloc(gc.get()); + + { + /* verify that MyType is constructible */ + MyType obj0; + + REQUIRE(obj0.member1_.ctor_ran_ == true); + } + + { + MyType * mem0 = MyType::make_0(alloc); + + REQUIRE(mem0 != nullptr); + REQUIRE(mem0->member1_.ctor_ran_ == false); + } + + { + MyType * mem1 = MyType::make_1(alloc); + + REQUIRE(mem1 != nullptr); + REQUIRE(mem1->member1_.ctor_ran_ == false); + } + + { + MyType * mem2 = MyType::make_2(alloc); + + REQUIRE(mem2 != nullptr); + REQUIRE(mem2->member1_.ctor_ran_ == true); + } + + { + MyType * mem3 = MyType::make_3(alloc); + + REQUIRE(mem3 != nullptr); + REQUIRE(mem3->member1_.ctor_ran_ == true); + } + + gp ptr; + { + REQUIRE(ptr.is_null()); + //ptr = MyType::make_0(); + } + } catch (std::exception & ex) { + std::cerr << "caught exception: " << ex.what() << std::endl; + REQUIRE(false); + } + } + + } + } /*namespace ut*/ } /*namespace xo*/ diff --git a/xo-alloc/utest/IAlloc.test.cpp b/xo-alloc/utest/IAlloc.test.cpp index 823791ab..7c1fdc56 100644 --- a/xo-alloc/utest/IAlloc.test.cpp +++ b/xo-alloc/utest/IAlloc.test.cpp @@ -3,26 +3,124 @@ * author: Roland Conybeare, Aug 2025 */ -#include "xo/allocutil/IAlloc.hpp" +//#include "xo/allocutil/IAlloc.hpp" +#include "xo/alloc/ArenaAlloc.hpp" +#include "xo/indentlog/print/tag.hpp" #include namespace xo { using xo::gc::IAlloc; + using xo::gc::ArenaAlloc; namespace ut { TEST_CASE("ialloc", "[alloc]") { + static_assert((sizeof(std::uintptr_t) == 8) && "possibly fine if this fails, but would want to know"); + REQUIRE(IAlloc::alloc_padding(0) == 0); - REQUIRE(IAlloc::alloc_padding(1) == 7); - REQUIRE(IAlloc::alloc_padding(2) == 6); - REQUIRE(IAlloc::alloc_padding(3) == 5); - REQUIRE(IAlloc::alloc_padding(4) == 4); - REQUIRE(IAlloc::alloc_padding(5) == 3); - REQUIRE(IAlloc::alloc_padding(6) == 2); - REQUIRE(IAlloc::alloc_padding(7) == 1); - REQUIRE(IAlloc::alloc_padding(8) == 0); - REQUIRE(IAlloc::alloc_padding(9) == 7); + + for (std::size_t i = 1; i < sizeof(std::uintptr_t); ++i) { + REQUIRE(IAlloc::alloc_padding(i) + i == IAlloc::c_alloc_alignment); + } + + REQUIRE(IAlloc::alloc_padding(IAlloc::c_alloc_alignment) == 0); + + for (std::size_t i = 1; i < sizeof(std::uintptr_t); ++i) { + REQUIRE(IAlloc::alloc_padding(IAlloc::c_alloc_alignment + i) + i == IAlloc::c_alloc_alignment); + } } + + /* although xo::gc::allocator is intended for + * IAlloc derivatives (so T is ArenaAlloc | GC), + * + * it only relies on allocate() and deallocate() methods + */ + + namespace { + struct TestCase { + explicit TestCase(size_t arena_z, size_t n, size_t n2) : arena_z_{arena_z}, n_{n}, n2_{n2} {} + + size_t arena_z_ = 0; + size_t n_ = 0; + size_t n2_ = 0; + }; + + std::vector s_testcase_v = { TestCase{1024*1024, 9, 13} }; + } + + TEST_CASE("gc.allocator", "[alloc]") + { + using xo::gc::allocator; + + constexpr bool c_debug_flag = false; + + for (size_t i_tc = 0, n_tc = s_testcase_v.size(); i_tc < n_tc; ++i_tc) { + INFO(xtag("i_tc", i_tc)); + + const TestCase & tc = s_testcase_v[i_tc]; + + up mm1 = ArenaAlloc::make("arena1", + tc.arena_z_, + c_debug_flag); + up mm2 = ArenaAlloc::make("arena2", + tc.arena_z_, + c_debug_flag); + + REQUIRE(mm1.get()); + REQUIRE(mm1->allocated() == 0); + + allocator alloc1(mm1.get()); + allocator alloc1a(mm1.get()); + + REQUIRE(mm2.get()); + REQUIRE(mm2->allocated() == 0); + + allocator alloc2(mm2.get()); + + SECTION("IAlloc identity determines allocator equality") { + REQUIRE(alloc1 == alloc1a); + REQUIRE(alloc1 != alloc2); + } + + int * p1 = nullptr; + size_t z1 = 0; + + SECTION("alloc space for ints") { + p1 = alloc1.allocate(tc.n_); + + REQUIRE(p1 != nullptr); + + // note: allowing for alignment + REQUIRE(mm1->allocated() >= sizeof(int32_t) * tc.n_); + REQUIRE(mm1->allocated() < sizeof(int32_t) * tc.n_ + IAlloc::c_alloc_alignment); + z1 = mm1->allocated(); + + // deallocate exists.. + alloc1.deallocate(p1, tc.n_); + + // ..but is a no-op + REQUIRE(mm1->allocated() == z1); + } + + int * p2 = nullptr; + + SECTION("allocator independence") { + REQUIRE(mm2->allocated() == 0); + + p2 = alloc2.allocate(tc.n2_); + + REQUIRE(p2 != nullptr); + REQUIRE(p1 != p2); + + REQUIRE(mm2->allocated() >= sizeof(int32_t) * tc.n2_); + REQUIRE(mm2->allocated() < sizeof(int32_t) * tc.n2_ + IAlloc::c_alloc_alignment); + + // mm1 unaffected by mm2 allocation + REQUIRE(mm1->allocated() == z1); + } + } + } + } /*namespace ut*/ } /*namespace xo*/ diff --git a/xo-allocutil/include/xo/allocutil/IAlloc.hpp b/xo-allocutil/include/xo/allocutil/IAlloc.hpp index a322e1db..b126d8a3 100644 --- a/xo-allocutil/include/xo/allocutil/IAlloc.hpp +++ b/xo-allocutil/include/xo/allocutil/IAlloc.hpp @@ -46,9 +46,11 @@ namespace xo { public: virtual ~IAlloc() {} + /** word size for alignment **/ + static constexpr uint32_t c_alloc_alignment = sizeof(std::uintptr_t); + static inline std::uint32_t alloc_padding(std::size_t z) { - /* word size for alignment */ - constexpr uint32_t c_bpw = sizeof(std::uintptr_t); + constexpr uint32_t c_bpw = c_alloc_alignment; /* round up to multiple of c_bpw, but map 0 -> 0 * (table assuming c_bpw==8) diff --git a/xo-allocutil/include/xo/allocutil/ObjectVisitor.hpp b/xo-allocutil/include/xo/allocutil/ObjectVisitor.hpp new file mode 100644 index 00000000..1719ef70 --- /dev/null +++ b/xo-allocutil/include/xo/allocutil/ObjectVisitor.hpp @@ -0,0 +1,53 @@ +/** @file ObjectVisitor.hpp + * + **/ + +#include "IAlloc.hpp" +#include + +namespace xo { + namespace gc { + /** @class ObjectVisitor + * @brief visit IObject* members of a T instance + * + * Garbage collector relies on being able to navigate to + * an IObject-instance to find+update embedded pointers to + * other IObjects. + * + * An IObject implemnetation must override + * IObject::_forward_children(IAlloc * gc) + * and call + * gc->forward_inplace(&child_) + * for each such child pointer. + * + * For non-template classes this is straightforward + * See for example + * xo::obj::List in object/List.hpp + * + * For a template class Foo that contains T-instances, + * need to handle case where T contains IObject pointers. + * See for example the + * xo::tree::RedBlackTree in ordinaltree/RedBlackTree.hpp + * + * Use ObjectVisitor::forward_children() to pick up + * navigation code for such template arguments. + **/ + template + class ObjectVisitor { + //void forward_children(T & target, IAlloc * gc) { (void)target; (void)gc; } + }; + +#define XO_TRIVIAL_OBJECT_VISITOR(TYPE) \ + template <> \ + class ObjectVisitor { \ + public: \ + static void forward_children(TYPE &, IAlloc *) {} \ + } + + XO_TRIVIAL_OBJECT_VISITOR(int32_t); + XO_TRIVIAL_OBJECT_VISITOR(double); + + } /*namespace gc*/ +} /*namespace xo*/ + +/* ObjectVisitor.hpp */ diff --git a/xo-ordinaltree/include/xo/ordinaltree/rbtree/Node.hpp b/xo-ordinaltree/include/xo/ordinaltree/rbtree/Node.hpp index 453ae5d3..6bb719eb 100644 --- a/xo-ordinaltree/include/xo/ordinaltree/rbtree/Node.hpp +++ b/xo-ordinaltree/include/xo/ordinaltree/rbtree/Node.hpp @@ -10,6 +10,7 @@ #include "xo/indentlog/scope.hpp" #include "xo/allocutil/IAlloc.hpp" #include "xo/allocutil/IObject.hpp" +#include "xo/allocutil/ObjectVisitor.hpp" #include "xo/allocutil/gc_allocator_traits.hpp" #include #include @@ -344,7 +345,7 @@ namespace xo { } } /*replace_child_reparent*/ - // ----- (possibly) inherited from GcObjectInterface ----- + // ----- inherited from GcObjectInterface ----- virtual TaggedPtr self_tp() const { return Reflect::make_tp(const_cast(this)); @@ -367,6 +368,8 @@ namespace xo { virtual std::size_t _forward_children(gc::IAlloc * gc) { if constexpr (GcObjectInterface::_requires_gc_hooks) { + using xo::gc::ObjectVisitor; + static_assert(std::is_convertible_v, "parent_ must be convertible to IObject*"); static_assert(std::is_convertible_v, @@ -376,9 +379,18 @@ namespace xo { gc->forward_inplace(reinterpret_cast(&child_v_[0])); gc->forward_inplace(reinterpret_cast(&child_v_[1])); - // how do we tell if any of - // {value_type, ReducedValue} - // also contain pointers that need forwarding? + /* must cast away const so we can forward */ + Key & key = const_cast(contents_.first); + ObjectVisitor::forward_children(key, gc); + + Value & value = contents_.second; + ObjectVisitor::forward_children(value, gc); + + ReducedValue & rv1 = reduced_.first; + ObjectVisitor::forward_children(rv1, gc); + + ReducedValue & rv2 = reduced_.second; + ObjectVisitor::forward_children(rv2, gc); return Node::_shallow_size(); } else { @@ -397,9 +409,8 @@ namespace xo { Color color_ = C_Red; /* size of subtree (#of key/value pairs) rooted at this node */ size_t size_ = 0; - /* .first = key associated with this node + /* .first = key associated with this node (const!) * .second = value associated with this node - * .third = reduced value */ value_type contents_; /* accumulator for some binary function of Values. @@ -432,7 +443,7 @@ namespace xo { */ std::array child_v_ = {nullptr, nullptr}; }; /*Node*/ - } /*namespace detail*/ + } /*namespace detail*/ } /*namespace tree*/ } /*namespace xo*/ diff --git a/xo-ordinaltree/utest/RedBlackTree-gc.test.cpp b/xo-ordinaltree/utest/RedBlackTree-gc.test.cpp index d0d8d587..aab694b6 100644 --- a/xo-ordinaltree/utest/RedBlackTree-gc.test.cpp +++ b/xo-ordinaltree/utest/RedBlackTree-gc.test.cpp @@ -74,18 +74,23 @@ namespace xo { } ); - xo::gc::allocator allocator(gc.get()); + xo::gc::allocator allocator(gc.get()); - /* verify that we can construct a tree node */ - RbTree::RbNode node; + /* 1. verify that tree node can be constructed. + * if it can't be constructed, the immediately-following construct + * will fail in a non-transparent way. + */ + RbTree::RbNode test_node; #ifdef NOT_YET - RedBlackTree::node_allocator_traits::construct(allocator, &node, - {0, 0.0}, - {0.0, 0.0}); -#endif + /* 2. verify that tree node can be constructed via + * allocator traits + */ + RbTree::node_allocator_traits::construct(allocator, + &test_node, + {0, 0.0}, + {0.0, 0.0}); -#ifdef NOT_YET RbTree rbtree(allocator, c_debug_flag); /* insert [0..n-1] in random order **/