From 5feccccc880a2c7645b6c2be5ae383a4fcf284c7 Mon Sep 17 00:00:00 2001 From: Cameron Miller Date: Wed, 8 Sep 2021 18:52:45 +0000 Subject: [PATCH] Restructure cache interfaces and cleanup cache implementation Signed-off-by: Cameron Miller --- .../cache_buffer_interface.hpp} | 17 ++++++++--------- .../rosbag2_cpp/cache/cache_consumer.hpp | 8 ++++---- .../cache/circular_message_cache.hpp | 9 ++++----- .../rosbag2_cpp/cache/message_cache.hpp | 9 ++++----- .../cache/message_cache_buffer.hpp | 10 +++++----- .../cache/message_cache_circular_buffer.hpp | 12 ++++++------ .../message_cache_interface.hpp} | 19 +++++++++---------- .../rosbag2_cpp/writers/sequential_writer.hpp | 4 ++-- .../src/rosbag2_cpp/cache/cache_consumer.cpp | 2 +- .../cache/circular_message_cache.cpp | 5 ++--- .../src/rosbag2_cpp/cache/message_cache.cpp | 5 ++--- 11 files changed, 47 insertions(+), 53 deletions(-) rename rosbag2_cpp/include/rosbag2_cpp/{cache_interfaces/base_cache_buffer_interface.hpp => cache/cache_buffer_interface.hpp} (74%) rename rosbag2_cpp/include/rosbag2_cpp/{cache_interfaces/base_message_cache_interface.hpp => cache/message_cache_interface.hpp} (65%) diff --git a/rosbag2_cpp/include/rosbag2_cpp/cache_interfaces/base_cache_buffer_interface.hpp b/rosbag2_cpp/include/rosbag2_cpp/cache/cache_buffer_interface.hpp similarity index 74% rename from rosbag2_cpp/include/rosbag2_cpp/cache_interfaces/base_cache_buffer_interface.hpp rename to rosbag2_cpp/include/rosbag2_cpp/cache/cache_buffer_interface.hpp index d9d710f0e1..2afdae34bf 100644 --- a/rosbag2_cpp/include/rosbag2_cpp/cache_interfaces/base_cache_buffer_interface.hpp +++ b/rosbag2_cpp/include/rosbag2_cpp/cache/cache_buffer_interface.hpp @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef ROSBAG2_CPP__CACHE_INTERFACES__BASE_CACHE_BUFFER_INTERFACE_HPP_ -#define ROSBAG2_CPP__CACHE_INTERFACES__BASE_CACHE_BUFFER_INTERFACE_HPP_ +#ifndef ROSBAG2_CPP__CACHE__CACHE_BUFFER_INTERFACE_HPP_ +#define ROSBAG2_CPP__CACHE__CACHE_BUFFER_INTERFACE_HPP_ #include #include @@ -23,14 +23,13 @@ namespace rosbag2_cpp { -using buffer_element_t = std::shared_ptr; -namespace cache_interfaces +namespace cache { - -class ROSBAG2_CPP_PUBLIC BaseCacheBufferInterface +using buffer_element_t = std::shared_ptr; +class ROSBAG2_CPP_PUBLIC CacheBufferInterface { public: - virtual ~BaseCacheBufferInterface() {} + virtual ~CacheBufferInterface() {} virtual bool push(buffer_element_t msg) = 0; @@ -41,7 +40,7 @@ class ROSBAG2_CPP_PUBLIC BaseCacheBufferInterface virtual const std::vector & data() = 0; }; -} // namespace cache_interfaces +} // namespace cache } // namespace rosbag2_cpp -#endif // ROSBAG2_CPP__CACHE_INTERFACES__BASE_CACHE_BUFFER_INTERFACE_HPP_ +#endif // ROSBAG2_CPP__CACHE__CACHE_BUFFER_INTERFACE_HPP_ diff --git a/rosbag2_cpp/include/rosbag2_cpp/cache/cache_consumer.hpp b/rosbag2_cpp/include/rosbag2_cpp/cache/cache_consumer.hpp index 27b5b2392a..89c8072bcd 100644 --- a/rosbag2_cpp/include/rosbag2_cpp/cache/cache_consumer.hpp +++ b/rosbag2_cpp/include/rosbag2_cpp/cache/cache_consumer.hpp @@ -23,7 +23,7 @@ #include #include "rosbag2_cpp/cache/message_cache.hpp" -#include "rosbag2_cpp/cache_interfaces/base_message_cache_interface.hpp" +#include "rosbag2_cpp/cache/message_cache_interface.hpp" // This is necessary because of using stl types here. It is completely safe, because // a) the member is not accessible from the outside @@ -62,10 +62,10 @@ class ROSBAG2_CPP_PUBLIC CacheConsumer { public: using consume_callback_function_t = std::function &)>; + std::vector &)>; CacheConsumer( - std::shared_ptr message_cache, + std::shared_ptr message_cache, consume_callback_function_t consume_callback); ~CacheConsumer(); @@ -77,7 +77,7 @@ class ROSBAG2_CPP_PUBLIC CacheConsumer void change_consume_callback(consume_callback_function_t callback); private: - std::shared_ptr message_cache_; + std::shared_ptr message_cache_; consume_callback_function_t consume_callback_; /// Write buffer data to a storage diff --git a/rosbag2_cpp/include/rosbag2_cpp/cache/circular_message_cache.hpp b/rosbag2_cpp/include/rosbag2_cpp/cache/circular_message_cache.hpp index 29bbaf806e..b4e85a8bb6 100644 --- a/rosbag2_cpp/include/rosbag2_cpp/cache/circular_message_cache.hpp +++ b/rosbag2_cpp/include/rosbag2_cpp/cache/circular_message_cache.hpp @@ -20,8 +20,8 @@ #include #include "rosbag2_cpp/cache/message_cache_circular_buffer.hpp" -#include "rosbag2_cpp/cache_interfaces/base_message_cache_interface.hpp" -#include "rosbag2_cpp/cache_interfaces/base_cache_buffer_interface.hpp" +#include "rosbag2_cpp/cache/message_cache_interface.hpp" +#include "rosbag2_cpp/cache/cache_buffer_interface.hpp" #include "rosbag2_cpp/visibility_control.hpp" #include "rosbag2_storage/serialized_bag_message.hpp" @@ -40,7 +40,7 @@ namespace cache { class ROSBAG2_CPP_PUBLIC CircularMessageCache - : public rosbag2_cpp::cache_interfaces::BaseMessageCacheInterface + : public MessageCacheInterface { public: explicit CircularMessageCache(uint64_t max_buffer_size); @@ -49,8 +49,7 @@ class ROSBAG2_CPP_PUBLIC CircularMessageCache void push(std::shared_ptr msg) override; /// get current buffer to consume - std::shared_ptr - consumer_buffer() override; + std::shared_ptr consumer_buffer() override; /// Swap the primary and secondary buffer before consumption. /// NOTE: consumer_buffer() should be called sequentially after diff --git a/rosbag2_cpp/include/rosbag2_cpp/cache/message_cache.hpp b/rosbag2_cpp/include/rosbag2_cpp/cache/message_cache.hpp index 7515775f96..4be8fb3a4d 100644 --- a/rosbag2_cpp/include/rosbag2_cpp/cache/message_cache.hpp +++ b/rosbag2_cpp/include/rosbag2_cpp/cache/message_cache.hpp @@ -24,8 +24,8 @@ #include #include "rosbag2_cpp/cache/message_cache_buffer.hpp" -#include "rosbag2_cpp/cache_interfaces/base_message_cache_interface.hpp" -#include "rosbag2_cpp/cache_interfaces/base_cache_buffer_interface.hpp" +#include "rosbag2_cpp/cache/message_cache_interface.hpp" +#include "rosbag2_cpp/cache/cache_buffer_interface.hpp" #include "rosbag2_cpp/visibility_control.hpp" #include "rosbag2_storage/serialized_bag_message.hpp" @@ -62,7 +62,7 @@ namespace cache * performance issues, most likely with the CacheConsumer consumer callback. */ class ROSBAG2_CPP_PUBLIC MessageCache - : public rosbag2_cpp::cache_interfaces::BaseMessageCacheInterface + : public MessageCacheInterface { public: explicit MessageCache(uint64_t max_buffer_size); @@ -91,8 +91,7 @@ class ROSBAG2_CPP_PUBLIC MessageCache void swap_buffers() override; /// Consumer API: get current buffer to consume - std::shared_ptr - consumer_buffer() override; + std::shared_ptr consumer_buffer() override; /// Exposes counts of messages dropped per topic std::unordered_map messages_dropped() const; diff --git a/rosbag2_cpp/include/rosbag2_cpp/cache/message_cache_buffer.hpp b/rosbag2_cpp/include/rosbag2_cpp/cache/message_cache_buffer.hpp index 7a3bd47150..0b4b6a0077 100644 --- a/rosbag2_cpp/include/rosbag2_cpp/cache/message_cache_buffer.hpp +++ b/rosbag2_cpp/include/rosbag2_cpp/cache/message_cache_buffer.hpp @@ -20,7 +20,7 @@ #include #include "rosbag2_cpp/visibility_control.hpp" -#include "rosbag2_cpp/cache_interfaces/base_cache_buffer_interface.hpp" +#include "rosbag2_cpp/cache/cache_buffer_interface.hpp" #include "rosbag2_storage/serialized_bag_message.hpp" // This is necessary because of using stl types here. It is completely safe, because @@ -46,7 +46,7 @@ namespace cache * ->byte_size() - like interface */ class ROSBAG2_CPP_PUBLIC MessageCacheBuffer - : public rosbag2_cpp::cache_interfaces::BaseCacheBufferInterface + : public CacheBufferInterface { public: explicit MessageCacheBuffer(const uint64_t max_cache_size); @@ -56,7 +56,7 @@ class ROSBAG2_CPP_PUBLIC MessageCacheBuffer * this results in exceeding buffer size, we mark buffer to drop all new incoming messages. * This flag is cleared when buffers are swapped. */ - bool push(rosbag2_cpp::buffer_element_t msg) override; + bool push(buffer_element_t msg) override; /// Clear buffer void clear() override; @@ -65,10 +65,10 @@ class ROSBAG2_CPP_PUBLIC MessageCacheBuffer size_t size() override; /// Get buffer data - const std::vector & data() override; + const std::vector & data() override; private: - std::vector buffer_; + std::vector buffer_; uint64_t buffer_bytes_size_ {0u}; const uint64_t max_bytes_size_; diff --git a/rosbag2_cpp/include/rosbag2_cpp/cache/message_cache_circular_buffer.hpp b/rosbag2_cpp/include/rosbag2_cpp/cache/message_cache_circular_buffer.hpp index 6bcfbe857a..4209d70784 100644 --- a/rosbag2_cpp/include/rosbag2_cpp/cache/message_cache_circular_buffer.hpp +++ b/rosbag2_cpp/include/rosbag2_cpp/cache/message_cache_circular_buffer.hpp @@ -20,7 +20,7 @@ #include #include "rosbag2_cpp/visibility_control.hpp" -#include "rosbag2_cpp/cache_interfaces/base_cache_buffer_interface.hpp" +#include "rosbag2_cpp/cache/cache_buffer_interface.hpp" #include "rosbag2_storage/serialized_bag_message.hpp" // This is necessary because of using stl types here. It is completely safe, because @@ -46,7 +46,7 @@ namespace cache * size. */ class ROSBAG2_CPP_PUBLIC MessageCacheCircularBuffer - : public rosbag2_cpp::cache_interfaces::BaseCacheBufferInterface + : public CacheBufferInterface { public: // Delete default constructor since max_cache_size is required @@ -57,7 +57,7 @@ class ROSBAG2_CPP_PUBLIC MessageCacheCircularBuffer * If buffer size has some space left, we push the message regardless of its size, * but if this results in exceeding buffer size, we begin dropping old messages. */ - bool push(rosbag2_cpp::buffer_element_t msg) override; + bool push(buffer_element_t msg) override; /// Clear buffer void clear() override; @@ -66,11 +66,11 @@ class ROSBAG2_CPP_PUBLIC MessageCacheCircularBuffer size_t size() override; /// Get buffer data - const std::vector & data() override; + const std::vector & data() override; private: - std::deque buffer_; - std::vector msg_vector_; + std::deque buffer_; + std::vector msg_vector_; uint64_t buffer_bytes_size_ {0u}; const uint64_t max_bytes_size_; }; diff --git a/rosbag2_cpp/include/rosbag2_cpp/cache_interfaces/base_message_cache_interface.hpp b/rosbag2_cpp/include/rosbag2_cpp/cache/message_cache_interface.hpp similarity index 65% rename from rosbag2_cpp/include/rosbag2_cpp/cache_interfaces/base_message_cache_interface.hpp rename to rosbag2_cpp/include/rosbag2_cpp/cache/message_cache_interface.hpp index f07b0651cb..fcf47d806a 100644 --- a/rosbag2_cpp/include/rosbag2_cpp/cache_interfaces/base_message_cache_interface.hpp +++ b/rosbag2_cpp/include/rosbag2_cpp/cache/message_cache_interface.hpp @@ -12,29 +12,28 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef ROSBAG2_CPP__CACHE_INTERFACES__BASE_MESSAGE_CACHE_INTERFACE_HPP_ -#define ROSBAG2_CPP__CACHE_INTERFACES__BASE_MESSAGE_CACHE_INTERFACE_HPP_ +#ifndef ROSBAG2_CPP__CACHE__MESSAGE_CACHE_INTERFACE_HPP_ +#define ROSBAG2_CPP__CACHE__MESSAGE_CACHE_INTERFACE_HPP_ #include #include "rosbag2_cpp/visibility_control.hpp" -#include "rosbag2_cpp/cache_interfaces/base_cache_buffer_interface.hpp" +#include "rosbag2_cpp/cache/cache_buffer_interface.hpp" #include "rosbag2_storage/serialized_bag_message.hpp" namespace rosbag2_cpp { -namespace cache_interfaces +namespace cache { -class ROSBAG2_CPP_PUBLIC BaseMessageCacheInterface +class ROSBAG2_CPP_PUBLIC MessageCacheInterface { public: - virtual ~BaseMessageCacheInterface() {} + virtual ~MessageCacheInterface() {} virtual void push(std::shared_ptr msg) = 0; - virtual std::shared_ptr - consumer_buffer() = 0; + virtual std::shared_ptr consumer_buffer() = 0; virtual void swap_buffers() = 0; @@ -45,7 +44,7 @@ class ROSBAG2_CPP_PUBLIC BaseMessageCacheInterface virtual void finalize() {} }; -} // namespace cache_interfaces +} // namespace cache } // namespace rosbag2_cpp -#endif // ROSBAG2_CPP__CACHE_INTERFACES__BASE_MESSAGE_CACHE_INTERFACE_HPP_ +#endif // ROSBAG2_CPP__CACHE__MESSAGE_CACHE_INTERFACE_HPP_ diff --git a/rosbag2_cpp/include/rosbag2_cpp/writers/sequential_writer.hpp b/rosbag2_cpp/include/rosbag2_cpp/writers/sequential_writer.hpp index 23cfa9258f..14b5602028 100644 --- a/rosbag2_cpp/include/rosbag2_cpp/writers/sequential_writer.hpp +++ b/rosbag2_cpp/include/rosbag2_cpp/writers/sequential_writer.hpp @@ -23,7 +23,7 @@ #include "rosbag2_cpp/cache/cache_consumer.hpp" #include "rosbag2_cpp/cache/message_cache.hpp" -#include "rosbag2_cpp/cache_interfaces/base_message_cache_interface.hpp" +#include "rosbag2_cpp/cache/message_cache_interface.hpp" #include "rosbag2_cpp/converter.hpp" #include "rosbag2_cpp/serialization_format_converter_factory.hpp" #include "rosbag2_cpp/writer_interfaces/base_writer_interface.hpp" @@ -116,7 +116,7 @@ class ROSBAG2_CPP_PUBLIC SequentialWriter std::unique_ptr converter_; bool use_cache_; - std::shared_ptr message_cache_; + std::shared_ptr message_cache_; std::unique_ptr cache_consumer_; void switch_to_next_storage(); diff --git a/rosbag2_cpp/src/rosbag2_cpp/cache/cache_consumer.cpp b/rosbag2_cpp/src/rosbag2_cpp/cache/cache_consumer.cpp index a76cd359ca..1632560b21 100644 --- a/rosbag2_cpp/src/rosbag2_cpp/cache/cache_consumer.cpp +++ b/rosbag2_cpp/src/rosbag2_cpp/cache/cache_consumer.cpp @@ -23,7 +23,7 @@ namespace cache { CacheConsumer::CacheConsumer( - std::shared_ptr message_cache, + std::shared_ptr message_cache, consume_callback_function_t consume_callback) : message_cache_(message_cache), consume_callback_(consume_callback) diff --git a/rosbag2_cpp/src/rosbag2_cpp/cache/circular_message_cache.cpp b/rosbag2_cpp/src/rosbag2_cpp/cache/circular_message_cache.cpp index 344d9b0544..c49b8d73ac 100644 --- a/rosbag2_cpp/src/rosbag2_cpp/cache/circular_message_cache.cpp +++ b/rosbag2_cpp/src/rosbag2_cpp/cache/circular_message_cache.cpp @@ -19,7 +19,7 @@ #include "rosbag2_cpp/cache/circular_message_cache.hpp" #include "rosbag2_cpp/cache/message_cache_circular_buffer.hpp" -#include "rosbag2_cpp/cache_interfaces/base_cache_buffer_interface.hpp" +#include "rosbag2_cpp/cache/cache_buffer_interface.hpp" #include "rosbag2_cpp/logging.hpp" namespace rosbag2_cpp @@ -39,8 +39,7 @@ void CircularMessageCache::push(std::shared_ptrpush(msg); } -std::shared_ptr CircularMessageCache:: -consumer_buffer() +std::shared_ptr CircularMessageCache::consumer_buffer() { return secondary_buffer_; } diff --git a/rosbag2_cpp/src/rosbag2_cpp/cache/message_cache.cpp b/rosbag2_cpp/src/rosbag2_cpp/cache/message_cache.cpp index f04024b75b..f1ba90d970 100644 --- a/rosbag2_cpp/src/rosbag2_cpp/cache/message_cache.cpp +++ b/rosbag2_cpp/src/rosbag2_cpp/cache/message_cache.cpp @@ -20,7 +20,7 @@ #include #include "rosbag2_cpp/cache/message_cache.hpp" -#include "rosbag2_cpp/cache_interfaces/base_message_cache_interface.hpp" +#include "rosbag2_cpp/cache/message_cache_interface.hpp" #include "rosbag2_cpp/logging.hpp" namespace rosbag2_cpp @@ -88,8 +88,7 @@ void MessageCache::swap_buffers() std::swap(primary_buffer_, secondary_buffer_); } -std::shared_ptr MessageCache:: -consumer_buffer() +std::shared_ptr MessageCache::consumer_buffer() { return secondary_buffer_; }