From 3cfc0d9c71c54c5c266d2451bcc793d9186d1c5c Mon Sep 17 00:00:00 2001 From: Keijo Mottonen Date: Thu, 9 Feb 2023 22:23:31 +0200 Subject: [PATCH] Improve type safety and remove the need for explicit destructor calls --- README.md | 24 ++-- .../fsm-example-morse/fsm-example-morse.cc | 2 - examples/fsm-example-morse/makefile | 9 +- examples/fsm-example-rgb/fsm-blue.cc | 1 - examples/fsm-example-rgb/fsm-green.cc | 1 - examples/fsm-example-rgb/fsm-red.cc | 1 - include/CoFSM.h | 125 +++++++++++------- 7 files changed, 101 insertions(+), 62 deletions(-) diff --git a/README.md b/README.md index 16681c5..62817b9 100644 --- a/README.md +++ b/README.md @@ -429,10 +429,10 @@ Now pass the event around the ring `numRoundsToRepeat` times both clockwise and ``` **Output:** ``` -Based on 10000 rounds around the ring of 1023 states in 0.868624 secs, meaning 10240000 events sent, -the speed of FSM's execution is 1.17888e+07 state transitions per second. +Based on 10000 rounds around the ring of 1023 states in 0.86698 secs, meaning 10240000 events sent, +the speed of FSM's execution is 1.18111e+07 state transitions per second ``` -It was pretty fast, actually. An ancient Core i5-4210U running at 2.7 GHz did about 12 million state transitions per second. This means that a single resume-run-suspend cycle took 229 clock cycles on average. +It is quite fast, actually. An ancient Core i5-4210U running at 2.7 GHz did about 12 million state transitions per second. This means that a single resume-run-suspend cycle took 229 clock cycles on average. Runnable code and makefile can be found in folder [fsm-example-ring](examples/fsm-example-ring) @@ -509,7 +509,6 @@ When an event arrives, the name of the event indicates the type of the associate In this case, the type of the object is `std::stop_token`. The object is accessed through a pointer (`pStop` in this case) which is acquired from the Event with `event >> pStop`. -If the object in the storage is trivially destructible (like an int or double), the object will simply wink out of existance without explicit destruction. However, because `std::stop_token` is not trivially destructible, we must call explicit destruction `event.destroy(pStop)`. Now the Event object is empty and can be reused for sending the next event. In this case, the new name of the event will be `StartBlinkEvent` and the data will be integer `iBlinkTimeMs`. This is done by calling `event.construct("StartBlinkEvent", iBlinkTimeMs)`. @@ -525,7 +524,6 @@ CoFSM::State IdleState(FSM& fsm) { event >> pStop; // Get a pointer to the event's data (which is a stop token). stopToken = std::move(*pStop); // Move to a local variable to be used later - event.destroy(pStop); // Explicit destruction needed because stop_token is not trivially destructible. event.construct("StartBlinkEvent", iBlinkTimeMs); // iBlinkTimeMs piggybacks on "StartBlinkEvent" } else if (...) @@ -542,7 +540,6 @@ The methods are as follows. Sets the name of the event as `name` and constructs an object of type `TT = std::decay_t` by calling contructor `TT{std::forward(t)}`. The object is emplaced in the storage of the event. If the current capacity of the storage space is too small, it will be expanded. This operation is somewhat similar to [push_back](https://en.cppreference.com/w/cpp/container/vector/push_back) method of `std::vector`.
Returns pointer to the constructed object (even though the return value is usually ignored).
- **Note:** If the data storage of the event is currently holding an object with non-trivial destructor, `destroy()` must be called before the next `construct()` may take place. Otherwise `std::runtime_error` will be thrown. For example: ```c++ std::vector v {1,2,3,4}; event.construct("VectorEvent", std::move(v)); @@ -550,7 +547,6 @@ The methods are as follows. std::vector* pV; event >> pV; // ... use *pV ... - event.destroy(pV); // Vector as non-trivial destructor, remember? ``` - `template `
`T* construct(std::string_view name, Args&&... args)`
@@ -561,7 +557,11 @@ The methods are as follows. Note that this does not deallocate the data storage of the event as it will be reused. After destroy(), the event becomes empty in the sense that is has neither name nor valid data. - `template T& operator>>(T*& p)`
- `T* pT; event >> pT;` gets pointer to the object of type T living in the storage space of the event. See the examples above. + `T* pT; event >> pT;` gets pointer to the object of type T living in the storage space of the event. See the examples above.
+ This is a type safe way to get access to the object in the storage. + If the type of the object is `T`, + then the type of the pointer on the right side of `>>` must be `T*`. + If it is not, `std::runtime_exception` will be thrown. - `void reserve(std::size_t size)` Ensures that the capacity of the storage space is at least `size` bytes. - `std::size_t capacity()` returns the cacpcity of the storage space in bytes. - `void clear()` Sets the capacity of the storage space to zero and deallocates the buffer. This operation may be needed if a single event uses a massive amount of memory, which is an overkill for the other events which will later be places in the same storage. But normally this is not needed. @@ -570,14 +570,16 @@ Also, the event becomes empty in the sense that is has neither name nor valid da - `std::string_view name()` Returns the name of the event as a string_view - `std::string nameAsString()` Returns a copy of the name of the event as a heap-allocated string. - `bool isEmpty()` Returns true if the event is empty (i.e. it has no name). If a state sends an empty event, the FSM will be suspended. +- `bool hasData()` Returns true if there is an object stored in the storage. - `void* data()` Returns pointer to the beginning of the data storage. Is a `nullptr` if `capacity() = 0`. - `template T* dataAs()` Returns pointer to the beginning of the data storage cast as `T*`. These two methods for getting a pointer to the event's data are identical: ```c++ - T* p1; // Method 1 - event >> p1; - auto p2 = event.dataAs(); // Method 2 + T* p1; + event >> p1; // Method 1 (safe) + auto p2 = event.dataAs(); // Method 2 (may be unsafe) assert(p1 == p2); ``` +However, `dataAs()` does not enforce the type of the destination pointer like operator `>>` does, so the latter should be preferred. ### CoFSM::State `State`is the return type of every state coroutine. Like every [coroutine](https://en.cppreference.com/w/cpp/language/coroutines), it is associated with a `handle` and a `promise`. Generally you don't need to worry about them or explicitly call any methods of `State` class. Some methods are given below anyway in case you want to experiment with coroutines. diff --git a/examples/fsm-example-morse/fsm-example-morse.cc b/examples/fsm-example-morse/fsm-example-morse.cc index 585496d..b6a9a03 100644 --- a/examples/fsm-example-morse/fsm-example-morse.cc +++ b/examples/fsm-example-morse/fsm-example-morse.cc @@ -162,8 +162,6 @@ CoFSM::State transmitReadyState(CoFSM::FSM& fsm) // Take the string from the event data and store. event >> pString; message = std::move(*pString); - // String is not trivially destructible so destroy explicitly from the event data. - event.destroy(pString); symbolsSent = 0; } else if (event == "TransmissionReadyEvent") { diff --git a/examples/fsm-example-morse/makefile b/examples/fsm-example-morse/makefile index 6866cca..7db0ac5 100644 --- a/examples/fsm-example-morse/makefile +++ b/examples/fsm-example-morse/makefile @@ -4,17 +4,24 @@ CC = g++ INCLUDE_DIR = ../../include # Compiler flag -CPPFLAGS = -O2 --pedantic-errors --std=c++20 -Wall -Wextra -I$(INCLUDE_DIR) +CPP_COMMON_FLAGS = --pedantic-errors --std=c++20 -Wall -Wextra -I$(INCLUDE_DIR) +CPP_DEBUG_FLAGS = -g -fsanitize=address +CPP_OPTIMIZATION_FLAGS = -O2 # The build target (i.e. the name of the executable) TARGET = fsm-example-morse +all: CPPFLAGS = $(CPP_OPTIMIZATION_FLAGS) $(CPP_COMMON_FLAGS) all: $(TARGET) # Use laptop's keyboard LEDs for demonstration. The binary must be run with "sudo ./fsm-example-morse" linux: EXTRAFLAGS = -DLINUX +linux: CPPFLAGS = $(CPP_OPTIMIZATION_FLAGS) $(CPP_COMMON_FLAGS) linux: $(TARGET) +debug: CPPFLAGS = $(CPP_DEBUG_FLAGS) $(CPP_COMMON_FLAGS) +debug: $(TARGET) + clean: rm -f *.o $(TARGET) diff --git a/examples/fsm-example-rgb/fsm-blue.cc b/examples/fsm-example-rgb/fsm-blue.cc index cacc36e..24f71a1 100644 --- a/examples/fsm-example-rgb/fsm-blue.cc +++ b/examples/fsm-example-rgb/fsm-blue.cc @@ -117,7 +117,6 @@ static CoFSM::State blueIdleState(FSM& fsm) { event >> pStop; // Stop token is in the payload of the handover event. stopToken = std::move(*pStop); - event.destroy(pStop); // Explicit destruction needed because stop_token is not trivially destructible. iBlinksLeft = iNumberOfBlinks; // Do this many blinks before handing over to another FSM event.construct("StartBlinkEvent", iBlinkTimeMs); // iBlinkTimeMs piggybacks on "StartBlinkEvent" } diff --git a/examples/fsm-example-rgb/fsm-green.cc b/examples/fsm-example-rgb/fsm-green.cc index 07fec91..d646a33 100644 --- a/examples/fsm-example-rgb/fsm-green.cc +++ b/examples/fsm-example-rgb/fsm-green.cc @@ -117,7 +117,6 @@ static CoFSM::State greenIdleState(FSM& fsm) { event >> pStop; // Stop token is in the payload of the handover event. stopToken = std::move(*pStop); - event.destroy(pStop); // Explicit destruction needed because stop_token is not trivially destructible. iBlinksLeft = iNumberOfBlinks; // Do this many blinks before handing over to another FSM event.construct("StartBlinkEvent", iBlinkTimeMs); // iBlinkTimeMs piggybacks on "StartBlinkEvent" } diff --git a/examples/fsm-example-rgb/fsm-red.cc b/examples/fsm-example-rgb/fsm-red.cc index 1ccee7e..76cb73b 100644 --- a/examples/fsm-example-rgb/fsm-red.cc +++ b/examples/fsm-example-rgb/fsm-red.cc @@ -117,7 +117,6 @@ static CoFSM::State redIdleState(FSM& fsm) { event >> pStop; // Stop token is in the payload of the handover event. stopToken = std::move(*pStop); - event.destroy(pStop); // Explicit destruction needed because stop_token is not trivially destructible. iBlinksLeft = iNumberOfBlinks; // Do this many blinks before handing over to another FSM event.construct("StartBlinkEvent", iBlinkTimeMs); // iBlinkTimeMs piggybacks on "StartBlinkEvent" } diff --git a/include/CoFSM.h b/include/CoFSM.h index c40c5aa..8ae5715 100644 --- a/include/CoFSM.h +++ b/include/CoFSM.h @@ -17,6 +17,7 @@ #include #include #include +#include namespace CoFSM { @@ -29,6 +30,9 @@ constexpr std::size_t hardware_constructive_interference_size = 64; static const std::string _sharedEmptyString{}; +template +concept Trivial = (std::is_trivially_destructible_v || std::is_same_v); + // Generic reusable Event class. // An object of this type hold its identity in a string_view // and data in a byte buffer. Hence an event object can be reused @@ -43,7 +47,7 @@ struct Event { _name = std::exchange(other._name, ""); _capacity = std::exchange(other._capacity, 0u); _data = std::exchange(other._data, nullptr); - _hasNontrivialDestructor = std::exchange(other._hasNontrivialDestructor, false); + _anyPtr = std::exchange(other._anyPtr, nullptr); } Event& operator=(Event&& other) noexcept @@ -53,14 +57,16 @@ struct Event { _name = std::exchange(other._name, ""); _capacity = std::exchange(other._capacity, 0u); _data = std::exchange(other._data, nullptr); - _hasNontrivialDestructor = std::exchange(other._hasNontrivialDestructor, false); + _anyPtr = std::exchange(other._anyPtr, nullptr); } return *this; } ~Event() { - assertDestruct(); + // If *_anyPtr contains an AnyPtr object which points to the buffer, + // the object living in the buffer will be destroyed at the destructor of AnyPtr + _anyPtr.reset(); delete [] _data; } @@ -68,19 +74,23 @@ struct Event { template T* construct(std::string_view name, Args&&... args) { - assertDestruct(); static_assert(!(std::is_same_v && sizeof...(Args) > 0), "Void event must not take constructor arguments."); + if (_anyPtr && _anyPtr->has_value()) + *_anyPtr = std::any(); // Destroy the object currently living in the buffer by implicitly invoking AnyPtr destructor. if constexpr (std::is_same_v) { this->_name = name; - this->_hasNontrivialDestructor = false; - return this->data(); + void* p = this->data(); + return p; } else { this->reserve(sizeof(T)); ::new (this->_data) T{std::forward(args)...}; this->_name = name; - this->_hasNontrivialDestructor = !std::is_trivially_destructible_v; - return this->dataAs(); + T* p = this->dataAs(); + // Store typed pointer into a type erased std::any object. + // Note that dynamic memory will not be allocated for AnyPtr object due to Small Buffer Optimization. + _anyPtr->emplace>(p); + return p; } } @@ -89,33 +99,25 @@ struct Event { std::decay_t* construct(std::string_view name, T&& t) { using TT = std::decay_t; - assertDestruct(); + if (_anyPtr && _anyPtr->has_value()) + *_anyPtr = std::any(); // Destroy the object currently living in the buffer by implicitly invoking AnyPtr destructor. this->reserve(sizeof(TT)); ::new (this->_data) TT{std::forward(t)}; this->_name = name; - this->_hasNontrivialDestructor = !std::is_trivially_destructible_v; - return this->dataAs(); + TT* p = this->dataAs(); + _anyPtr->emplace>(p); + return p; } // Destroys the object pointed by _data unless the type T is // void or T is trivially destructible. // After this call, the event will be empty. - // Note: If the data buffer holds a non-trivially destructible object, - // you must call this function before the life-time - // of the Event object ends. Otherwise the object - // stored in the data buffer is winked out of existance - // without proper destruction. template void destroy(T* = nullptr) { - if constexpr (std::is_same_v || std::is_trivially_destructible_v) - assertDestruct(); // We did not call destructor. Check if we should have. - else - if (_data && this->_hasNontrivialDestructor) - this->dataAs()->~T(); - + if (_anyPtr && _anyPtr->has_value()) + *_anyPtr = std::any(); // Destroy the object currently living in the buffer by implicitly invoking AnyPtr destructor. this->_name = ""; - this->_hasNontrivialDestructor = false; } // Reinterprets the data buffer as an object of type T. @@ -134,10 +136,12 @@ struct Event { // Allows you to get a pointer to the payload of type T using syntax "event >> p" where T* p; // Returns reference to the payload object so you can also use the result directly without // dereferening p. For example: "auto x = (event >> p) + 1;" means "event >> p; auto x = *p + 1;" + // If the type of the object stored in the buffer is not T, an exception will be thrown. + // So you can not accidentally read the data in a wrong format. template T& operator>>(T*& p) { - p = this->dataAs(); + p = this->safeCast(); return *p; } @@ -155,24 +159,28 @@ struct Event { // Releases the data allocated from the heap and empties the name. void clear() { - assertDestruct(); + if (!_data) + return; + + _anyPtr.reset(); // Destroy the object in the buffer, if any. _name = ""; _capacity = 0; delete [] _data; _data = nullptr; - _hasNontrivialDestructor = false; } // Reserves space for event data. The existing data may be wiped out. void reserve(std::size_t size) { if (_capacity < size) { - assertDestruct(); - delete [] _data; + if (!_anyPtr) // Make a new empty AnyPtr object + _anyPtr = std::make_unique_for_overwrite(); + else if (_anyPtr->has_value()) + *_anyPtr = std::any{}; // Destroy the object in the buffer, if any. _name = ""; _capacity = size; + delete [] _data; _data = new std::byte[size]; - _hasNontrivialDestructor = false; } } @@ -186,9 +194,13 @@ struct Event { // Returns true if the event is empty (i.e. name string is not set) bool isEmpty() const { return _name.empty(); } - // Returns the name of the event. + // Checks if the event has data in the buffer. + bool hasData() const { return (_anyPtr && _anyPtr->has_value()); } + + // Returns the name of the event as a string_view. std::string_view name() const { return _name; } + // The same as above but as a string. std::string nameAsString() const { return std::string(_name); } private: @@ -196,17 +208,43 @@ struct Event { Event(const Event&) = delete; Event& operator=(const Event&) = delete; - // This function is called before the data buffer is about - // to wink out of existance. If the buffer is still holding - // a non-trivially destructible object which has not been explicitly destroy()'ed, - // a runtime_error will be thrown. - void assertDestruct() const + // A typed pointer to the object in the storage space. + // When AnyPtr is destroyed, the object in the storage is also destroyed. + template + struct AnyPtr { - if (_hasNontrivialDestructor) { - std::string msg = "Attempt to reuse or destroy an event of type '" + std::string(_name) + - "' without calling event.destroy(pointer-to-data) first."; - throw std::runtime_error(msg); + AnyPtr(T* p = nullptr) : ptr(p) {} + T* ptr; + ~AnyPtr() + { + if (ptr) + ptr->~T(); + } + }; + + // No need for an explicit destructor call if T is trivially destructible. + template + struct AnyPtr + { + AnyPtr(T* p = nullptr) : ptr(p) {} + T* ptr; + }; + + // Get pointer to the data buffer if T is the type of the object in the buffer. + // Otherwise, throw an exception. + template + T* safeCast() + { + try { + if (_anyPtr && _anyPtr->has_value()) + return std::any_cast&>(*_anyPtr).ptr; + else + throw std::runtime_error("CoFSM::Event does not contain data so data pointer can not be returned."); + } + catch (const std::bad_any_cast&) { + throw std::runtime_error("Attempt to store pointer to the object in CoFSM::Event into a variable of wrong type."); } + return nullptr; } // Name of the object store in the data buffer. @@ -216,10 +254,9 @@ struct Event { std::size_t _capacity = 0; // Pointer to data buffer std::byte* _data = nullptr; - // The object which has been constructed in the data buffer - // has non-trivial destructor, so destroy must be - // called before the buffer is reused for another object. - bool _hasNontrivialDestructor = false; + // An std::any object which contains an object of type AnyPtr where T is the type + // of the object living in the buffer. T = void if there is no object in the buffer. + std::unique_ptr _anyPtr; }; // Event // Returns true if the name of the event is sv. @@ -392,9 +429,7 @@ class FSM { FSM() { _name = asHex(this); }; FSM(const FSM&) = delete; - FSM(FSM&&) noexcept = default; FSM& operator=(const FSM&) = delete; - FSM& operator=(FSM&&) noexcept = default; ~FSM() = default; // Returns the name of the FSM