diff --git a/doc/developer-guide/internal-libraries/Extendible.en.rst b/doc/developer-guide/internal-libraries/Extendible.en.rst index 26e03923657..8bb595e16bb 100644 --- a/doc/developer-guide/internal-libraries/Extendible.en.rst +++ b/doc/developer-guide/internal-libraries/Extendible.en.rst @@ -80,18 +80,19 @@ the type's constructor, destructor, and serializer. And to avoid corruption, the } -When an derived class is instantiated, :func:`template<> alloc()` will allocate a block of memory for the derived class and all added -fields. The only memory overhead per instance is an uint16 used as a offset to the start of the extendible block. +When an derived class is instantiated, :func:`template<> create()` will allocate a block of memory for the derived class and all added +fields. The only memory overhead per instance is an uint16 used as a offset to the start of the extendible block. Then the constructor of the class +is called, followed by the constructors of each extendible field. .. code-block:: cpp ExtendibleExample* alloc_example() { - return ext::alloc(); + return ext::create(); } Memory Layout ------------- -One block of memory is allocated per |Extendible|, which included all member variables and extended fields. +One block of memory is allocated per |Extendible|, which include all member variables and extended fields. Within the block, memory is arranged in the following order: #. Derived members (+padding align next field) @@ -129,8 +130,8 @@ which simplifies the code using it. Also this provides compile errors for common } PluginFunc() { - Food *banana = ext::alloc(); - Car *camry = ext::alloc(); + Food *banana = ext::create(); + Car *camry = ext::create(); // Common user errors @@ -140,11 +141,11 @@ which simplifies the code using it. Also this provides compile errors for common float speed = ext::get(banana,fld_max_speed); // ^^^^^^^^^^^^^ - // Compile error: Cannot downcast banana to type Extendible + // Compile error: Cannot convert banana to type Extendible float weight = ext::get(camry,fld_food_weight); // ^^^^^^^^^^^^^^^ - // Compile error: Cannot downcast camry to type Extendible, even though Car and Food each have a 'weight' field, the FieldId is strongly typed. + // Compile error: Cannot convert camry to type Extendible, even though Car and Food each have a 'weight' field, the FieldId is strongly typed. } @@ -157,20 +158,20 @@ which simplifies the code using it. Also this provides compile errors for common Fruit.schema.addField(fld_has_seeds, "has_seeds"); - Fruit mango = ext::alloc(); + Fruit mango = ext::create(); - ext::set(mango, fld_has_seeds) = true; // downcasts mango to Extendible - ext::set(mango, fld_food_weight) = 2; // downcasts mango to Extendible + ext::set(mango, fld_has_seeds) = true; // converts mango to Extendible + ext::set(mango, fld_food_weight) = 2; // converts mango to Extendible ext::set(mango, fld_max_speed) = 9; // ^^^^^^^^^^^^^ - // Compile error: Cannot downcast mango to type Extendible + // Compile error: Cannot convert mango to type Extendible Inheritance ----------- Unfortunately it is non-trivial handle multiple |Extendible| super types in the same inheritance tree. - :func:`template<> alloc()` handles allocation and initialization of the entire `Derived` class, but it is dependent on each class defining :code:`using super_type = *some_super_class*;` so that it recurse through the classes. + :func:`template<> create()` handles allocation and initialization of the entire `Derived` class, but it is dependent on each class defining :code:`using super_type = *some_super_class*;` so that it recurse through the classes. .. code-block:: cpp @@ -191,7 +192,7 @@ Inheritance ext::FieldId> ext_a_1; ext::FieldId ext_c_1; - C &x = *(ext::alloc()); + C &x = *(ext::create()); ext::viewFormat(x); :func:`viewFormat` prints a diagram of the position and size of bytes used within the allocated @@ -235,8 +236,9 @@ Namespace `ext` one schema instance per |Extendible| to define contained |FieldDesc| -.. function:: template Extendible* alloc() +.. function:: template Extendible* create() + To be used in place of `new Derived_t()`. Allocate a block of memory. Construct the base data. Recursively construct and initialize `Derived_t::super_type` and its |Extendible| classes. diff --git a/include/tscore/Extendible.h b/include/tscore/Extendible.h index 16630edcdeb..2ac826ffeb5 100644 --- a/include/tscore/Extendible.h +++ b/include/tscore/Extendible.h @@ -50,6 +50,13 @@ #include "tscore/ink_memory.h" #include "tscore/ink_defs.h" +////////////////////////////////////////// +/// SUPPORT MACRO +#define DEF_EXT_NEW_DEL(cls) \ + void *operator new(size_t sz) { return ats_malloc(ext::sizeOf()); } \ + void *operator new(size_t sz, void *ptr) { return ptr; } \ + void operator delete(void *ptr) { free(ptr); } + ////////////////////////////////////////// /// HELPER CLASSES @@ -141,9 +148,11 @@ namespace details // internal stuff { public: std::unordered_map fields; ///< defined elements of the blob by name - size_t alloc_size = 0; ///< bytes to allocate for fields - uint8_t alloc_align = 1; ///< alignment of block - std::atomic_uint instance_count = {0}; ///< the number of Extendible instances in use. + size_t alloc_size = 0; ///< bytes to allocate for fields + uint8_t alloc_align = 1; ///< alignment of block + std::atomic cnt_constructed = {0}; ///< the number of Extendible created. + std::atomic cnt_fld_constructed = {0}; ///< the number of Extendible that constructed fields. + std::atomic cnt_destructed = {0}; ///< the number of Extendible destroyed. public: Schema() {} @@ -163,7 +172,7 @@ namespace details // internal stuff /// ext::Extendible allows code (and Plugins) to declare member-like variables during system init. /* - * This class uses a special allocator (ext::alloc) to extend the memory allocated to store run-time static + * This class uses a special allocator (ext::create) to extend the memory allocated to store run-time static * variables, which are registered by plugins during system init. The API is in a functional style to support * multiple inheritance of Extendible classes. This is templated so static variables are instanced per Derived * type, because we need to have different field schema per type. @@ -199,10 +208,10 @@ template class Extendible protected: Extendible(); - // use ext::alloc() exclusively for allocation and initialization + // use ext::create() exclusively for allocation and initialization /** destruct all fields */ - ~Extendible() { schema.callDestructor(uintptr_t(this) + ext_loc); } + ~Extendible(); private: /** construct all fields */ @@ -214,7 +223,7 @@ template class Extendible return uintptr_t(this) + ext_loc; } - template friend T *alloc(); + template friend T *create(); template friend uintptr_t details::initRecurseSuper(T &, uintptr_t); template friend FieldPtr details::FieldPtrGet(Extendible const &, details::FieldDesc const &); template friend std::string viewFormat(T const &, uintptr_t, int); @@ -575,6 +584,17 @@ sizeOf(size_t size = sizeof(Derived_t)) template Extendible::Extendible() { ink_assert(ext::details::areFieldsFinalized()); + // don't call callConstructor until the derived class is fully constructed. + ++schema.cnt_constructed; +} + +template Extendible::~Extendible() +{ + // assert callConstructors was called. + ink_assert(ext_loc); + schema.callDestructor(uintptr_t(this) + ext_loc); + ++schema.cnt_destructed; + ink_assert(schema.cnt_destructed <= schema.cnt_fld_constructed); } /// tell this extendible where it's memory offset start is. Added to support inheriting from extendible classes @@ -584,8 +604,9 @@ Extendible::initFields(uintptr_t start_ptr) { ink_assert(ext_loc == 0); start_ptr = ROUNDUP(start_ptr, schema.alloc_align); // pad the previous struct, so that our fields are memaligned correctly - ext_loc = uint16_t(start_ptr - uintptr_t(this)); // store the offset to be used by ext::get and ext::set - ink_assert(ext_loc < 256); + ink_assert(start_ptr - uintptr_t(this) < UINT16_MAX); + ext_loc = uint16_t(start_ptr - uintptr_t(this)); // store the offset to be used by ext::get and ext::set + ink_assert(ext_loc > 0); schema.callConstructor(start_ptr); // construct all fields return start_ptr + schema.alloc_size; // return the end of the extendible data } @@ -608,23 +629,26 @@ namespace details } return tail_ptr; } + } // namespace details // allocate and initialize an extendible data structure -template +template Derived_t * -alloc() +create(Args &&... args) { - static_assert(std::is_base_of, Derived_t>::value); + // don't instantiate until all Fields are finalized. ink_assert(ext::details::areFieldsFinalized()); - // calculate the memory needed + // calculate the memory needed for the class and all Extendible blocks const size_t type_size = ext::sizeOf(); - // alloc a block of memory - Derived_t *ptr = (Derived_t *)ats_memalign(alignof(Derived_t), type_size); - // Extendible_t *ptr = (Extendible_t *)::operator new(type_size); // alloc a block of memory + + // alloc one block of memory + Derived_t *ptr = static_cast(ats_memalign(alignof(Derived_t), type_size)); + // construct (recursively super-to-sub class) - new (ptr) Derived_t(); + new (ptr) Derived_t(std::forward(args)...); + // define extendible blocks start offsets (recursively super-to-sub class) details::initRecurseSuper(*ptr, uintptr_t(ptr) + sizeof(Derived_t)); return ptr; diff --git a/src/tscore/Extendible.cc b/src/tscore/Extendible.cc index 2ca6237223b..7d39f8d968b 100644 --- a/src/tscore/Extendible.cc +++ b/src/tscore/Extendible.cc @@ -44,7 +44,7 @@ namespace details void Schema::updateMemOffsets() { - ink_release_assert(instance_count == 0); + ink_release_assert(cnt_constructed == cnt_destructed); uint32_t acc_offset = 0; alloc_align = 1; @@ -86,7 +86,7 @@ namespace details bool Schema::reset() { - if (instance_count > 0) { + if (cnt_constructed > cnt_destructed) { // free instances before calling this so we don't leak memory return false; } @@ -99,7 +99,9 @@ namespace details Schema::callConstructor(uintptr_t ext_loc) { ink_assert(ext_loc); - ++instance_count; // don't allow schema modification + ++cnt_fld_constructed; // don't allow schema modification + ink_assert(cnt_fld_constructed <= cnt_constructed); + // init all extendible memory to 0, incase constructors don't memset(reinterpret_cast(ext_loc), 0, alloc_size); @@ -119,7 +121,6 @@ namespace details elm.second.destructor(FieldPtr(ext_loc + elm.second.field_offset)); } } - --instance_count; } size_t @@ -132,7 +133,7 @@ namespace details bool Schema::no_instances() const { - return instance_count == 0; + return cnt_constructed == cnt_destructed; } } // namespace details diff --git a/src/tscore/unit_tests/test_Extendible.cc b/src/tscore/unit_tests/test_Extendible.cc index 23691844b24..da092971550 100644 --- a/src/tscore/unit_tests/test_Extendible.cc +++ b/src/tscore/unit_tests/test_Extendible.cc @@ -65,6 +65,8 @@ TEST_CASE("AtomicBit Atomic test") // Extendible Inheritance Tests struct A : public Extendible { + using self_type = A; + DEF_EXT_NEW_DEL(self_type); uint16_t a = {1}; }; @@ -74,14 +76,18 @@ class B : public A { public: using super_type = A; - uint16_t b = {2}; + using self_type = B; + DEF_EXT_NEW_DEL(self_type); + uint16_t b = {2}; }; class C : public B, public Extendible { public: using super_type = B; - uint16_t c = {3}; + using self_type = C; + DEF_EXT_NEW_DEL(self_type); + uint16_t c = {3}; // operator[] template decltype(auto) operator[](F field) const { return ext::get(*this, field); } @@ -95,6 +101,46 @@ memDelta(void *p, void *q) { return uintptr_t(q) - uintptr_t(p); } +A *a_ptr = nullptr; +TEST_CASE("Create A", "") +{ + ext::details::areFieldsFinalized() = true; + a_ptr = ext::create(); + CHECK(Extendible::schema.no_instances() == false); +} +TEST_CASE("Delete A", "") +{ + delete a_ptr; + CHECK(Extendible::schema.no_instances()); +} +TEST_CASE("Create B", "") +{ + a_ptr = ext::create(); + CHECK(Extendible::schema.no_instances() == false); +} +TEST_CASE("Delete B", "") +{ + delete a_ptr; + CHECK(Extendible::schema.no_instances()); +} +TEST_CASE("Create C", "") +{ + a_ptr = ext::create(); + CHECK(Extendible::schema.no_instances() == false); + CHECK(Extendible::schema.no_instances() == false); +} +TEST_CASE("Delete C", "") +{ + delete static_cast(a_ptr); + CHECK(Extendible::schema.no_instances()); + CHECK(Extendible::schema.no_instances()); + CHECK(Extendible::schema.cnt_constructed == 3); + CHECK(Extendible::schema.cnt_fld_constructed == 3); + CHECK(Extendible::schema.cnt_destructed == 3); + CHECK(Extendible::schema.cnt_constructed == 1); + CHECK(Extendible::schema.cnt_fld_constructed == 1); + CHECK(Extendible::schema.cnt_destructed == 1); +} TEST_CASE("Extendible Memory Allocations", "") { ext::details::areFieldsFinalized() = false; @@ -107,7 +153,7 @@ TEST_CASE("Extendible Memory Allocations", "") CHECK(ext::sizeOf() == w * 4); CHECK(ext::sizeOf() == w * 7); - C &x = *(ext::alloc()); + C &x = *(ext::create()); // 0 1 2 3 4 5 6 //[ EA*, a, b,EC*, c, EA, EC] // @@ -136,7 +182,7 @@ TEST_CASE("Extendible Memory Allocations", "") TEST_CASE("Extendible Pointer Math", "") { - C &x = *(ext::alloc()); + C &x = *(ext::create()); CHECK(x.a == 1); CHECK(x.b == 2); @@ -169,6 +215,8 @@ TEST_CASE("Extendible Pointer Math", "") // Extendible is abstract and must be derived in a CRTP struct Derived : Extendible { + using self_type = Derived; + DEF_EXT_NEW_DEL(self_type); string m_str; // operator[] for shorthand @@ -193,7 +241,7 @@ struct Derived : Extendible { void * DerivedExtalloc() { - return ext::alloc(); + return ext::create(); } void DerivedExtFree(void *ptr) @@ -265,7 +313,7 @@ TEST_CASE("Extendible", "") // I don't use SECTIONS because this modifies static variables many times, is not thread safe. INFO("Extendible()") { - ptr = ext::alloc(); + ptr = ext::create(); REQUIRE(ptr != nullptr); } @@ -277,7 +325,7 @@ TEST_CASE("Extendible", "") INFO("Schema Reset") { - ptr = ext::alloc(); + ptr = ext::create(); REQUIRE(Derived::schema.no_instances() == false); REQUIRE(Derived::schema.reset() == false); delete ptr; @@ -289,7 +337,7 @@ TEST_CASE("Extendible", "") INFO("shared_ptr") { - shared_ptr sptr(ext::alloc()); + shared_ptr sptr(ext::create()); REQUIRE(Derived::schema.no_instances() == false); REQUIRE(sptr); } @@ -306,7 +354,7 @@ TEST_CASE("Extendible", "") INFO("Extendible delete ptr"); { for (int i = 0; i < 10; i++) { - ptr = ext::alloc(); + ptr = ext::create(); REQUIRE(ptr != nullptr); INFO(__LINE__); REQUIRE(Derived::schema.no_instances() == false); @@ -319,7 +367,7 @@ TEST_CASE("Extendible", "") INFO("test bit field"); { - shared_ptr sptr{ext::alloc()}; + shared_ptr sptr{ext::create()}; Derived &ref = *sptr; CHECK(ext::viewFormat(ref) == Derived::testFormat()); @@ -356,7 +404,7 @@ TEST_CASE("Extendible", "") CHECK(ext::sizeOf() == expected_size); ext::details::areFieldsFinalized() = true; - shared_ptr sptr(ext::alloc()); + shared_ptr sptr(ext::create()); Derived &ref = *sptr; CHECK(ext::viewFormat(ref) == Derived::testFormat()); using Catch::Matchers::Contains; @@ -385,7 +433,7 @@ TEST_CASE("Extendible", "") size_t expected_size = sizeof(Derived) + 1 + sizeof(std::atomic_int) * 2; CHECK(ext::sizeOf() == expected_size); - shared_ptr sptr(ext::alloc()); + shared_ptr sptr(ext::create()); Derived &ref = *sptr; CHECK(ext::get(ref, int_a) == 0); CHECK(ext::get(ref, int_b) == 0);