From be84897ed608eca79b421e56523fdd9dfae56abf Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Wed, 14 Oct 2015 08:29:50 +0300 Subject: [PATCH 01/25] First stab at global refactoring of memory management - the library builds, but unit tests are failing horribly --- libuavcan/include/uavcan/build_config.hpp | 18 -- .../uavcan/node/generic_subscriber.hpp | 13 +- libuavcan/include/uavcan/node/node.hpp | 23 +- .../include/uavcan/node/service_client.hpp | 72 +++-- .../include/uavcan/node/service_server.hpp | 23 +- libuavcan/include/uavcan/node/sub_node.hpp | 6 +- libuavcan/include/uavcan/node/subscriber.hpp | 23 +- .../distributed/raft_core.hpp | 4 +- .../node_discoverer.hpp | 6 +- .../protocol/global_time_sync_slave.hpp | 3 +- .../uavcan/protocol/node_info_retriever.hpp | 5 +- .../uavcan/protocol/node_status_monitor.hpp | 2 +- .../can_acceptance_filter_configurator.hpp | 2 +- .../transport/outgoing_transfer_registry.hpp | 22 +- .../uavcan/transport/transfer_buffer.hpp | 47 +--- .../uavcan/transport/transfer_listener.hpp | 28 +- libuavcan/include/uavcan/util/map.hpp | 254 ++---------------- libuavcan/include/uavcan/util/multiset.hpp | 94 ++----- .../src/transport/uc_transfer_buffer.cpp | 147 +--------- libuavcan/test/node/test_node.hpp | 2 +- .../distributed/server.cpp | 3 +- .../node_discoverer.cpp | 4 +- libuavcan/test/transport/dispatcher.cpp | 10 +- .../test/transport/incoming_transfer.cpp | 2 +- .../transport/outgoing_transfer_registry.cpp | 2 +- libuavcan/test/transport/transfer_buffer.cpp | 32 +-- .../test/transport/transfer_listener.cpp | 12 +- .../test/transport/transfer_receiver.cpp | 2 +- libuavcan/test/transport/transfer_sender.cpp | 12 +- .../test/transport/transfer_test_helpers.cpp | 2 +- .../test/transport/transfer_test_helpers.hpp | 6 +- libuavcan/test/util/map.cpp | 44 ++- libuavcan/test/util/multiset.cpp | 34 +-- 33 files changed, 203 insertions(+), 756 deletions(-) diff --git a/libuavcan/include/uavcan/build_config.hpp b/libuavcan/include/uavcan/build_config.hpp index 89688a3056..fedea852b7 100644 --- a/libuavcan/include/uavcan/build_config.hpp +++ b/libuavcan/include/uavcan/build_config.hpp @@ -239,24 +239,6 @@ static const unsigned MaxCanAcceptanceFilters = UAVCAN_MAX_CAN_ACCEPTANCE_FILTER static const unsigned MaxCanAcceptanceFilters = 32; #endif -/** - * This constant defines how many receiver objects will be statically pre-allocated by classes that listen to messages - * of type uavcan.protocol.NodeStatus from other nodes. If the number of publishers exceeds this value, extra - * listeners will be allocated in the dynamic memory (one block per listener). - * - * The default value is safe to use in any application (since extra memory can always be retrieved from the pool). - * Applications that are extremely memory sensitive and are not expected to operate in large networks can override - * the default with a lower value. - * - * Note that nodes that aren't using classes that monitor the network (e.g. NodeStatusMonitor, dynamic node ID - * allocation servers) do not depend on this configuration parameter in any way. - */ -#ifdef UAVCAN_MAX_NETWORK_SIZE_HINT -static const unsigned MaxNetworkSizeHint = UAVCAN_MAX_NETWORK_SIZE_HINT; -#else -static const unsigned MaxNetworkSizeHint = 64; ///< Rest will go into the dynamic memory -#endif - } #endif // UAVCAN_BUILD_CONFIG_HPP_INCLUDED diff --git a/libuavcan/include/uavcan/node/generic_subscriber.hpp b/libuavcan/include/uavcan/node/generic_subscriber.hpp index bb203788c6..eca19c829f 100644 --- a/libuavcan/include/uavcan/node/generic_subscriber.hpp +++ b/libuavcan/include/uavcan/node/generic_subscriber.hpp @@ -119,25 +119,16 @@ public: * This helper class does some compile-time magic on the transport layer machinery. For authorized personnel only. */ template class TransferListenerTemplate = TransferListener + template class TransferListenerTemplate = TransferListener > class UAVCAN_EXPORT TransferListenerInstantiationHelper { enum { DataTypeMaxByteLen = BitLenToByteLen::Result }; enum { NeedsBuffer = int(DataTypeMaxByteLen) > int(GuaranteedPayloadLenPerFrame) }; enum { BufferSize = NeedsBuffer ? DataTypeMaxByteLen : 0 }; -#if UAVCAN_TINY - enum { NumStaticBufs = 0 }; - enum { NumStaticReceivers = 0 }; -#else - enum { NumStaticBufs = NeedsBuffer ? NumStaticBufs_: 0 }; - enum { NumStaticReceivers = NumStaticReceivers_ }; -#endif public: - typedef TransferListenerTemplate Type; + typedef TransferListenerTemplate Type; }; /** diff --git a/libuavcan/include/uavcan/node/node.hpp b/libuavcan/include/uavcan/node/node.hpp index 6ea59a2244..d9b6502736 100644 --- a/libuavcan/include/uavcan/node/node.hpp +++ b/libuavcan/include/uavcan/node/node.hpp @@ -35,22 +35,8 @@ namespace uavcan * For simple nodes this number can be reduced. * For high-traffic nodes the recommended minimum is * like 16K * (number of CAN ifaces + 1). - * - * @tparam OutgoingTransferRegistryStaticEntries Number of statically allocated objects - * to track Transfer ID for outgoing transfers. - * Normally it should be equal to expected number of - * publishers and service callers, but it's not necessary. - * Additional objects for Transfer ID tracking will - * be allocated in the memory pool if needed. - * Default value is acceptable for any use case. */ -template +template class UAVCAN_EXPORT Node : public INode { enum @@ -61,7 +47,7 @@ class UAVCAN_EXPORT Node : public INode typedef PoolAllocator Allocator; Allocator pool_allocator_; - OutgoingTransferRegistry outgoing_trans_reg_; + OutgoingTransferRegistry outgoing_trans_reg_; Scheduler scheduler_; NodeStatusProvider proto_nsp_; @@ -260,9 +246,8 @@ public: // ---------------------------------------------------------------------------- -template -int Node::start( - const TransferPriority priority) +template +int Node::start(const TransferPriority priority) { if (started_) { diff --git a/libuavcan/include/uavcan/node/service_client.hpp b/libuavcan/include/uavcan/node/service_client.hpp index daea7ac7cf..a7cce0dd4e 100644 --- a/libuavcan/include/uavcan/node/service_client.hpp +++ b/libuavcan/include/uavcan/node/service_client.hpp @@ -21,13 +21,11 @@ namespace uavcan { -template +template class UAVCAN_EXPORT ServiceResponseTransferListenerInstantiationHelper { -public: // so much templating it hurts +public: typedef typename TransferListenerInstantiationHelper::Type Type; }; @@ -217,24 +215,18 @@ public: * In C++11 mode this type defaults to std::function<>. * In C++03 mode this type defaults to a plain function pointer; use binder to * call member functions as callbacks. - * - * @tparam NumStaticCalls_ Number of concurrent calls that the class will be able to handle without using the - * memory pool. Note that this is NOT the maximum possible number of concurrent calls, - * there's no such limit. Defaults to one. */ template = UAVCAN_CPP11 - typename Callback_ = std::function&)>, + typename Callback_ = std::function&)> #else - typename Callback_ = void (*)(const ServiceCallResult&), + typename Callback_ = void (*)(const ServiceCallResult&) #endif - unsigned NumStaticCalls_ = 1 > class UAVCAN_EXPORT ServiceClient : public GenericSubscriber::Type> + typename ServiceResponseTransferListenerInstantiationHelper::Type> , public ServiceClientBase { public: @@ -244,16 +236,13 @@ public: typedef ServiceCallResult ServiceCallResultType; typedef Callback_ Callback; - enum { NumStaticCalls = NumStaticCalls_ }; - private: typedef ServiceClient SelfType; typedef GenericPublisher PublisherType; - typedef typename ServiceResponseTransferListenerInstantiationHelper::Type - TransferListenerType; + typedef typename ServiceResponseTransferListenerInstantiationHelper::Type TransferListenerType; typedef GenericSubscriber SubscriberType; - typedef Multiset CallRegistry; + typedef Multiset CallRegistry; struct TimeoutCallbackCaller { @@ -424,8 +413,8 @@ public: // ---------------------------------------------------------------------------- -template -void ServiceClient::invokeCallback(ServiceCallResultType& result) +template +void ServiceClient::invokeCallback(ServiceCallResultType& result) { if (coerceOrFallback(callback_, true)) { @@ -437,8 +426,8 @@ void ServiceClient::invokeCallback(Servic } } -template -bool ServiceClient::shouldAcceptFrame(const RxFrame& frame) const +template +bool ServiceClient::shouldAcceptFrame(const RxFrame& frame) const { UAVCAN_ASSERT(frame.getTransferType() == TransferTypeServiceResponse); // Other types filtered out by dispatcher @@ -447,9 +436,8 @@ bool ServiceClient::shouldAcceptFrame(con } -template -void ServiceClient:: -handleReceivedDataStruct(ReceivedDataStructure& response) +template +void ServiceClient::handleReceivedDataStruct(ReceivedDataStructure& response) { UAVCAN_ASSERT(response.getTransferType() == TransferTypeServiceResponse); @@ -460,8 +448,8 @@ handleReceivedDataStruct(ReceivedDataStructure& response) } -template -void ServiceClient::handleDeadline(MonotonicTime) +template +void ServiceClient::handleDeadline(MonotonicTime) { UAVCAN_TRACE("ServiceClient", "Shared deadline event received"); /* @@ -484,8 +472,8 @@ void ServiceClient::handleDeadline(Monoto } } -template -int ServiceClient::addCallState(ServiceCallID call_id) +template +int ServiceClient::addCallState(ServiceCallID call_id) { if (call_registry_.isEmpty()) { @@ -507,16 +495,16 @@ int ServiceClient::addCallState(ServiceCa return 0; } -template -int ServiceClient::call(NodeID server_node_id, const RequestType& request) +template +int ServiceClient::call(NodeID server_node_id, const RequestType& request) { ServiceCallID dummy; return call(server_node_id, request, dummy); } -template -int ServiceClient::call(NodeID server_node_id, const RequestType& request, - ServiceCallID& out_call_id) +template +int ServiceClient::call(NodeID server_node_id, const RequestType& request, + ServiceCallID& out_call_id) { if (!coerceOrFallback(callback_, true)) { @@ -573,8 +561,8 @@ int ServiceClient::call(NodeID server_nod return publisher_res; } -template -void ServiceClient::cancelCall(ServiceCallID call_id) +template +void ServiceClient::cancelCall(ServiceCallID call_id) { call_registry_.removeFirstWhere(CallStateMatchingPredicate(call_id)); if (call_registry_.isEmpty()) @@ -583,21 +571,21 @@ void ServiceClient::cancelCall(ServiceCal } } -template -void ServiceClient::cancelAllCalls() +template +void ServiceClient::cancelAllCalls() { call_registry_.clear(); SubscriberType::stop(); } -template -bool ServiceClient::hasPendingCallToServer(NodeID server_node_id) const +template +bool ServiceClient::hasPendingCallToServer(NodeID server_node_id) const { return NULL != call_registry_.find(ServerSearchPredicate(server_node_id)); } -template -ServiceCallID ServiceClient::getCallIDByIndex(unsigned index) const +template +ServiceCallID ServiceClient::getCallIDByIndex(unsigned index) const { const CallState* const id = call_registry_.getByIndex(index); return (id == NULL) ? ServiceCallID() : id->getCallID(); diff --git a/libuavcan/include/uavcan/node/service_server.hpp b/libuavcan/include/uavcan/node/service_server.hpp index d635d9579d..d362502800 100644 --- a/libuavcan/include/uavcan/node/service_server.hpp +++ b/libuavcan/include/uavcan/node/service_server.hpp @@ -77,33 +77,19 @@ public: * In C++11 mode this type defaults to std::function<>. * In C++03 mode this type defaults to a plain function pointer; use binder to * call member functions as callbacks. - * - * @tparam NumStaticReceivers Number of statically allocated receiver objects. If there's more service - * clients for this service, extra receivers will be allocated in the memory pool. - * - * @tparam NumStaticBufs Number of statically allocated receiver buffers. If there's more concurrent - * incoming transfers, extra buffers will be allocated in the memory pool. */ template = UAVCAN_CPP11 typename Callback_ = std::function&, - ServiceResponseDataStructure&)>, + ServiceResponseDataStructure&)> #else typename Callback_ = void (*)(const ReceivedDataStructure&, - ServiceResponseDataStructure&), -#endif -#if UAVCAN_TINY - unsigned NumStaticReceivers = 0, - unsigned NumStaticBufs = 0 -#else - unsigned NumStaticReceivers = 2, - unsigned NumStaticBufs = 1 + ServiceResponseDataStructure&) #endif > class UAVCAN_EXPORT ServiceServer : public GenericSubscriber::Type> + typename TransferListenerInstantiationHelper::Type> { public: typedef DataType_ DataType; @@ -112,8 +98,7 @@ public: typedef Callback_ Callback; private: - typedef typename TransferListenerInstantiationHelper::Type - TransferListenerType; + typedef typename TransferListenerInstantiationHelper::Type TransferListenerType; typedef GenericSubscriber SubscriberType; typedef GenericPublisher PublisherType; diff --git a/libuavcan/include/uavcan/node/sub_node.hpp b/libuavcan/include/uavcan/node/sub_node.hpp index ec5484365d..2a7de956ef 100644 --- a/libuavcan/include/uavcan/node/sub_node.hpp +++ b/libuavcan/include/uavcan/node/sub_node.hpp @@ -20,9 +20,7 @@ namespace uavcan * Please refer to the @ref Node<> for documentation concerning the template arguments; refer to the tutorials * to lean how to use libuavcan in multiprocess applications. */ -template +template class UAVCAN_EXPORT SubNode : public INode { enum @@ -33,7 +31,7 @@ class UAVCAN_EXPORT SubNode : public INode typedef PoolAllocator Allocator; Allocator pool_allocator_; - OutgoingTransferRegistry outgoing_trans_reg_; + OutgoingTransferRegistry outgoing_trans_reg_; Scheduler scheduler_; uint64_t internal_failure_cnt_; diff --git a/libuavcan/include/uavcan/node/subscriber.hpp b/libuavcan/include/uavcan/node/subscriber.hpp index ac094e5c32..b82233dddd 100644 --- a/libuavcan/include/uavcan/node/subscriber.hpp +++ b/libuavcan/include/uavcan/node/subscriber.hpp @@ -34,38 +34,23 @@ namespace uavcan * In C++11 mode this type defaults to std::function<>. * In C++03 mode this type defaults to a plain function pointer; use binder to * call member functions as callbacks. - * - * @tparam NumStaticReceivers Number of statically allocated receiver objects. If there's more publishers - * of this message, extra receivers will be allocated in the memory pool. - * - * @tparam NumStaticBufs Number of statically allocated receiver buffers. If there's more concurrent - * incoming transfers, extra buffers will be allocated in the memory pool. */ template = UAVCAN_CPP11 - typename Callback_ = std::function&)>, + typename Callback_ = std::function&)> #else - typename Callback_ = void (*)(const ReceivedDataStructure&), -#endif -#if UAVCAN_TINY - unsigned NumStaticReceivers = 0, - unsigned NumStaticBufs = 0 -#else - unsigned NumStaticReceivers = 2, - unsigned NumStaticBufs = 1 + typename Callback_ = void (*)(const ReceivedDataStructure&) #endif > class UAVCAN_EXPORT Subscriber : public GenericSubscriber::Type> + typename TransferListenerInstantiationHelper::Type> { public: typedef Callback_ Callback; private: - typedef typename TransferListenerInstantiationHelper::Type - TransferListenerType; + typedef typename TransferListenerInstantiationHelper::Type TransferListenerType; typedef GenericSubscriber BaseType; Callback callback_; diff --git a/libuavcan/include/uavcan/protocol/dynamic_node_id_server/distributed/raft_core.hpp b/libuavcan/include/uavcan/protocol/dynamic_node_id_server/distributed/raft_core.hpp index 7e8a0eafa7..4080d8bbae 100644 --- a/libuavcan/include/uavcan/protocol/dynamic_node_id_server/distributed/raft_core.hpp +++ b/libuavcan/include/uavcan/protocol/dynamic_node_id_server/distributed/raft_core.hpp @@ -126,8 +126,8 @@ private: */ ServiceServer append_entries_srv_; ServiceClient append_entries_client_; - ServiceServer request_vote_srv_; - ServiceClient request_vote_client_; + ServiceServer request_vote_srv_; + ServiceClient request_vote_client_; /* * Methods diff --git a/libuavcan/include/uavcan/protocol/dynamic_node_id_server/node_discoverer.hpp b/libuavcan/include/uavcan/protocol/dynamic_node_id_server/node_discoverer.hpp index 2e4a015c63..4f856e7090 100644 --- a/libuavcan/include/uavcan/protocol/dynamic_node_id_server/node_discoverer.hpp +++ b/libuavcan/include/uavcan/protocol/dynamic_node_id_server/node_discoverer.hpp @@ -84,7 +84,7 @@ class NodeDiscoverer : TimerBase { } }; - typedef Map NodeMap; + typedef Map NodeMap; /** * When this number of attempts has been made, the discoverer will give up and assume that the node @@ -101,10 +101,10 @@ class NodeDiscoverer : TimerBase IEventTracer& tracer_; BitSet committed_node_mask_; ///< Nodes that are marked will not be queried - NodeMap node_map_; ///< Will not work in UAVCAN_TINY + NodeMap node_map_; ServiceClient get_node_info_client_; - Subscriber node_status_sub_; + Subscriber node_status_sub_; /* * Methods diff --git a/libuavcan/include/uavcan/protocol/global_time_sync_slave.hpp b/libuavcan/include/uavcan/protocol/global_time_sync_slave.hpp index 2d9336ffb5..770cd721ad 100644 --- a/libuavcan/include/uavcan/protocol/global_time_sync_slave.hpp +++ b/libuavcan/include/uavcan/protocol/global_time_sync_slave.hpp @@ -31,8 +31,7 @@ class UAVCAN_EXPORT GlobalTimeSyncSlave : Noncopyable void (GlobalTimeSyncSlave::*)(const ReceivedDataStructure&)> GlobalTimeSyncCallback; - // Static buffers are explicitly disabled because time should never be unicasted. - Subscriber sub_; + Subscriber sub_; UtcTime prev_ts_utc_; MonotonicTime prev_ts_mono_; diff --git a/libuavcan/include/uavcan/protocol/node_info_retriever.hpp b/libuavcan/include/uavcan/protocol/node_info_retriever.hpp index 3089150f2f..9ff2bcc79b 100644 --- a/libuavcan/include/uavcan/protocol/node_info_retriever.hpp +++ b/libuavcan/include/uavcan/protocol/node_info_retriever.hpp @@ -156,7 +156,6 @@ private: } }; - enum { NumStaticCalls = 2 }; enum { DefaultNumRequestAttempts = 16 }; enum { DefaultTimerIntervalMSec = 40 }; ///< Read explanation in the class documentation @@ -165,9 +164,9 @@ private: */ Entry entries_[NodeID::Max]; // [1, NodeID::Max] - Multiset listeners_; + Multiset listeners_; - ServiceClient get_node_info_client_; + ServiceClient get_node_info_client_; MonotonicDuration request_interval_; diff --git a/libuavcan/include/uavcan/protocol/node_status_monitor.hpp b/libuavcan/include/uavcan/protocol/node_status_monitor.hpp index 2c3c1256fa..c915ec6d10 100644 --- a/libuavcan/include/uavcan/protocol/node_status_monitor.hpp +++ b/libuavcan/include/uavcan/protocol/node_status_monitor.hpp @@ -75,7 +75,7 @@ private: typedef MethodBinder TimerCallback; - Subscriber sub_; + Subscriber sub_; TimerEventForwarder timer_; diff --git a/libuavcan/include/uavcan/transport/can_acceptance_filter_configurator.hpp b/libuavcan/include/uavcan/transport/can_acceptance_filter_configurator.hpp index 8b9a716343..e7c7d6e7a1 100644 --- a/libuavcan/include/uavcan/transport/can_acceptance_filter_configurator.hpp +++ b/libuavcan/include/uavcan/transport/can_acceptance_filter_configurator.hpp @@ -65,7 +65,7 @@ private: static const unsigned DefaultAnonMsgMask = 0xFF; static const unsigned DefaultAnonMsgID = 0x0; - typedef uavcan::Multiset MultisetConfigContainer; + typedef uavcan::Multiset MultisetConfigContainer; static CanFilterConfig mergeFilters(CanFilterConfig &a_, CanFilterConfig &b_); static uint8_t countBits(uint32_t n_); diff --git a/libuavcan/include/uavcan/transport/outgoing_transfer_registry.hpp b/libuavcan/include/uavcan/transport/outgoing_transfer_registry.hpp index 4a93fd2be7..c6d2c0c412 100644 --- a/libuavcan/include/uavcan/transport/outgoing_transfer_registry.hpp +++ b/libuavcan/include/uavcan/transport/outgoing_transfer_registry.hpp @@ -60,6 +60,8 @@ public: * Outgoing transfer registry keeps track of Transfer ID values for all currently existing local transfer senders. * If a local transfer sender was inactive for a sufficiently long time, the outgoing transfer registry will * remove the respective Transfer ID tracking object. + * + * TODO: Deinterface this. */ class UAVCAN_EXPORT IOutgoingTransferRegistry { @@ -74,7 +76,6 @@ public: }; -template class UAVCAN_EXPORT OutgoingTransferRegistry : public IOutgoingTransferRegistry, Noncopyable { struct Value @@ -123,7 +124,7 @@ class UAVCAN_EXPORT OutgoingTransferRegistry : public IOutgoingTransferRegistry, } }; - Map map_; + Map map_; public: explicit OutgoingTransferRegistry(IPoolAllocator& allocator) @@ -140,11 +141,12 @@ public: // ---------------------------------------------------------------------------- /* - * OutgoingTransferRegistry<> + * OutgoingTransferRegistry + * TODO: deinterface and move to .cpp */ -template -TransferID* OutgoingTransferRegistry::accessOrCreate(const OutgoingTransferRegistryKey& key, - MonotonicTime new_deadline) +inline +TransferID* OutgoingTransferRegistry::accessOrCreate(const OutgoingTransferRegistryKey& key, + MonotonicTime new_deadline) { UAVCAN_ASSERT(!new_deadline.isZero()); Value* p = map_.access(key); @@ -161,14 +163,14 @@ TransferID* OutgoingTransferRegistry::accessOrCreate(const Out return &p->tid; } -template -bool OutgoingTransferRegistry::exists(DataTypeID dtid, TransferType tt) const +inline +bool OutgoingTransferRegistry::exists(DataTypeID dtid, TransferType tt) const { return NULL != map_.find(ExistenceCheckingPredicate(dtid, tt)); } -template -void OutgoingTransferRegistry::cleanup(MonotonicTime ts) +inline +void OutgoingTransferRegistry::cleanup(MonotonicTime ts) { map_.removeAllWhere(DeadlineExpiredPredicate(ts)); } diff --git a/libuavcan/include/uavcan/transport/transfer_buffer.hpp b/libuavcan/include/uavcan/transport/transfer_buffer.hpp index 977bd3e368..7ea35145be 100644 --- a/libuavcan/include/uavcan/transport/transfer_buffer.hpp +++ b/libuavcan/include/uavcan/transport/transfer_buffer.hpp @@ -194,8 +194,6 @@ public: virtual int read(unsigned offset, uint8_t* data, unsigned len) const; virtual int write(unsigned offset, const uint8_t* data, unsigned len); - - bool migrateFrom(const TransferBufferManagerEntry* tbme); }; template @@ -250,11 +248,7 @@ class TransferBufferManagerImpl : public ITransferBufferManager, Noncopyable IPoolAllocator& allocator_; const uint16_t max_buf_size_; - virtual StaticTransferBufferManagerEntryImpl* getStaticByIndex(uint16_t index) const = 0; - - StaticTransferBufferManagerEntryImpl* findFirstStatic(const TransferBufferManagerKey& key); DynamicTransferBufferManagerEntry* findFirstDynamic(const TransferBufferManagerKey& key); - void optimizeStorage(); public: TransferBufferManagerImpl(uint16_t max_buf_size, IPoolAllocator& allocator) @@ -269,49 +263,20 @@ public: virtual void remove(const TransferBufferManagerKey& key); virtual bool isEmpty() const; - unsigned getNumDynamicBuffers() const; - unsigned getNumStaticBuffers() const; -}; - -template -class UAVCAN_EXPORT TransferBufferManager : public TransferBufferManagerImpl -{ - mutable StaticTransferBufferManagerEntry static_buffers_[NumStaticBufs]; - - virtual StaticTransferBufferManagerEntry* getStaticByIndex(uint16_t index) const - { - return (index < NumStaticBufs) ? &static_buffers_[index] : NULL; - } - -public: - explicit TransferBufferManager(IPoolAllocator& allocator) - : TransferBufferManagerImpl(MaxBufSize, allocator) - { -#if UAVCAN_TINY - StaticAssert<(NumStaticBufs == 0)>::check(); // Static buffers in UAVCAN_TINY mode are not allowed -#endif - StaticAssert<(MaxBufSize > 0)>::check(); - } + unsigned getNumBuffers() const; }; template -class UAVCAN_EXPORT TransferBufferManager : public TransferBufferManagerImpl +class UAVCAN_EXPORT TransferBufferManager : public TransferBufferManagerImpl { - virtual StaticTransferBufferManagerEntry* getStaticByIndex(uint16_t) const - { - return NULL; - } - public: - explicit TransferBufferManager(IPoolAllocator& allocator) - : TransferBufferManagerImpl(MaxBufSize, allocator) - { - StaticAssert<(MaxBufSize > 0)>::check(); - } + explicit TransferBufferManager(IPoolAllocator& allocator) : + TransferBufferManagerImpl(MaxBufSize, allocator) + { } }; template <> -class UAVCAN_EXPORT TransferBufferManager<0, 0> : public ITransferBufferManager +class UAVCAN_EXPORT TransferBufferManager<0> : public ITransferBufferManager { public: TransferBufferManager() { } diff --git a/libuavcan/include/uavcan/transport/transfer_listener.hpp b/libuavcan/include/uavcan/transport/transfer_listener.hpp index 66bfe9f6f7..181a2e36b1 100644 --- a/libuavcan/include/uavcan/transport/transfer_listener.hpp +++ b/libuavcan/include/uavcan/transport/transfer_listener.hpp @@ -99,7 +99,7 @@ public: class UAVCAN_EXPORT TransferListenerBase : public LinkedListNode, Noncopyable { const DataTypeDescriptor& data_type_; - MapBase& receivers_; + Map& receivers_; ITransferBufferManager& bufmgr_; TransferPerfCounter& perf_; const TransferCRC crc_base_; ///< Pre-initialized with data type hash, thus constant @@ -123,7 +123,7 @@ class UAVCAN_EXPORT TransferListenerBase : public LinkedListNode& receivers, + Map& receivers, ITransferBufferManager& bufmgr) : data_type_(data_type) , receivers_(receivers) @@ -157,24 +157,18 @@ public: /** * This class should be derived by transfer receivers (subscribers, servers). */ -template +template class UAVCAN_EXPORT TransferListener : public TransferListenerBase { - TransferBufferManager bufmgr_; - Map receivers_; + TransferBufferManager bufmgr_; + Map receivers_; public: TransferListener(TransferPerfCounter& perf, const DataTypeDescriptor& data_type, IPoolAllocator& allocator) : TransferListenerBase(perf, data_type, receivers_, bufmgr_) , bufmgr_(allocator) , receivers_(allocator) - { -#if UAVCAN_TINY - StaticAssert::check(); - StaticAssert::check(); -#endif - StaticAssert<(NumStaticReceivers >= NumStaticBufs)>::check(); // Otherwise it would be meaningless - } + { } virtual ~TransferListener() { @@ -200,15 +194,15 @@ public: /** * This class should be derived by callers. */ -template -class UAVCAN_EXPORT TransferListenerWithFilter : public TransferListener +template +class UAVCAN_EXPORT TransferListenerWithFilter : public TransferListener { const ITransferAcceptanceFilter* filter_; virtual void handleFrame(const RxFrame& frame); public: - typedef TransferListener BaseType; + typedef TransferListener BaseType; TransferListenerWithFilter(TransferPerfCounter& perf, const DataTypeDescriptor& data_type, IPoolAllocator& allocator) @@ -227,8 +221,8 @@ public: /* * TransferListenerWithFilter<> */ -template -void TransferListenerWithFilter::handleFrame(const RxFrame& frame) +template +void TransferListenerWithFilter::handleFrame(const RxFrame& frame) { if (filter_ != NULL) { diff --git a/libuavcan/include/uavcan/util/map.hpp b/libuavcan/include/uavcan/util/map.hpp index 9c43394ba3..d7c2b29625 100644 --- a/libuavcan/include/uavcan/util/map.hpp +++ b/libuavcan/include/uavcan/util/map.hpp @@ -18,9 +18,7 @@ namespace uavcan /** * Slow but memory efficient KV container. * - * KV pairs can be allocated in a static buffer or in the node's memory pool if the static buffer is exhausted. - * When a KV pair is deleted from the static buffer, one pair from the memory pool will be moved in the free - * slot of the static buffer, so the use of the memory pool is minimized. + * KV pairs will be allocated in the node's memory pool. * * Please be aware that this container does not perform any speed optimizations to minimize memory footprint, * so the complexity of most operations is O(N). @@ -32,10 +30,8 @@ namespace uavcan * Size of Key + Value + padding must not exceed MemPoolBlockSize. */ template -class UAVCAN_EXPORT MapBase : Noncopyable +class UAVCAN_EXPORT Map : Noncopyable { - template friend class Map; - public: struct KVPair { @@ -102,16 +98,9 @@ private: LinkedListRoot list_; IPoolAllocator& allocator_; -#if !UAVCAN_TINY - KVPair* const static_; - const unsigned num_static_entries_; -#endif KVPair* findKey(const Key& key); -#if !UAVCAN_TINY - void optimizeStorage(); -#endif void compact(); struct YesPredicate @@ -119,30 +108,18 @@ private: bool operator()(const Key&, const Value&) const { return true; } }; -protected: -#if UAVCAN_TINY - MapBase(IPoolAllocator& allocator) - : allocator_(allocator) - { - UAVCAN_ASSERT(Key() == Key()); - } -#else - MapBase(KVPair* static_buf, unsigned num_static_entries, IPoolAllocator& allocator) - : allocator_(allocator) - , static_(static_buf) - , num_static_entries_(num_static_entries) - { - UAVCAN_ASSERT(Key() == Key()); - } -#endif - - /// Derived class destructor must call clear(); - ~MapBase() - { - UAVCAN_ASSERT(getSize() == 0); - } - public: + Map(IPoolAllocator& allocator) : + allocator_(allocator) + { + UAVCAN_ASSERT(Key() == Key()); + } + + ~Map() + { + clear(); + } + /** * Returns null pointer if there's no such entry. */ @@ -192,69 +169,20 @@ public: */ bool isEmpty() const { return find(YesPredicate()) == NULL; } - unsigned getSize() const; - /** - * For testing, do not use directly. + * Complexity is O(N). */ - unsigned getNumStaticPairs() const; - unsigned getNumDynamicPairs() const; -}; - - -template -class UAVCAN_EXPORT Map : public MapBase -{ - typename MapBase::KVPair static_[NumStaticEntries]; - -public: - -#if !UAVCAN_TINY - - // This instantiation will not be valid in UAVCAN_TINY mode - explicit Map(IPoolAllocator& allocator) - : MapBase(static_, NumStaticEntries, allocator) - { } - - ~Map() { this->clear(); } - -#endif // !UAVCAN_TINY -}; - - -template -class UAVCAN_EXPORT Map : public MapBase -{ -public: - explicit Map(IPoolAllocator& allocator) -#if UAVCAN_TINY - : MapBase(allocator) -#else - : MapBase(NULL, 0, allocator) -#endif - { } - - ~Map() { this->clear(); } + unsigned getSize() const; }; // ---------------------------------------------------------------------------- /* - * MapBase<> + * Map<> */ template -typename MapBase::KVPair* MapBase::findKey(const Key& key) +typename Map::KVPair* Map::findKey(const Key& key) { -#if !UAVCAN_TINY - for (unsigned i = 0; i < num_static_entries_; i++) - { - if (static_[i].match(key)) - { - return static_ + i; - } - } -#endif - KVGroup* p = list_.get(); while (p) { @@ -268,64 +196,8 @@ typename MapBase::KVPair* MapBase::findKey(const Key& ke return NULL; } -#if !UAVCAN_TINY - template -void MapBase::optimizeStorage() -{ - while (true) - { - // Looking for first EMPTY static entry - KVPair* stat = NULL; - for (unsigned i = 0; i < num_static_entries_; i++) - { - if (static_[i].match(Key())) - { - stat = static_ + i; - break; - } - } - if (stat == NULL) - { - break; - } - - // Looking for the first NON-EMPTY dynamic entry, erasing immediately - KVGroup* p = list_.get(); - KVPair dyn; - while (p) - { - bool stop = false; - for (int i = 0; i < KVGroup::NumKV; i++) - { - if (!p->kvs[i].match(Key())) // Non empty - { - dyn = p->kvs[i]; // Copy by value - p->kvs[i] = KVPair(); // Erase immediately - stop = true; - break; - } - } - if (stop) - { - break; - } - p = p->getNextListNode(); - } - if (dyn.match(Key())) - { - break; - } - - // Migrating - *stat = dyn; - } -} - -#endif // !UAVCAN_TINY - -template -void MapBase::compact() +void Map::compact() { KVGroup* p = list_.get(); while (p) @@ -350,7 +222,7 @@ void MapBase::compact() } template -Value* MapBase::access(const Key& key) +Value* Map::access(const Key& key) { UAVCAN_ASSERT(!(key == Key())); KVPair* const kv = findKey(key); @@ -358,7 +230,7 @@ Value* MapBase::access(const Key& key) } template -Value* MapBase::insert(const Key& key, const Value& value) +Value* Map::insert(const Key& key, const Value& value) { UAVCAN_ASSERT(!(key == Key())); remove(key); @@ -381,40 +253,23 @@ Value* MapBase::insert(const Key& key, const Value& value) } template -void MapBase::remove(const Key& key) +void Map::remove(const Key& key) { UAVCAN_ASSERT(!(key == Key())); KVPair* const kv = findKey(key); if (kv) { *kv = KVPair(); -#if !UAVCAN_TINY - optimizeStorage(); -#endif compact(); } } template template -void MapBase::removeAllWhere(Predicate predicate) +void Map::removeAllWhere(Predicate predicate) { unsigned num_removed = 0; -#if !UAVCAN_TINY - for (unsigned i = 0; i < num_static_entries_; i++) - { - if (!static_[i].match(Key())) - { - if (predicate(static_[i].key, static_[i].value)) - { - num_removed++; - static_[i] = KVPair(); - } - } - } -#endif - KVGroup* p = list_.get(); while (p != NULL) { @@ -438,30 +293,14 @@ void MapBase::removeAllWhere(Predicate predicate) if (num_removed > 0) { -#if !UAVCAN_TINY - optimizeStorage(); -#endif compact(); } } template template -const Key* MapBase::find(Predicate predicate) const +const Key* Map::find(Predicate predicate) const { -#if !UAVCAN_TINY - for (unsigned i = 0; i < num_static_entries_; i++) - { - if (!static_[i].match(Key())) - { - if (predicate(static_[i].key, static_[i].value)) - { - return &static_[i].key; - } - } - } -#endif - KVGroup* p = list_.get(); while (p != NULL) { @@ -485,29 +324,14 @@ const Key* MapBase::find(Predicate predicate) const } template -void MapBase::clear() +void Map::clear() { removeAllWhere(YesPredicate()); } template -typename MapBase::KVPair* MapBase::getByIndex(unsigned index) +typename Map::KVPair* Map::getByIndex(unsigned index) { -#if !UAVCAN_TINY - // Checking the static storage - for (unsigned i = 0; i < num_static_entries_; i++) - { - if (!static_[i].match(Key())) - { - if (index == 0) - { - return static_ + i; - } - index--; - } - } -#endif - // Slowly crawling through the dynamic storage KVGroup* p = list_.get(); while (p != NULL) @@ -534,35 +358,13 @@ typename MapBase::KVPair* MapBase::getByIndex(unsigned i } template -const typename MapBase::KVPair* MapBase::getByIndex(unsigned index) const +const typename Map::KVPair* Map::getByIndex(unsigned index) const { - return const_cast*>(this)->getByIndex(index); + return const_cast*>(this)->getByIndex(index); } template -unsigned MapBase::getSize() const -{ - return getNumStaticPairs() + getNumDynamicPairs(); -} - -template -unsigned MapBase::getNumStaticPairs() const -{ - unsigned num = 0; -#if !UAVCAN_TINY - for (unsigned i = 0; i < num_static_entries_; i++) - { - if (!static_[i].match(Key())) - { - num++; - } - } -#endif - return num; -} - -template -unsigned MapBase::getNumDynamicPairs() const +unsigned Map::getSize() const { unsigned num = 0; KVGroup* p = list_.get(); diff --git a/libuavcan/include/uavcan/util/multiset.hpp b/libuavcan/include/uavcan/util/multiset.hpp index c1a99d31ea..ae04dfd4e0 100644 --- a/libuavcan/include/uavcan/util/multiset.hpp +++ b/libuavcan/include/uavcan/util/multiset.hpp @@ -23,11 +23,9 @@ namespace uavcan * Slow but memory efficient unordered multiset. Unlike Map<>, this container does not move objects, so * they don't have to be copyable. * - * Items can be allocated in a static buffer or in the node's memory pool if the static buffer is exhausted. - * - * Number of static entries must not be less than 1. + * Items will be allocated in the node's memory pool. */ -template +template class UAVCAN_EXPORT Multiset : Noncopyable { struct Item : ::uavcan::Noncopyable @@ -122,7 +120,6 @@ private: */ LinkedListRoot list_; IPoolAllocator& allocator_; - Item static_[NumStaticEntries]; /* * Methods @@ -333,13 +330,7 @@ public: * Counts number of items stored. * Best case complexity is O(N). */ - unsigned getSize() const { return getNumStaticItems() + getNumDynamicItems(); } - - /** - * For testing, do not use directly. - */ - unsigned getNumStaticItems() const; - unsigned getNumDynamicItems() const; + unsigned getSize() const; }; // ---------------------------------------------------------------------------- @@ -347,21 +338,10 @@ public: /* * Multiset<> */ -template -typename Multiset::Item* Multiset::findOrCreateFreeSlot() +template +typename Multiset::Item* Multiset::findOrCreateFreeSlot() { -#if !UAVCAN_TINY - // Search in static pool - for (unsigned i = 0; i < NumStaticEntries; i++) - { - if (!static_[i].isConstructed()) - { - return &static_[i]; - } - } -#endif - - // Search in dynamic pool + // Search { Chunk* p = list_.get(); while (p) @@ -375,7 +355,7 @@ typename Multiset::Item* Multiset::fin } } - // Create new dynamic chunk + // Create new chunk Chunk* const chunk = Chunk::instantiate(allocator_); if (chunk == NULL) { @@ -385,8 +365,8 @@ typename Multiset::Item* Multiset::fin return &chunk->items[0]; } -template -void Multiset::compact() +template +void Multiset::compact() { Chunk* p = list_.get(); while (p) @@ -410,30 +390,12 @@ void Multiset::compact() } } -template +template template -void Multiset::removeWhere(Predicate predicate, const RemoveStrategy strategy) +void Multiset::removeWhere(Predicate predicate, const RemoveStrategy strategy) { unsigned num_removed = 0; -#if !UAVCAN_TINY - for (unsigned i = 0; i < NumStaticEntries; i++) - { - if (static_[i].isConstructed()) - { - if (predicate(*static_[i].ptr)) - { - num_removed++; - static_[i].destroy(); - if (strategy == RemoveOne) - { - break; - } - } - } - } -#endif - Chunk* p = list_.get(); while (p != NULL) { @@ -470,23 +432,10 @@ void Multiset::removeWhere(Predicate predicate, const Remov } } -template +template template -T* Multiset::find(Predicate predicate) +T* Multiset::find(Predicate predicate) { -#if !UAVCAN_TINY - for (unsigned i = 0; i < NumStaticEntries; i++) - { - if (static_[i].isConstructed()) - { - if (predicate(*static_[i].ptr)) - { - return static_[i].ptr; - } - } - } -#endif - Chunk* p = list_.get(); while (p != NULL) { @@ -508,21 +457,8 @@ T* Multiset::find(Predicate predicate) return NULL; } -template -unsigned Multiset::getNumStaticItems() const -{ - unsigned num = 0; -#if !UAVCAN_TINY - for (unsigned i = 0; i < NumStaticEntries; i++) - { - num += static_[i].isConstructed() ? 1U : 0U; - } -#endif - return num; -} - -template -unsigned Multiset::getNumDynamicItems() const +template +unsigned Multiset::getSize() const { unsigned num = 0; Chunk* p = list_.get(); diff --git a/libuavcan/src/transport/uc_transfer_buffer.cpp b/libuavcan/src/transport/uc_transfer_buffer.cpp index fb3774c7d9..f684d8ecd7 100644 --- a/libuavcan/src/transport/uc_transfer_buffer.cpp +++ b/libuavcan/src/transport/uc_transfer_buffer.cpp @@ -291,58 +291,9 @@ int StaticTransferBufferManagerEntryImpl::write(unsigned offset, const uint8_t* return buf_.write(offset, data, len); } -bool StaticTransferBufferManagerEntryImpl::migrateFrom(const TransferBufferManagerEntry* tbme) -{ - if (tbme == NULL || tbme->isEmpty()) - { - UAVCAN_ASSERT(0); - return false; - } - - // Resetting self and moving all data from the source - TransferBufferManagerEntry::reset(tbme->getKey()); - const int res = tbme->read(0, buf_.getRawPtr(), buf_.getSize()); - if (res < 0) - { - TransferBufferManagerEntry::reset(); - return false; - } - buf_.setMaxWritePos(uint16_t(res)); - if (res < int(buf_.getSize())) - { - return true; - } - - // Now we need to make sure that all data can fit the storage - uint8_t dummy = 0; - if (tbme->read(buf_.getSize(), &dummy, 1) > 0) - { - TransferBufferManagerEntry::reset(); // Damn, the buffer was too large - return false; - } - return true; -} - /* * TransferBufferManagerImpl */ -StaticTransferBufferManagerEntryImpl* TransferBufferManagerImpl::findFirstStatic(const TransferBufferManagerKey& key) -{ - for (unsigned i = 0; true; i++) - { - StaticTransferBufferManagerEntryImpl* const sb = getStaticByIndex(uint16_t(i)); - if (sb == NULL) - { - break; - } - if (sb->getKey() == key) - { - return sb; - } - } - return NULL; -} - DynamicTransferBufferManagerEntry* TransferBufferManagerImpl::findFirstDynamic(const TransferBufferManagerKey& key) { DynamicTransferBufferManagerEntry* dyn = dynamic_buffers_.get(); @@ -358,40 +309,6 @@ DynamicTransferBufferManagerEntry* TransferBufferManagerImpl::findFirstDynamic(c return NULL; } -void TransferBufferManagerImpl::optimizeStorage() -{ - while (!dynamic_buffers_.isEmpty()) - { - StaticTransferBufferManagerEntryImpl* const sb = findFirstStatic(TransferBufferManagerKey()); - if (sb == NULL) - { - break; - } - DynamicTransferBufferManagerEntry* dyn = dynamic_buffers_.get(); - UAVCAN_ASSERT(dyn); - UAVCAN_ASSERT(!dyn->isEmpty()); - if (sb->migrateFrom(dyn)) - { - UAVCAN_ASSERT(!dyn->isEmpty()); - UAVCAN_TRACE("TransferBufferManager", "Storage optimization: Migrated %s", - dyn->getKey().toString().c_str()); - dynamic_buffers_.remove(dyn); - DynamicTransferBufferManagerEntry::destroy(dyn, allocator_); - } - else - { - /* Migration can fail if a dynamic buffer contains more data than a static buffer can accomodate. - * This should never happen during normal operation because dynamic buffers are limited in growth. - */ - UAVCAN_TRACE("TransferBufferManager", "Storage optimization: MIGRATION FAILURE %s MAXSIZE %u", - dyn->getKey().toString().c_str(), max_buf_size_); - UAVCAN_ASSERT(0); - sb->reset(); - break; - } - } -} - TransferBufferManagerImpl::~TransferBufferManagerImpl() { DynamicTransferBufferManagerEntry* dyn = dynamic_buffers_.get(); @@ -411,11 +328,6 @@ ITransferBuffer* TransferBufferManagerImpl::access(const TransferBufferManagerKe UAVCAN_ASSERT(0); return NULL; } - TransferBufferManagerEntry* tbme = findFirstStatic(key); - if (tbme) - { - return tbme; - } return findFirstDynamic(key); } @@ -428,27 +340,17 @@ ITransferBuffer* TransferBufferManagerImpl::create(const TransferBufferManagerKe } remove(key); - TransferBufferManagerEntry* tbme = findFirstStatic(TransferBufferManagerKey()); + DynamicTransferBufferManagerEntry* tbme = DynamicTransferBufferManagerEntry::instantiate(allocator_, max_buf_size_); if (tbme == NULL) { - DynamicTransferBufferManagerEntry* dyn = - DynamicTransferBufferManagerEntry::instantiate(allocator_, max_buf_size_); - tbme = dyn; - if (dyn == NULL) - { - return NULL; // Epic fail. - } - dynamic_buffers_.insert(dyn); - UAVCAN_TRACE("TransferBufferManager", "Dynamic buffer created [st=%u, dyn=%u], %s", - getNumStaticBuffers(), getNumDynamicBuffers(), key.toString().c_str()); - } - else - { - UAVCAN_TRACE("TransferBufferManager", "Static buffer created [st=%u, dyn=%u], %s", - getNumStaticBuffers(), getNumDynamicBuffers(), key.toString().c_str()); + return NULL; // Epic fail. } - if (tbme) + dynamic_buffers_.insert(tbme); + + UAVCAN_TRACE("TransferBufferManager", "Buffer created [num=%u], %s", getNumBuffers(), key.toString().c_str()); + + if (tbme != NULL) { UAVCAN_ASSERT(tbme->isEmpty()); tbme->reset(key); @@ -460,19 +362,10 @@ void TransferBufferManagerImpl::remove(const TransferBufferManagerKey& key) { UAVCAN_ASSERT(!key.isEmpty()); - TransferBufferManagerEntry* const tbme = findFirstStatic(key); - if (tbme) - { - UAVCAN_TRACE("TransferBufferManager", "Static buffer deleted, %s", key.toString().c_str()); - tbme->reset(); - optimizeStorage(); - return; - } - DynamicTransferBufferManagerEntry* dyn = findFirstDynamic(key); - if (dyn) + if (dyn != NULL) { - UAVCAN_TRACE("TransferBufferManager", "Dynamic buffer deleted, %s", key.toString().c_str()); + UAVCAN_TRACE("TransferBufferManager", "Buffer deleted, %s", key.toString().c_str()); dynamic_buffers_.remove(dyn); DynamicTransferBufferManagerEntry::destroy(dyn, allocator_); } @@ -480,30 +373,12 @@ void TransferBufferManagerImpl::remove(const TransferBufferManagerKey& key) bool TransferBufferManagerImpl::isEmpty() const { - return (getNumStaticBuffers() == 0) && (getNumDynamicBuffers() == 0); + return getNumBuffers() == 0; } -unsigned TransferBufferManagerImpl::getNumDynamicBuffers() const +unsigned TransferBufferManagerImpl::getNumBuffers() const { return dynamic_buffers_.getLength(); } -unsigned TransferBufferManagerImpl::getNumStaticBuffers() const -{ - unsigned res = 0; - for (unsigned i = 0; true; i++) - { - StaticTransferBufferManagerEntryImpl* const sb = getStaticByIndex(uint16_t(i)); - if (sb == NULL) - { - break; - } - if (!sb->isEmpty()) - { - res++; - } - } - return res; -} - } diff --git a/libuavcan/test/node/test_node.hpp b/libuavcan/test/node/test_node.hpp index f02ab6b552..1549f5bb45 100644 --- a/libuavcan/test/node/test_node.hpp +++ b/libuavcan/test/node/test_node.hpp @@ -20,7 +20,7 @@ struct TestNode : public uavcan::INode { uavcan::PoolAllocator pool; - uavcan::OutgoingTransferRegistry<8> otr; + uavcan::OutgoingTransferRegistry otr; uavcan::Scheduler scheduler; uint64_t internal_failure_count; diff --git a/libuavcan/test/protocol/dynamic_node_id_server/distributed/server.cpp b/libuavcan/test/protocol/dynamic_node_id_server/distributed/server.cpp index d5cbb4b626..7d7d9a3913 100644 --- a/libuavcan/test/protocol/dynamic_node_id_server/distributed/server.cpp +++ b/libuavcan/test/protocol/dynamic_node_id_server/distributed/server.cpp @@ -190,6 +190,5 @@ TEST(dynamic_node_id_server, ObjectSizes) std::cout << "ServiceServer: " << sizeof(ServiceServer) << std::endl; std::cout << "ServiceClient: " << sizeof(ServiceClient) << std::endl; std::cout << "ServiceServer: " << sizeof(ServiceServer) << std::endl; - std::cout << "ServiceClient:" - << sizeof(ServiceClient&), 5>) << std::endl; + std::cout << "ServiceClient: " << sizeof(ServiceClient) << std::endl; } diff --git a/libuavcan/test/protocol/dynamic_node_id_server/node_discoverer.cpp b/libuavcan/test/protocol/dynamic_node_id_server/node_discoverer.cpp index 288d299b6d..eb3fe234b4 100644 --- a/libuavcan/test/protocol/dynamic_node_id_server/node_discoverer.cpp +++ b/libuavcan/test/protocol/dynamic_node_id_server/node_discoverer.cpp @@ -281,7 +281,5 @@ TEST(dynamic_node_id_server_NodeDiscoverer, Sizes) std::cout << "BitSet: " << sizeof(BitSet) << std::endl; std::cout << "ServiceClient: " << sizeof(ServiceClient) << std::endl; std::cout << "protocol::GetNodeInfo::Response: " << sizeof(protocol::GetNodeInfo::Response) << std::endl; - std::cout << "Subscriber: " - << sizeof(Subscriber&), 64, 0>) << std::endl; + std::cout << "Subscriber: " << sizeof(Subscriber) << std::endl; } diff --git a/libuavcan/test/transport/dispatcher.cpp b/libuavcan/test/transport/dispatcher.cpp index d9e2d937d4..e2daa02f63 100644 --- a/libuavcan/test/transport/dispatcher.cpp +++ b/libuavcan/test/transport/dispatcher.cpp @@ -57,7 +57,7 @@ TEST(Dispatcher, Reception) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry<8> out_trans_reg(pool); + uavcan::OutgoingTransferRegistry out_trans_reg(pool); uavcan::Dispatcher dispatcher(driver, pool, clockmock, out_trans_reg); ASSERT_TRUE(dispatcher.setNodeID(SELF_NODE_ID)); // Can be set only once @@ -86,7 +86,7 @@ TEST(Dispatcher, Reception) makeDataType(uavcan::DataTypeKindService, 1) }; - typedef TestListener<512, 2, 2> Subscriber; + typedef TestListener<512> Subscriber; typedef std::auto_ptr SubscriberPtr; static const int NUM_SUBSCRIBERS = 6; SubscriberPtr subscribers[NUM_SUBSCRIBERS] = @@ -255,7 +255,7 @@ TEST(Dispatcher, Transmission) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry<8> out_trans_reg(pool); + uavcan::OutgoingTransferRegistry out_trans_reg(pool); uavcan::Dispatcher dispatcher(driver, pool, clockmock, out_trans_reg); ASSERT_TRUE(dispatcher.setNodeID(SELF_NODE_ID)); // Can be set only once @@ -319,7 +319,7 @@ TEST(Dispatcher, Spin) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry<8> out_trans_reg(poolmgr); + uavcan::OutgoingTransferRegistry out_trans_reg(poolmgr); uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock, out_trans_reg); ASSERT_TRUE(dispatcher.setNodeID(SELF_NODE_ID)); // Can be set only once @@ -365,7 +365,7 @@ TEST(Dispatcher, Loopback) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry<8> out_trans_reg(poolmgr); + uavcan::OutgoingTransferRegistry out_trans_reg(poolmgr); uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock, out_trans_reg); ASSERT_TRUE(dispatcher.setNodeID(SELF_NODE_ID)); diff --git a/libuavcan/test/transport/incoming_transfer.cpp b/libuavcan/test/transport/incoming_transfer.cpp index 2264d848fc..8c52dab8d8 100644 --- a/libuavcan/test/transport/incoming_transfer.cpp +++ b/libuavcan/test/transport/incoming_transfer.cpp @@ -73,7 +73,7 @@ TEST(MultiFrameIncomingTransfer, Basic) using uavcan::MultiFrameIncomingTransfer; NullAllocator poolmgr; // We don't need dynamic memory - uavcan::TransferBufferManager<256, 1> bufmgr(poolmgr); + uavcan::TransferBufferManager<256> bufmgr(poolmgr); const RxFrame frame = makeFrame(); uavcan::TransferBufferManagerKey bufmgr_key(frame.getSrcNodeID(), frame.getTransferType()); diff --git a/libuavcan/test/transport/outgoing_transfer_registry.cpp b/libuavcan/test/transport/outgoing_transfer_registry.cpp index aaffb357a0..9ad8b334d1 100644 --- a/libuavcan/test/transport/outgoing_transfer_registry.cpp +++ b/libuavcan/test/transport/outgoing_transfer_registry.cpp @@ -13,7 +13,7 @@ TEST(OutgoingTransferRegistry, Basic) { using uavcan::OutgoingTransferRegistryKey; NullAllocator poolmgr; // Empty - uavcan::OutgoingTransferRegistry<4> otr(poolmgr); + uavcan::OutgoingTransferRegistry otr(poolmgr); otr.cleanup(tsMono(1000)); diff --git a/libuavcan/test/transport/transfer_buffer.cpp b/libuavcan/test/transport/transfer_buffer.cpp index 719bf5d08c..96c9fe77c5 100644 --- a/libuavcan/test/transport/transfer_buffer.cpp +++ b/libuavcan/test/transport/transfer_buffer.cpp @@ -229,10 +229,10 @@ TEST(TransferBufferManager, Basic) using uavcan::TransferBufferManagerKey; using uavcan::ITransferBuffer; - static const int POOL_BLOCKS = 6; + static const int POOL_BLOCKS = 8; uavcan::PoolAllocator pool; - typedef TransferBufferManager TransferBufferManagerType; + typedef TransferBufferManager TransferBufferManagerType; std::auto_ptr mgr(new TransferBufferManagerType(pool)); // Empty @@ -250,40 +250,32 @@ TEST(TransferBufferManager, Basic) TransferBufferManagerKey(64, uavcan::TransferTypeMessageBroadcast) }; - // Static 0 ASSERT_TRUE((tbb = mgr->create(keys[0]))); ASSERT_EQ(MGR_MAX_BUFFER_SIZE, fillTestData(MGR_TEST_DATA[0], tbb)); - ASSERT_EQ(1, mgr->getNumStaticBuffers()); + ASSERT_EQ(1, mgr->getNumBuffers()); - // Static 1 ASSERT_TRUE((tbb = mgr->create(keys[1]))); ASSERT_EQ(MGR_MAX_BUFFER_SIZE, fillTestData(MGR_TEST_DATA[1], tbb)); - ASSERT_EQ(2, mgr->getNumStaticBuffers()); - ASSERT_EQ(0, mgr->getNumDynamicBuffers()); + ASSERT_EQ(0, mgr->getNumBuffers()); ASSERT_EQ(0, pool.getNumUsedBlocks()); - // Dynamic 0 ASSERT_TRUE((tbb = mgr->create(keys[2]))); ASSERT_EQ(1, pool.getNumUsedBlocks()); // Empty dynamic buffer occupies one block ASSERT_EQ(MGR_MAX_BUFFER_SIZE, fillTestData(MGR_TEST_DATA[2], tbb)); - ASSERT_EQ(2, mgr->getNumStaticBuffers()); - ASSERT_EQ(1, mgr->getNumDynamicBuffers()); + ASSERT_EQ(1, mgr->getNumBuffers()); ASSERT_LT(1, pool.getNumUsedBlocks()); std::cout << "TransferBufferManager - Basic: Pool usage: " << pool.getNumUsedBlocks() << std::endl; - // Dynamic 2 ASSERT_TRUE((tbb = mgr->create(keys[3]))); ASSERT_LT(0, pool.getNumUsedBlocks()); ASSERT_LT(0, fillTestData(MGR_TEST_DATA[3], tbb)); - ASSERT_EQ(2, mgr->getNumStaticBuffers()); - ASSERT_EQ(2, mgr->getNumDynamicBuffers()); + ASSERT_EQ(2, mgr->getNumBuffers()); // Dynamic 3 - will fail due to OOM ASSERT_FALSE((tbb = mgr->create(keys[4]))); - ASSERT_EQ(2, mgr->getNumStaticBuffers()); - ASSERT_EQ(2, mgr->getNumDynamicBuffers()); + ASSERT_EQ(2, mgr->getNumBuffers()); // Making sure all buffers contain proper data ASSERT_TRUE((tbb = mgr->access(keys[0]))); @@ -298,18 +290,14 @@ TEST(TransferBufferManager, Basic) ASSERT_TRUE((tbb = mgr->access(keys[3]))); ASSERT_TRUE(matchAgainst(MGR_TEST_DATA[3], *tbb)); - // Freeing one static buffer; one dynamic must migrate mgr->remove(keys[1]); ASSERT_FALSE(mgr->access(keys[1])); - ASSERT_EQ(2, mgr->getNumStaticBuffers()); - ASSERT_EQ(1, mgr->getNumDynamicBuffers()); // One migrated to the static + ASSERT_EQ(1, mgr->getNumBuffers()); ASSERT_LT(0, pool.getNumFreeBlocks()); - // Removing NodeID 0; one dynamic must migrate mgr->remove(keys[0]); ASSERT_FALSE(mgr->access(keys[0])); - ASSERT_EQ(2, mgr->getNumStaticBuffers()); - ASSERT_EQ(0, mgr->getNumDynamicBuffers()); + ASSERT_EQ(0, mgr->getNumBuffers()); // At this time we have the following NodeID: 2, 127 ASSERT_TRUE((tbb = mgr->access(keys[2]))); @@ -336,7 +324,7 @@ TEST(TransferBufferManager, Basic) TEST(TransferBufferManager, EmptySpecialization) { - uavcan::TransferBufferManager<0, 0> mgr; + uavcan::TransferBufferManager<0> mgr; (void)mgr; ASSERT_GE(sizeof(void*), sizeof(mgr)); } diff --git a/libuavcan/test/transport/transfer_listener.cpp b/libuavcan/test/transport/transfer_listener.cpp index 5a6f2726ae..d5e07596e0 100644 --- a/libuavcan/test/transport/transfer_listener.cpp +++ b/libuavcan/test/transport/transfer_listener.cpp @@ -38,7 +38,7 @@ TEST(TransferListener, BasicMFT) uavcan::PoolAllocator pool; uavcan::TransferPerfCounter perf; - TestListener<256, 1, 1> subscriber(perf, type, pool); + TestListener<256> subscriber(perf, type, pool); /* * Test data @@ -95,7 +95,7 @@ TEST(TransferListener, CrcFailure) NullAllocator poolmgr; // No dynamic memory uavcan::TransferPerfCounter perf; - TestListener<256, 2, 2> subscriber(perf, type, poolmgr); // Static buffer only, 2 entries + TestListener<256> subscriber(perf, type, poolmgr); // Static buffer only, 2 entries /* * Generating transfers with damaged payload (CRC is not valid) @@ -138,7 +138,7 @@ TEST(TransferListener, BasicSFT) NullAllocator poolmgr; // No dynamic memory. At all. uavcan::TransferPerfCounter perf; - TestListener<0, 0, 5> subscriber(perf, type, poolmgr); // Max buf size is 0, i.e. SFT-only + TestListener<0> subscriber(perf, type, poolmgr); // Max buf size is 0, i.e. SFT-only TransferListenerEmulator emulator(subscriber, type); const Transfer transfers[] = @@ -174,7 +174,7 @@ TEST(TransferListener, Cleanup) NullAllocator poolmgr; // No dynamic memory uavcan::TransferPerfCounter perf; - TestListener<256, 1, 2> subscriber(perf, type, poolmgr); // Static buffer only, 1 entry + TestListener<256> subscriber(perf, type, poolmgr); // Static buffer only, 1 entry /* * Generating transfers @@ -229,7 +229,7 @@ TEST(TransferListener, AnonymousTransfers) NullAllocator poolmgr; uavcan::TransferPerfCounter perf; - TestListener<0, 0, 0> subscriber(perf, type, poolmgr); + TestListener<0> subscriber(perf, type, poolmgr); TransferListenerEmulator emulator(subscriber, type); const Transfer transfers[] = @@ -261,5 +261,5 @@ TEST(TransferListener, Sizes) { using namespace uavcan; - std::cout << "sizeof(TransferListener<64, 1, 2>): " << sizeof(TransferListener<64, 1, 2>) << std::endl; + std::cout << "sizeof(TransferListener<64>): " << sizeof(TransferListener<64>) << std::endl; } diff --git a/libuavcan/test/transport/transfer_receiver.cpp b/libuavcan/test/transport/transfer_receiver.cpp index fad00f45b4..f58f21a3d6 100644 --- a/libuavcan/test/transport/transfer_receiver.cpp +++ b/libuavcan/test/transport/transfer_receiver.cpp @@ -76,7 +76,7 @@ struct Context { NullAllocator pool; // We don't need dynamic memory for this test uavcan::TransferReceiver receiver; // Must be default constructible and copyable - uavcan::TransferBufferManager bufmgr; + uavcan::TransferBufferManager bufmgr; Context() : bufmgr(pool) diff --git a/libuavcan/test/transport/transfer_sender.cpp b/libuavcan/test/transport/transfer_sender.cpp index 36a40b6b2b..62205c68c1 100644 --- a/libuavcan/test/transport/transfer_sender.cpp +++ b/libuavcan/test/transport/transfer_sender.cpp @@ -34,7 +34,7 @@ TEST(TransferSender, Basic) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry<8> out_trans_reg(poolmgr); + uavcan::OutgoingTransferRegistry out_trans_reg(poolmgr); static const uavcan::NodeID TX_NODE_ID(64); static const uavcan::NodeID RX_NODE_ID(65); @@ -123,9 +123,9 @@ TEST(TransferSender, Basic) } } - TestListener<512, 2, 2> sub_msg(dispatcher_rx.getTransferPerfCounter(), TYPES[0], poolmgr); - TestListener<512, 2, 2> sub_srv_req(dispatcher_rx.getTransferPerfCounter(), TYPES[1], poolmgr); - TestListener<512, 2, 2> sub_srv_resp(dispatcher_rx.getTransferPerfCounter(), TYPES[1], poolmgr); + TestListener<512> sub_msg(dispatcher_rx.getTransferPerfCounter(), TYPES[0], poolmgr); + TestListener<512> sub_srv_req(dispatcher_rx.getTransferPerfCounter(), TYPES[1], poolmgr); + TestListener<512> sub_srv_resp(dispatcher_rx.getTransferPerfCounter(), TYPES[1], poolmgr); dispatcher_rx.registerMessageListener(&sub_msg); dispatcher_rx.registerServiceRequestListener(&sub_srv_req); @@ -195,7 +195,7 @@ TEST(TransferSender, Loopback) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry<8> out_trans_reg(poolmgr); + uavcan::OutgoingTransferRegistry out_trans_reg(poolmgr); static const uavcan::NodeID TX_NODE_ID(64); uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock, out_trans_reg); @@ -236,7 +236,7 @@ TEST(TransferSender, PassiveMode) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry<8> out_trans_reg(poolmgr); + uavcan::OutgoingTransferRegistry out_trans_reg(poolmgr); uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock, out_trans_reg); diff --git a/libuavcan/test/transport/transfer_test_helpers.cpp b/libuavcan/test/transport/transfer_test_helpers.cpp index c4dc7d7606..f7de3410f1 100644 --- a/libuavcan/test/transport/transfer_test_helpers.cpp +++ b/libuavcan/test/transport/transfer_test_helpers.cpp @@ -11,7 +11,7 @@ TEST(TransferTestHelpers, Transfer) { uavcan::PoolAllocator pool; - uavcan::TransferBufferManager<128, 1> mgr(pool); + uavcan::TransferBufferManager<128> mgr(pool); uavcan::TransferBufferAccessor tba(mgr, uavcan::TransferBufferManagerKey(0, uavcan::TransferTypeMessageBroadcast)); uavcan::RxFrame frame(uavcan::Frame(123, uavcan::TransferTypeMessageBroadcast, 1, 0, 0), diff --git a/libuavcan/test/transport/transfer_test_helpers.hpp b/libuavcan/test/transport/transfer_test_helpers.hpp index d423f142e8..d1b4f76375 100644 --- a/libuavcan/test/transport/transfer_test_helpers.hpp +++ b/libuavcan/test/transport/transfer_test_helpers.hpp @@ -117,10 +117,10 @@ struct Transfer * In reality, uavcan::TransferListener should accept only specific transfer types * which are dispatched/filtered by uavcan::Dispatcher. */ -template -class TestListener : public uavcan::TransferListener +template +class TestListener : public uavcan::TransferListener { - typedef uavcan::TransferListener Base; + typedef uavcan::TransferListener Base; std::queue transfers_; diff --git a/libuavcan/test/util/map.cpp b/libuavcan/test/util/map.cpp index 60386058ec..947bb52010 100644 --- a/libuavcan/test/util/map.cpp +++ b/libuavcan/test/util/map.cpp @@ -2,6 +2,11 @@ * Copyright (C) 2014 Pavel Kirienko */ +#if __GNUC__ +// We need auto_ptr for compatibility reasons +# pragma GCC diagnostic ignored "-Wdeprecated-declarations" +#endif + #include #include #include @@ -46,7 +51,7 @@ TEST(Map, Basic) static const int POOL_BLOCKS = 3; uavcan::PoolAllocator pool; - typedef Map MapType; + typedef Map MapType; std::auto_ptr map(new MapType(pool)); // Empty @@ -57,25 +62,23 @@ TEST(Map, Basic) ASSERT_FALSE(map->getByIndex(1)); ASSERT_FALSE(map->getByIndex(10000)); - // Static insertion + // Insertion ASSERT_EQ("a", *map->insert("1", "a")); ASSERT_EQ("b", *map->insert("2", "b")); - ASSERT_EQ(0, pool.getNumUsedBlocks()); - ASSERT_EQ(2, map->getNumStaticPairs()); - ASSERT_EQ(0, map->getNumDynamicPairs()); + ASSERT_EQ(1, pool.getNumUsedBlocks()); + ASSERT_EQ(2, map->getSize()); // Ordering ASSERT_TRUE(map->getByIndex(0)->match("1")); ASSERT_TRUE(map->getByIndex(1)->match("2")); - // Dynamic insertion + // Insertion ASSERT_EQ("c", *map->insert("3", "c")); ASSERT_EQ(1, pool.getNumUsedBlocks()); ASSERT_EQ("d", *map->insert("4", "d")); - ASSERT_EQ(1, pool.getNumUsedBlocks()); // Assuming that at least 2 items fit one block - ASSERT_EQ(2, map->getNumStaticPairs()); - ASSERT_EQ(2, map->getNumDynamicPairs()); + ASSERT_EQ(2, pool.getNumUsedBlocks()); // Assuming that at least 2 items fit one block + ASSERT_EQ(4, map->getSize()); // Making sure everything is here ASSERT_EQ("a", *map->access("1")); @@ -116,12 +119,11 @@ TEST(Map, Basic) ASSERT_EQ("4", *map->find(ValueFindPredicate("D"))); ASSERT_FALSE(map->find(KeyFindPredicate("nonexistent_value"))); - // Removing one static + // Removing one map->remove("1"); // One of dynamics now migrates to the static storage map->remove("foo"); // There's no such thing anyway - ASSERT_EQ(1, pool.getNumUsedBlocks()); - ASSERT_EQ(2, map->getNumStaticPairs()); - ASSERT_EQ(1, map->getNumDynamicPairs()); + ASSERT_EQ(2, pool.getNumUsedBlocks()); + ASSERT_EQ(3, map->getSize()); ASSERT_FALSE(map->access("1")); ASSERT_EQ("B", *map->access("2")); @@ -135,9 +137,8 @@ TEST(Map, Basic) // Removing another static map->remove("2"); - ASSERT_EQ(2, map->getNumStaticPairs()); - ASSERT_EQ(0, map->getNumDynamicPairs()); - ASSERT_EQ(0, pool.getNumUsedBlocks()); // No dynamic entries left + ASSERT_EQ(1, map->getSize()); + ASSERT_EQ(1, pool.getNumUsedBlocks()); // No dynamic entries left ASSERT_FALSE(map->access("1")); ASSERT_FALSE(map->access("2")); @@ -172,13 +173,7 @@ TEST(Map, Basic) ASSERT_FALSE(map->access("value")); // Removing odd values - nearly half of them - ASSERT_EQ(2, map->getNumStaticPairs()); - const unsigned num_dynamics_old = map->getNumDynamicPairs(); map->removeAllWhere(oddValuePredicate); - ASSERT_EQ(2, map->getNumStaticPairs()); - const unsigned num_dynamics_new = map->getNumDynamicPairs(); - std::cout << "Num of dynamic pairs reduced from " << num_dynamics_old << " to " << num_dynamics_new << std::endl; - ASSERT_LT(num_dynamics_new, num_dynamics_old); // Making sure there's no odd values left for (unsigned kv_int = 0; kv_int <= max_key_integer; kv_int++) @@ -220,8 +215,7 @@ TEST(Map, NoStatic) ASSERT_EQ("a", *map->insert("1", "a")); ASSERT_EQ("b", *map->insert("2", "b")); ASSERT_EQ(1, pool.getNumUsedBlocks()); - ASSERT_EQ(0, map->getNumStaticPairs()); - ASSERT_EQ(2, map->getNumDynamicPairs()); + ASSERT_EQ(2, map->getSize()); // Ordering ASSERT_TRUE(map->getByIndex(0)->match("1")); @@ -238,7 +232,7 @@ TEST(Map, PrimitiveKey) static const int POOL_BLOCKS = 3; uavcan::PoolAllocator pool; - typedef Map MapType; + typedef Map MapType; std::auto_ptr map(new MapType(pool)); // Empty diff --git a/libuavcan/test/util/multiset.cpp b/libuavcan/test/util/multiset.cpp index 505deb8ddc..bfd6384049 100644 --- a/libuavcan/test/util/multiset.cpp +++ b/libuavcan/test/util/multiset.cpp @@ -71,7 +71,7 @@ TEST(Multiset, Basic) static const int POOL_BLOCKS = 3; uavcan::PoolAllocator pool; - typedef Multiset MultisetType; + typedef Multiset MultisetType; std::auto_ptr mset(new MultisetType(pool)); typedef SummationOperator StringConcatenationOperator; @@ -86,9 +86,8 @@ TEST(Multiset, Basic) // Static addion ASSERT_EQ("1", *mset->emplace("1")); ASSERT_EQ("2", *mset->emplace("2")); - ASSERT_EQ(0, pool.getNumUsedBlocks()); - ASSERT_EQ(2, mset->getNumStaticItems()); - ASSERT_EQ(0, mset->getNumDynamicItems()); + ASSERT_LE(1, pool.getNumUsedBlocks()); // One or more + ASSERT_EQ(2, mset->getSize()); // Ordering ASSERT_TRUE(*mset->getByIndex(0) == "1"); @@ -103,12 +102,11 @@ TEST(Multiset, Basic) // Dynamic addition ASSERT_EQ("3", *mset->emplace("3")); ASSERT_EQ("3", *mset->getByIndex(2)); - ASSERT_EQ(1, pool.getNumUsedBlocks()); + ASSERT_LE(1, pool.getNumUsedBlocks()); // One or more ASSERT_EQ("4", *mset->emplace("4")); ASSERT_LE(1, pool.getNumUsedBlocks()); // One or more - ASSERT_EQ(2, mset->getNumStaticItems()); - ASSERT_EQ(2, mset->getNumDynamicItems()); + ASSERT_EQ(4, mset->getSize()); // Making sure everything is here ASSERT_EQ("1", *mset->getByIndex(0)); @@ -117,9 +115,6 @@ TEST(Multiset, Basic) ASSERT_FALSE(mset->getByIndex(100)); ASSERT_FALSE(mset->getByIndex(4)); - const std::string data_at_pos2 = *mset->getByIndex(2); - const std::string data_at_pos3 = *mset->getByIndex(3); - // Finding some items ASSERT_EQ("1", *mset->find(FindPredicate("1"))); ASSERT_EQ("2", *mset->find(FindPredicate("2"))); @@ -134,23 +129,10 @@ TEST(Multiset, Basic) ASSERT_EQ(4, op.accumulator.size()); } - // Removing one static; ordering will be preserved + // Removing some mset->removeFirst("1"); mset->removeFirst("foo"); // There's no such thing anyway - ASSERT_LE(1, pool.getNumUsedBlocks()); - ASSERT_EQ(1, mset->getNumStaticItems()); - ASSERT_EQ(2, mset->getNumDynamicItems()); // This container does not move items - - // Ordering has not changed - ASSERT_EQ("2", *mset->getByIndex(0)); // Entry "1" was here - ASSERT_EQ(data_at_pos2, *mset->getByIndex(1)); - ASSERT_EQ(data_at_pos3, *mset->getByIndex(2)); - - // Removing another static mset->removeFirst("2"); - ASSERT_EQ(0, mset->getNumStaticItems()); - ASSERT_EQ(2, mset->getNumDynamicItems()); - ASSERT_LE(1, pool.getNumUsedBlocks()); // Adding some new items unsigned max_value_integer = 0; @@ -219,7 +201,7 @@ TEST(Multiset, PrimitiveKey) static const int POOL_BLOCKS = 3; uavcan::PoolAllocator pool; - typedef Multiset MultisetType; + typedef Multiset MultisetType; std::auto_ptr mset(new MultisetType(pool)); // Empty @@ -269,7 +251,7 @@ TEST(Multiset, NoncopyableWithCounter) static const int POOL_BLOCKS = 3; uavcan::PoolAllocator pool; - typedef Multiset MultisetType; + typedef Multiset MultisetType; std::auto_ptr mset(new MultisetType(pool)); ASSERT_EQ(0, NoncopyableWithCounter::num_objects); From dc5fdbb1cc245a3779b1db48c4df9635fe496ac4 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Wed, 14 Oct 2015 08:38:04 +0300 Subject: [PATCH 02/25] Unit tests no longer segfault, 26 are failing --- libuavcan/test/transport/outgoing_transfer_registry.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libuavcan/test/transport/outgoing_transfer_registry.cpp b/libuavcan/test/transport/outgoing_transfer_registry.cpp index 9ad8b334d1..50eb67eb9a 100644 --- a/libuavcan/test/transport/outgoing_transfer_registry.cpp +++ b/libuavcan/test/transport/outgoing_transfer_registry.cpp @@ -12,7 +12,7 @@ TEST(OutgoingTransferRegistry, Basic) { using uavcan::OutgoingTransferRegistryKey; - NullAllocator poolmgr; // Empty + uavcan::PoolAllocator poolmgr; uavcan::OutgoingTransferRegistry otr(poolmgr); otr.cleanup(tsMono(1000)); From 3e6102d4796de26cf0bc77a729c1d47ee3eec003 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Wed, 14 Oct 2015 08:51:20 +0300 Subject: [PATCH 03/25] Map<> and Multiset<> fixed, 22 tests to go --- libuavcan/test/util/map.cpp | 51 +++----------------------------- libuavcan/test/util/multiset.cpp | 16 ---------- 2 files changed, 4 insertions(+), 63 deletions(-) diff --git a/libuavcan/test/util/map.cpp b/libuavcan/test/util/map.cpp index 947bb52010..0fa31f03f1 100644 --- a/libuavcan/test/util/map.cpp +++ b/libuavcan/test/util/map.cpp @@ -87,14 +87,6 @@ TEST(Map, Basic) ASSERT_EQ("d", *map->access("4")); ASSERT_FALSE(map->access("hi")); - // Ordering - ASSERT_TRUE(map->getByIndex(0)->match("1")); - ASSERT_TRUE(map->getByIndex(1)->match("2")); - ASSERT_TRUE(map->getByIndex(2)->match("3")); - ASSERT_TRUE(map->getByIndex(3)->match("4")); - ASSERT_FALSE(map->getByIndex(4)); - ASSERT_FALSE(map->getByIndex(1000)); - // Modifying existing entries *map->access("1") = "A"; *map->access("2") = "B"; @@ -130,22 +122,17 @@ TEST(Map, Basic) ASSERT_EQ("C", *map->access("3")); ASSERT_EQ("D", *map->access("4")); - // Ordering has not changed - first dynamic entry has moved to the first static slot - ASSERT_TRUE(map->getByIndex(0)->match("3")); - ASSERT_TRUE(map->getByIndex(1)->match("2")); - ASSERT_TRUE(map->getByIndex(2)->match("4")); - - // Removing another static + // Removing another map->remove("2"); - ASSERT_EQ(1, map->getSize()); - ASSERT_EQ(1, pool.getNumUsedBlocks()); // No dynamic entries left + ASSERT_EQ(2, map->getSize()); + ASSERT_EQ(2, pool.getNumUsedBlocks()); ASSERT_FALSE(map->access("1")); ASSERT_FALSE(map->access("2")); ASSERT_EQ("C", *map->access("3")); ASSERT_EQ("D", *map->access("4")); - // Adding some new dynamics + // Adding some new unsigned max_key_integer = 0; for (int i = 0; i < 100; i++) { @@ -195,36 +182,6 @@ TEST(Map, Basic) } -TEST(Map, NoStatic) -{ - using uavcan::Map; - - static const int POOL_BLOCKS = 3; - uavcan::PoolAllocator pool; - - typedef Map MapType; - std::auto_ptr map(new MapType(pool)); - - // Empty - ASSERT_FALSE(map->access("hi")); - map->remove("foo"); - ASSERT_EQ(0, pool.getNumUsedBlocks()); - ASSERT_FALSE(map->getByIndex(0)); - - // Insertion - ASSERT_EQ("a", *map->insert("1", "a")); - ASSERT_EQ("b", *map->insert("2", "b")); - ASSERT_EQ(1, pool.getNumUsedBlocks()); - ASSERT_EQ(2, map->getSize()); - - // Ordering - ASSERT_TRUE(map->getByIndex(0)->match("1")); - ASSERT_TRUE(map->getByIndex(1)->match("2")); - ASSERT_FALSE(map->getByIndex(3)); - ASSERT_FALSE(map->getByIndex(1000)); -} - - TEST(Map, PrimitiveKey) { using uavcan::Map; diff --git a/libuavcan/test/util/multiset.cpp b/libuavcan/test/util/multiset.cpp index bfd6384049..715d99922d 100644 --- a/libuavcan/test/util/multiset.cpp +++ b/libuavcan/test/util/multiset.cpp @@ -108,10 +108,6 @@ TEST(Multiset, Basic) ASSERT_LE(1, pool.getNumUsedBlocks()); // One or more ASSERT_EQ(4, mset->getSize()); - // Making sure everything is here - ASSERT_EQ("1", *mset->getByIndex(0)); - ASSERT_EQ("2", *mset->getByIndex(1)); - // 2 and 3 are not tested because their placement depends on number of items per dynamic block ASSERT_FALSE(mset->getByIndex(100)); ASSERT_FALSE(mset->getByIndex(4)); @@ -219,16 +215,6 @@ TEST(Multiset, PrimitiveKey) ASSERT_EQ(4, *mset->emplace(4)); ASSERT_EQ(4, mset->getSize()); -#if UAVCAN_CPP_VERSION >= UAVCAN_CPP11 - // Only C++11 because C++03 uses one entry per pool block which breaks ordering - ASSERT_EQ(1, *mset->getByIndex(0)); - ASSERT_EQ(2, *mset->getByIndex(1)); - ASSERT_EQ(3, *mset->getByIndex(2)); - ASSERT_EQ(4, *mset->getByIndex(3)); - ASSERT_FALSE(mset->getByIndex(5)); - ASSERT_FALSE(mset->getByIndex(1000)); -#endif - // Summation and clearing { SummationOperator summation_operator; @@ -268,11 +254,9 @@ TEST(Multiset, NoncopyableWithCounter) mset->removeFirst(NoncopyableWithCounter(0)); ASSERT_EQ(4, NoncopyableWithCounter::num_objects); - ASSERT_EQ(123, mset->getByIndex(0)->value); mset->removeFirstWhere(&NoncopyableWithCounter::isNegative); ASSERT_EQ(3, NoncopyableWithCounter::num_objects); - ASSERT_EQ(456, mset->getByIndex(1)->value); // -456 is now removed mset->removeAllWhere(&NoncopyableWithCounter::isNegative); ASSERT_EQ(2, NoncopyableWithCounter::num_objects); // Only 1 and 2 are left From 706198fa38d685e819a9f4667cd22f70501a745b Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Wed, 14 Oct 2015 09:06:15 +0300 Subject: [PATCH 04/25] Transport test fixes, 10 to go --- libuavcan/test/transport/dispatcher.cpp | 2 +- libuavcan/test/transport/incoming_transfer.cpp | 2 +- libuavcan/test/transport/transfer_receiver.cpp | 6 ++---- libuavcan/test/transport/transfer_sender.cpp | 6 +++--- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/libuavcan/test/transport/dispatcher.cpp b/libuavcan/test/transport/dispatcher.cpp index e2daa02f63..b33ace4931 100644 --- a/libuavcan/test/transport/dispatcher.cpp +++ b/libuavcan/test/transport/dispatcher.cpp @@ -52,7 +52,7 @@ static const uavcan::NodeID SELF_NODE_ID(64); TEST(Dispatcher, Reception) { - uavcan::PoolAllocator pool; + uavcan::PoolAllocator pool; SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); diff --git a/libuavcan/test/transport/incoming_transfer.cpp b/libuavcan/test/transport/incoming_transfer.cpp index 8c52dab8d8..735d0027b6 100644 --- a/libuavcan/test/transport/incoming_transfer.cpp +++ b/libuavcan/test/transport/incoming_transfer.cpp @@ -72,7 +72,7 @@ TEST(MultiFrameIncomingTransfer, Basic) using uavcan::RxFrame; using uavcan::MultiFrameIncomingTransfer; - NullAllocator poolmgr; // We don't need dynamic memory + uavcan::PoolAllocator poolmgr; uavcan::TransferBufferManager<256> bufmgr(poolmgr); const RxFrame frame = makeFrame(); diff --git a/libuavcan/test/transport/transfer_receiver.cpp b/libuavcan/test/transport/transfer_receiver.cpp index f58f21a3d6..878a41a70f 100644 --- a/libuavcan/test/transport/transfer_receiver.cpp +++ b/libuavcan/test/transport/transfer_receiver.cpp @@ -74,15 +74,13 @@ const uavcan::TransferBufferManagerKey RxFrameGenerator::DEFAULT_KEY(42, uavcan: template struct Context { - NullAllocator pool; // We don't need dynamic memory for this test + uavcan::PoolAllocator pool; uavcan::TransferReceiver receiver; // Must be default constructible and copyable uavcan::TransferBufferManager bufmgr; Context() : bufmgr(pool) - { - assert(pool.allocate(1) == NULL); - } + { } ~Context() { diff --git a/libuavcan/test/transport/transfer_sender.cpp b/libuavcan/test/transport/transfer_sender.cpp index 62205c68c1..ff5595e19a 100644 --- a/libuavcan/test/transport/transfer_sender.cpp +++ b/libuavcan/test/transport/transfer_sender.cpp @@ -29,7 +29,7 @@ static int sendOne(uavcan::TransferSender& sender, const std::string& data, TEST(TransferSender, Basic) { - NullAllocator poolmgr; + uavcan::PoolAllocator poolmgr; SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); @@ -190,7 +190,7 @@ struct TransferSenderTestLoopbackFrameListener : public uavcan::LoopbackFrameLis TEST(TransferSender, Loopback) { - NullAllocator poolmgr; + uavcan::PoolAllocator poolmgr; SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); @@ -231,7 +231,7 @@ TEST(TransferSender, Loopback) TEST(TransferSender, PassiveMode) { - NullAllocator poolmgr; + uavcan::PoolAllocator poolmgr; SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); From 34b32ce0c0979012aa2a86f6133321ed95b0fe84 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Wed, 14 Oct 2015 09:48:19 +0300 Subject: [PATCH 05/25] More transport test fixes, 4 to go --- .../can_acceptance_filter_configurator.cpp | 20 +++++++++--------- libuavcan/test/transport/transfer_buffer.cpp | 21 +++++++------------ .../test/transport/transfer_listener.cpp | 19 ++++++++++------- 3 files changed, 28 insertions(+), 32 deletions(-) diff --git a/libuavcan/test/transport/can_acceptance_filter_configurator.cpp b/libuavcan/test/transport/can_acceptance_filter_configurator.cpp index ab5e79aa53..ff3e647989 100644 --- a/libuavcan/test/transport/can_acceptance_filter_configurator.cpp +++ b/libuavcan/test/transport/can_acceptance_filter_configurator.cpp @@ -145,10 +145,10 @@ TEST(CanAcceptanceFilter, Basic_test) std::cout << "config.MK [" << i << "]= " << configure_array.getByIndex(i)->mask << std::endl; } - ASSERT_EQ(configure_array.getByIndex(0)->id, 0); - ASSERT_EQ(configure_array.getByIndex(0)->mask, 255); - ASSERT_EQ(configure_array.getByIndex(1)->id, 256000); - ASSERT_EQ(configure_array.getByIndex(1)->mask, 16771968); + ASSERT_EQ(configure_array.getByIndex(0)->id, 256000); + ASSERT_EQ(configure_array.getByIndex(0)->mask, 16771968); + ASSERT_EQ(configure_array.getByIndex(1)->id, 0); + ASSERT_EQ(configure_array.getByIndex(1)->mask, 255); ASSERT_EQ(configure_array.getByIndex(2)->id, 6272); ASSERT_EQ(configure_array.getByIndex(2)->mask, 32640); ASSERT_EQ(configure_array.getByIndex(3)->id, 262144); @@ -170,16 +170,16 @@ TEST(CanAcceptanceFilter, Basic_test) for (uint16_t i = 0; icreate(keys[3]))); - ASSERT_LT(0, pool.getNumUsedBlocks()); ASSERT_LT(0, fillTestData(MGR_TEST_DATA[3], tbb)); - ASSERT_EQ(2, mgr->getNumBuffers()); - - // Dynamic 3 - will fail due to OOM - ASSERT_FALSE((tbb = mgr->create(keys[4]))); - ASSERT_EQ(2, mgr->getNumBuffers()); + ASSERT_EQ(4, mgr->getNumBuffers()); // Making sure all buffers contain proper data ASSERT_TRUE((tbb = mgr->access(keys[0]))); @@ -292,12 +285,12 @@ TEST(TransferBufferManager, Basic) mgr->remove(keys[1]); ASSERT_FALSE(mgr->access(keys[1])); - ASSERT_EQ(1, mgr->getNumBuffers()); + ASSERT_EQ(3, mgr->getNumBuffers()); ASSERT_LT(0, pool.getNumFreeBlocks()); mgr->remove(keys[0]); ASSERT_FALSE(mgr->access(keys[0])); - ASSERT_EQ(0, mgr->getNumBuffers()); + ASSERT_EQ(2, mgr->getNumBuffers()); // At this time we have the following NodeID: 2, 127 ASSERT_TRUE((tbb = mgr->access(keys[2]))); diff --git a/libuavcan/test/transport/transfer_listener.cpp b/libuavcan/test/transport/transfer_listener.cpp index d5e07596e0..a4ea34b2b0 100644 --- a/libuavcan/test/transport/transfer_listener.cpp +++ b/libuavcan/test/transport/transfer_listener.cpp @@ -34,7 +34,7 @@ TEST(TransferListener, BasicMFT) { const uavcan::DataTypeDescriptor type(uavcan::DataTypeKindMessage, 123, uavcan::DataTypeSignature(123456789), "A"); - static const int NUM_POOL_BLOCKS = 12; // This number is just enough to pass the test + static const int NUM_POOL_BLOCKS = 100; uavcan::PoolAllocator pool; uavcan::TransferPerfCounter perf; @@ -93,7 +93,8 @@ TEST(TransferListener, CrcFailure) { const uavcan::DataTypeDescriptor type(uavcan::DataTypeKindMessage, 123, uavcan::DataTypeSignature(123456789), "A"); - NullAllocator poolmgr; // No dynamic memory + static const int NUM_POOL_BLOCKS = 100; + uavcan::PoolAllocator poolmgr; uavcan::TransferPerfCounter perf; TestListener<256> subscriber(perf, type, poolmgr); // Static buffer only, 2 entries @@ -136,7 +137,8 @@ TEST(TransferListener, BasicSFT) { const uavcan::DataTypeDescriptor type(uavcan::DataTypeKindMessage, 123, uavcan::DataTypeSignature(123456789), "A"); - NullAllocator poolmgr; // No dynamic memory. At all. + static const int NUM_POOL_BLOCKS = 100; + uavcan::PoolAllocator poolmgr; uavcan::TransferPerfCounter perf; TestListener<0> subscriber(perf, type, poolmgr); // Max buf size is 0, i.e. SFT-only @@ -148,7 +150,6 @@ TEST(TransferListener, BasicSFT) emulator.makeTransfer(16, uavcan::TransferTypeServiceRequest, 2, ""), emulator.makeTransfer(16, uavcan::TransferTypeServiceRequest, 3, "abc"), emulator.makeTransfer(16, uavcan::TransferTypeServiceResponse, 4, ""), - emulator.makeTransfer(16, uavcan::TransferTypeServiceResponse, 2, ""), // New TT, ignored due to OOM emulator.makeTransfer(16, uavcan::TransferTypeServiceRequest, 2, "foo"), // Same as 2, not ignored emulator.makeTransfer(16, uavcan::TransferTypeServiceRequest, 2, "123456789abc"), // Same as 2, not SFT - ignore emulator.makeTransfer(16, uavcan::TransferTypeServiceRequest, 2, "bar"), // Same as 2, not ignored @@ -161,8 +162,8 @@ TEST(TransferListener, BasicSFT) ASSERT_TRUE(subscriber.matchAndPop(transfers[2])); ASSERT_TRUE(subscriber.matchAndPop(transfers[3])); ASSERT_TRUE(subscriber.matchAndPop(transfers[4])); - ASSERT_TRUE(subscriber.matchAndPop(transfers[6])); - ASSERT_TRUE(subscriber.matchAndPop(transfers[8])); + ASSERT_TRUE(subscriber.matchAndPop(transfers[5])); + ASSERT_TRUE(subscriber.matchAndPop(transfers[7])); ASSERT_TRUE(subscriber.isEmpty()); } @@ -172,7 +173,8 @@ TEST(TransferListener, Cleanup) { const uavcan::DataTypeDescriptor type(uavcan::DataTypeKindMessage, 123, uavcan::DataTypeSignature(123456789), "A"); - NullAllocator poolmgr; // No dynamic memory + static const int NUM_POOL_BLOCKS = 100; + uavcan::PoolAllocator poolmgr; uavcan::TransferPerfCounter perf; TestListener<256> subscriber(perf, type, poolmgr); // Static buffer only, 1 entry @@ -227,7 +229,8 @@ TEST(TransferListener, AnonymousTransfers) { const uavcan::DataTypeDescriptor type(uavcan::DataTypeKindMessage, 123, uavcan::DataTypeSignature(123456789), "A"); - NullAllocator poolmgr; + static const int NUM_POOL_BLOCKS = 100; + uavcan::PoolAllocator poolmgr; uavcan::TransferPerfCounter perf; TestListener<0> subscriber(perf, type, poolmgr); From 1149fc316e807c10f1ca0748853236c89cc61f2f Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Wed, 14 Oct 2015 09:54:27 +0300 Subject: [PATCH 06/25] OTR test fix --- libuavcan/test/transport/outgoing_transfer_registry.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libuavcan/test/transport/outgoing_transfer_registry.cpp b/libuavcan/test/transport/outgoing_transfer_registry.cpp index 50eb67eb9a..393a0abb7f 100644 --- a/libuavcan/test/transport/outgoing_transfer_registry.cpp +++ b/libuavcan/test/transport/outgoing_transfer_registry.cpp @@ -12,7 +12,7 @@ TEST(OutgoingTransferRegistry, Basic) { using uavcan::OutgoingTransferRegistryKey; - uavcan::PoolAllocator poolmgr; + uavcan::PoolAllocator poolmgr; uavcan::OutgoingTransferRegistry otr(poolmgr); otr.cleanup(tsMono(1000)); From 898e78fd06af4346235c9fd4d1d0fc7dc4155ec6 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Wed, 14 Oct 2015 09:57:08 +0300 Subject: [PATCH 07/25] Node tests fixed --- libuavcan/test/node/node.cpp | 7 ++++--- libuavcan/test/node/sub_node.cpp | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/libuavcan/test/node/node.cpp b/libuavcan/test/node/node.cpp index 9fc8cc56bf..cc4a4ff031 100644 --- a/libuavcan/test/node/node.cpp +++ b/libuavcan/test/node/node.cpp @@ -31,11 +31,12 @@ TEST(Node, Basic) swver.minor = 1; swver.vcs_commit = 0xDEADBEEF; + std::cout << "sizeof(uavcan::Node<0>): " << sizeof(uavcan::Node<0>) << std::endl; + /* * uavcan::Node */ - uavcan::Node<0> node1(nodes.can_a, nodes.clock_a); - std::cout << "sizeof(uavcan::Node<0>): " << sizeof(uavcan::Node<0>) << std::endl; + uavcan::Node<1024> node1(nodes.can_a, nodes.clock_a); node1.setName("com.example"); node1.setNodeID(1); node1.setSoftwareVersion(swver); @@ -43,7 +44,7 @@ TEST(Node, Basic) /* * Companion test node */ - uavcan::Node<0> node2(nodes.can_b, nodes.clock_b); + uavcan::Node<1024> node2(nodes.can_b, nodes.clock_b); node2.setName("foobar"); node2.setNodeID(2); node2.setSoftwareVersion(swver); diff --git a/libuavcan/test/node/sub_node.cpp b/libuavcan/test/node/sub_node.cpp index b2c0f96af2..3cb04df8dd 100644 --- a/libuavcan/test/node/sub_node.cpp +++ b/libuavcan/test/node/sub_node.cpp @@ -31,10 +31,12 @@ TEST(SubNode, Basic) swver.minor = 1; swver.vcs_commit = 0xDEADBEEF; + std::cout << "sizeof(uavcan::SubNode<0>): " << sizeof(uavcan::SubNode<0>) << std::endl; + /* * uavcan::Node */ - uavcan::Node<0> node1(nodes.can_a, nodes.clock_a); + uavcan::Node<1024> node1(nodes.can_a, nodes.clock_a); node1.setName("com.example"); node1.setNodeID(1); node1.setSoftwareVersion(swver); @@ -42,8 +44,7 @@ TEST(SubNode, Basic) /* * uavcan::SubNode */ - uavcan::SubNode<0> node2(nodes.can_b, nodes.clock_b); - std::cout << "sizeof(uavcan::SubNode<0>): " << sizeof(uavcan::SubNode<0>) << std::endl; + uavcan::SubNode<1024> node2(nodes.can_b, nodes.clock_b); BackgroundSpinner bgspinner(node2, node1); bgspinner.startPeriodic(uavcan::MonotonicDuration::fromMSec(10)); From 763e96b6ed9d49f572cec3b37cf18531f3405ca5 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Wed, 14 Oct 2015 10:10:15 +0300 Subject: [PATCH 08/25] Test node pool increased to 1024 blocks; this fixes the last test --- libuavcan/test/node/test_node.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libuavcan/test/node/test_node.hpp b/libuavcan/test/node/test_node.hpp index 1549f5bb45..ea187c809f 100644 --- a/libuavcan/test/node/test_node.hpp +++ b/libuavcan/test/node/test_node.hpp @@ -19,7 +19,7 @@ struct TestNode : public uavcan::INode { - uavcan::PoolAllocator pool; + uavcan::PoolAllocator pool; uavcan::OutgoingTransferRegistry otr; uavcan::Scheduler scheduler; uint64_t internal_failure_count; From a9fdf44fa984343a4efbb8c19fc0b50efb19b108 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Wed, 14 Oct 2015 19:58:51 +0300 Subject: [PATCH 09/25] Transfer buffering system detemplatized; compiles but tests are failing --- .../uavcan/node/generic_subscriber.hpp | 49 ++-- .../include/uavcan/node/service_client.hpp | 17 +- .../include/uavcan/node/service_server.hpp | 6 +- libuavcan/include/uavcan/node/subscriber.hpp | 6 +- .../include/uavcan/transport/dispatcher.hpp | 28 +- .../uavcan/transport/transfer_buffer.hpp | 241 ++++++------------ .../uavcan/transport/transfer_listener.hpp | 92 ++----- libuavcan/src/node/uc_generic_subscriber.cpp | 6 +- .../uc_can_acceptance_filter_configurator.cpp | 4 +- libuavcan/src/transport/uc_dispatcher.cpp | 28 +- .../src/transport/uc_transfer_buffer.cpp | 212 +++++++-------- .../src/transport/uc_transfer_listener.cpp | 46 +++- libuavcan/test/transport/dispatcher.cpp | 28 +- .../test/transport/incoming_transfer.cpp | 2 +- libuavcan/test/transport/transfer_buffer.cpp | 23 +- .../test/transport/transfer_listener.cpp | 18 +- .../test/transport/transfer_receiver.cpp | 20 +- libuavcan/test/transport/transfer_sender.cpp | 6 +- .../test/transport/transfer_test_helpers.cpp | 2 +- .../test/transport/transfer_test_helpers.hpp | 11 +- 20 files changed, 335 insertions(+), 510 deletions(-) diff --git a/libuavcan/include/uavcan/node/generic_subscriber.hpp b/libuavcan/include/uavcan/node/generic_subscriber.hpp index eca19c829f..6a61e0b041 100644 --- a/libuavcan/include/uavcan/node/generic_subscriber.hpp +++ b/libuavcan/include/uavcan/node/generic_subscriber.hpp @@ -100,9 +100,9 @@ protected: ~GenericSubscriberBase() { } - int genericStart(TransferListenerBase* listener, bool (Dispatcher::*registration_method)(TransferListenerBase*)); + int genericStart(TransferListener* listener, bool (Dispatcher::*registration_method)(TransferListener*)); - void stop(TransferListenerBase* listener); + void stop(TransferListener* listener); public: /** @@ -115,22 +115,6 @@ public: INode& getNode() const { return node_; } }; -/** - * This helper class does some compile-time magic on the transport layer machinery. For authorized personnel only. - */ -template class TransferListenerTemplate = TransferListener - > -class UAVCAN_EXPORT TransferListenerInstantiationHelper -{ - enum { DataTypeMaxByteLen = BitLenToByteLen::Result }; - enum { NeedsBuffer = int(DataTypeMaxByteLen) > int(GuaranteedPayloadLenPerFrame) }; - enum { BufferSize = NeedsBuffer ? DataTypeMaxByteLen : 0 }; - -public: - typedef TransferListenerTemplate Type; -}; - /** * Please note that the reference passed to the RX callback points to a stack-allocated object, which means * that it gets invalidated shortly after the callback returns. @@ -151,9 +135,15 @@ class UAVCAN_EXPORT GenericSubscriber : public GenericSubscriberBase } public: - TransferForwarder(SelfType& obj, const DataTypeDescriptor& data_type, IPoolAllocator& allocator) - : TransferListenerType(obj.node_.getDispatcher().getTransferPerfCounter(), data_type, allocator) - , obj_(obj) + TransferForwarder(SelfType& obj, + const DataTypeDescriptor& data_type, + uint16_t max_buffer_size, + IPoolAllocator& allocator) : + TransferListenerType(obj.node_.getDispatcher().getTransferPerfCounter(), + data_type, + max_buffer_size, + allocator), + obj_(obj) { } }; @@ -163,20 +153,19 @@ class UAVCAN_EXPORT GenericSubscriber : public GenericSubscriberBase void handleIncomingTransfer(IncomingTransfer& transfer); - int genericStart(bool (Dispatcher::*registration_method)(TransferListenerBase*)); + int genericStart(bool (Dispatcher::*registration_method)(TransferListener*)); protected: struct ReceivedDataStructureSpec : public ReceivedDataStructure { ReceivedDataStructureSpec() { } - ReceivedDataStructureSpec(const IncomingTransfer* arg_transfer) - : ReceivedDataStructure(arg_transfer) + ReceivedDataStructureSpec(const IncomingTransfer* arg_transfer) : + ReceivedDataStructure(arg_transfer) { } }; - explicit GenericSubscriber(INode& node) - : GenericSubscriberBase(node) + explicit GenericSubscriber(INode& node) : GenericSubscriberBase(node) { } virtual ~GenericSubscriber() { stop(); } @@ -247,8 +236,10 @@ int GenericSubscriber::checkInit() return -ErrUnknownDataType; } - forwarder_.template construct - (*this, *descr, node_.getAllocator()); + static const uint16_t MaxBufferSize = BitLenToByteLen::Result; + + forwarder_.template construct + (*this, *descr, MaxBufferSize, node_.getAllocator()); return 0; } @@ -286,7 +277,7 @@ void GenericSubscriber::handleIncomi template int GenericSubscriber:: -genericStart(bool (Dispatcher::*registration_method)(TransferListenerBase*)) +genericStart(bool (Dispatcher::*registration_method)(TransferListener*)) { const int res = checkInit(); if (res < 0) diff --git a/libuavcan/include/uavcan/node/service_client.hpp b/libuavcan/include/uavcan/node/service_client.hpp index a7cce0dd4e..d195dcc721 100644 --- a/libuavcan/include/uavcan/node/service_client.hpp +++ b/libuavcan/include/uavcan/node/service_client.hpp @@ -20,15 +20,6 @@ namespace uavcan { - -template -class UAVCAN_EXPORT ServiceResponseTransferListenerInstantiationHelper -{ -public: - typedef typename TransferListenerInstantiationHelper::Type Type; -}; - /** * This struct describes a pending service call. * Refer to @ref ServiceClient to learn more about service calls. @@ -225,8 +216,7 @@ template class UAVCAN_EXPORT ServiceClient : public GenericSubscriber::Type> + typename DataType_::Response, TransferListenerWithFilter> , public ServiceClientBase { public: @@ -239,8 +229,7 @@ public: private: typedef ServiceClient SelfType; typedef GenericPublisher PublisherType; - typedef typename ServiceResponseTransferListenerInstantiationHelper::Type TransferListenerType; - typedef GenericSubscriber SubscriberType; + typedef GenericSubscriber SubscriberType; typedef Multiset CallRegistry; @@ -537,7 +526,7 @@ int ServiceClient::call(NodeID server_node_id, const Reque * Configuring the listener so it will accept only the matching responses * TODO move to init(), but this requires to somewhat refactor GenericSubscriber<> (remove TransferForwarder) */ - TransferListenerType* const tl = SubscriberType::getTransferListener(); + TransferListenerWithFilter* const tl = SubscriberType::getTransferListener(); if (tl == NULL) { UAVCAN_ASSERT(0); // Must have been created diff --git a/libuavcan/include/uavcan/node/service_server.hpp b/libuavcan/include/uavcan/node/service_server.hpp index d362502800..5b40ad8836 100644 --- a/libuavcan/include/uavcan/node/service_server.hpp +++ b/libuavcan/include/uavcan/node/service_server.hpp @@ -88,8 +88,7 @@ template class UAVCAN_EXPORT ServiceServer - : public GenericSubscriber::Type> + : public GenericSubscriber { public: typedef DataType_ DataType; @@ -98,8 +97,7 @@ public: typedef Callback_ Callback; private: - typedef typename TransferListenerInstantiationHelper::Type TransferListenerType; - typedef GenericSubscriber SubscriberType; + typedef GenericSubscriber SubscriberType; typedef GenericPublisher PublisherType; PublisherType publisher_; diff --git a/libuavcan/include/uavcan/node/subscriber.hpp b/libuavcan/include/uavcan/node/subscriber.hpp index b82233dddd..1ee4c09888 100644 --- a/libuavcan/include/uavcan/node/subscriber.hpp +++ b/libuavcan/include/uavcan/node/subscriber.hpp @@ -43,15 +43,13 @@ template class UAVCAN_EXPORT Subscriber - : public GenericSubscriber::Type> + : public GenericSubscriber { public: typedef Callback_ Callback; private: - typedef typename TransferListenerInstantiationHelper::Type TransferListenerType; - typedef GenericSubscriber BaseType; + typedef GenericSubscriber BaseType; Callback callback_; diff --git a/libuavcan/include/uavcan/transport/dispatcher.hpp b/libuavcan/include/uavcan/transport/dispatcher.hpp index 99cee5b0a8..33ac1f2e5c 100644 --- a/libuavcan/include/uavcan/transport/dispatcher.hpp +++ b/libuavcan/include/uavcan/transport/dispatcher.hpp @@ -86,14 +86,14 @@ class UAVCAN_EXPORT Dispatcher : Noncopyable class ListenerRegistry { - LinkedListRoot list_; + LinkedListRoot list_; class DataTypeIDInsertionComparator { const DataTypeID id_; public: explicit DataTypeIDInsertionComparator(DataTypeID id) : id_(id) { } - bool operator()(const TransferListenerBase* listener) const + bool operator()(const TransferListener* listener) const { UAVCAN_ASSERT(listener); return id_ > listener->getDataTypeDescriptor().getID(); @@ -103,15 +103,15 @@ class UAVCAN_EXPORT Dispatcher : Noncopyable public: enum Mode { UniqueListener, ManyListeners }; - bool add(TransferListenerBase* listener, Mode mode); - void remove(TransferListenerBase* listener); + bool add(TransferListener* listener, Mode mode); + void remove(TransferListener* listener); bool exists(DataTypeID dtid) const; void cleanup(MonotonicTime ts); void handleFrame(const RxFrame& frame); unsigned getNumEntries() const { return list_.getLength(); } - const LinkedListRoot& getList() const { return list_; } + const LinkedListRoot& getList() const { return list_; } }; ListenerRegistry lmsg_; @@ -162,13 +162,13 @@ public: void cleanup(MonotonicTime ts); - bool registerMessageListener(TransferListenerBase* listener); - bool registerServiceRequestListener(TransferListenerBase* listener); - bool registerServiceResponseListener(TransferListenerBase* listener); + bool registerMessageListener(TransferListener* listener); + bool registerServiceRequestListener(TransferListener* listener); + bool registerServiceResponseListener(TransferListener* listener); - void unregisterMessageListener(TransferListenerBase* listener); - void unregisterServiceRequestListener(TransferListenerBase* listener); - void unregisterServiceResponseListener(TransferListenerBase* listener); + void unregisterMessageListener(TransferListener* listener); + void unregisterServiceRequestListener(TransferListener* listener); + void unregisterServiceResponseListener(TransferListener* listener); bool hasSubscriber(DataTypeID dtid) const; bool hasPublisher(DataTypeID dtid) const; @@ -185,15 +185,15 @@ public: * removed from this list as soon as the corresponding service call is complete. * @{ */ - const LinkedListRoot& getListOfMessageListeners() const + const LinkedListRoot& getListOfMessageListeners() const { return lmsg_.getList(); } - const LinkedListRoot& getListOfServiceRequestListeners() const + const LinkedListRoot& getListOfServiceRequestListeners() const { return lsrv_req_.getList(); } - const LinkedListRoot& getListOfServiceResponseListeners() const + const LinkedListRoot& getListOfServiceResponseListeners() const { return lsrv_resp_.getList(); } diff --git a/libuavcan/include/uavcan/transport/transfer_buffer.hpp b/libuavcan/include/uavcan/transport/transfer_buffer.hpp index 7ea35145be..d4e05fac2f 100644 --- a/libuavcan/include/uavcan/transport/transfer_buffer.hpp +++ b/libuavcan/include/uavcan/transport/transfer_buffer.hpp @@ -16,6 +16,47 @@ namespace uavcan { +/** + * Standalone static buffer + */ +class StaticTransferBufferImpl : public ITransferBuffer +{ + uint8_t* const data_; + const uint16_t size_; + uint16_t max_write_pos_; + +public: + StaticTransferBufferImpl(uint8_t* buf, uint16_t buf_size) : + data_(buf), + size_(buf_size), + max_write_pos_(0) + { } + + virtual int read(unsigned offset, uint8_t* data, unsigned len) const; + virtual int write(unsigned offset, const uint8_t* data, unsigned len); + + void reset(); + + uint16_t getSize() const { return size_; } + + uint8_t* getRawPtr() { return data_; } + const uint8_t* getRawPtr() const { return data_; } + + uint16_t getMaxWritePos() const { return max_write_pos_; } + void setMaxWritePos(uint16_t value) { max_write_pos_ = value; } +}; + +template +class UAVCAN_EXPORT StaticTransferBuffer : public StaticTransferBufferImpl +{ + uint8_t buffer_[Size]; +public: + StaticTransferBuffer() : StaticTransferBufferImpl(buffer_, Size) + { + StaticAssert<(Size > 0)>::check(); + } +}; + /** * Internal for TransferBufferManager */ @@ -53,41 +94,13 @@ public: #endif }; -/** - * Internal for TransferBufferManager - */ -class UAVCAN_EXPORT TransferBufferManagerEntry : public ITransferBuffer, Noncopyable -{ - TransferBufferManagerKey key_; - -protected: - virtual void resetImpl() = 0; - -public: - TransferBufferManagerEntry() { } - - explicit TransferBufferManagerEntry(const TransferBufferManagerKey& key) - : key_(key) - { } - - const TransferBufferManagerKey& getKey() const { return key_; } - bool isEmpty() const { return key_.isEmpty(); } - - void reset(const TransferBufferManagerKey& key = TransferBufferManagerKey()) - { - key_ = key; - resetImpl(); - } -}; - /** * Resizable gather/scatter storage. * reset() call releases all memory blocks. * Supports unordered write operations - from higher to lower offsets */ -class UAVCAN_EXPORT DynamicTransferBufferManagerEntry - : public TransferBufferManagerEntry - , public LinkedListNode +class UAVCAN_EXPORT TransferBufferManagerEntry : public ITransferBuffer + , public LinkedListNode { struct Block : LinkedListNode { @@ -107,116 +120,58 @@ class UAVCAN_EXPORT DynamicTransferBufferManagerEntry LinkedListRoot blocks_; // Blocks are ordered from lower to higher buffer offset uint16_t max_write_pos_; const uint16_t max_size_; - - /// Reset functionality must be implemented in a non-virtual method to call it safely from the destructor. - void doReset(); - - virtual void resetImpl(); + TransferBufferManagerKey key_; public: - DynamicTransferBufferManagerEntry(IPoolAllocator& allocator, uint16_t max_size) - : allocator_(allocator) - , max_write_pos_(0) - , max_size_(max_size) + TransferBufferManagerEntry(IPoolAllocator& allocator, uint16_t max_size) : + allocator_(allocator), + max_write_pos_(0), + max_size_(max_size) { StaticAssert<(Block::Size > 8)>::check(); IsDynamicallyAllocatable::check(); - IsDynamicallyAllocatable::check(); + IsDynamicallyAllocatable::check(); } - virtual ~DynamicTransferBufferManagerEntry() - { - doReset(); - } + virtual ~TransferBufferManagerEntry() { reset(); } - static DynamicTransferBufferManagerEntry* instantiate(IPoolAllocator& allocator, uint16_t max_size); - static void destroy(DynamicTransferBufferManagerEntry*& obj, IPoolAllocator& allocator); - - virtual int read(unsigned offset, uint8_t* data, unsigned len) const; - virtual int write(unsigned offset, const uint8_t* data, unsigned len); -}; - -/** - * Standalone static buffer - */ -class StaticTransferBufferImpl : public ITransferBuffer -{ - uint8_t* const data_; - const uint16_t size_; - uint16_t max_write_pos_; - -public: - StaticTransferBufferImpl(uint8_t* buf, uint16_t buf_size) - : data_(buf) - , size_(buf_size) - , max_write_pos_(0) - { } + static TransferBufferManagerEntry* instantiate(IPoolAllocator& allocator, uint16_t max_size); + static void destroy(TransferBufferManagerEntry*& obj, IPoolAllocator& allocator); virtual int read(unsigned offset, uint8_t* data, unsigned len) const; virtual int write(unsigned offset, const uint8_t* data, unsigned len); - void reset(); + void reset(const TransferBufferManagerKey& key = TransferBufferManagerKey()); - uint16_t getSize() const { return size_; } - - uint8_t* getRawPtr() { return data_; } - const uint8_t* getRawPtr() const { return data_; } - - uint16_t getMaxWritePos() const { return max_write_pos_; } - void setMaxWritePos(uint16_t value) { max_write_pos_ = value; } -}; - -template -class UAVCAN_EXPORT StaticTransferBuffer : public StaticTransferBufferImpl -{ - uint8_t buffer_[Size]; -public: - StaticTransferBuffer() - : StaticTransferBufferImpl(buffer_, Size) - { - StaticAssert<(Size > 0)>::check(); - } + const TransferBufferManagerKey& getKey() const { return key_; } + bool isEmpty() const { return key_.isEmpty(); } }; /** - * Statically allocated storage for the buffer manager + * Buffer manager implementation. */ -class StaticTransferBufferManagerEntryImpl : public TransferBufferManagerEntry +class TransferBufferManager : public Noncopyable { - StaticTransferBufferImpl buf_; + LinkedListRoot buffers_; + IPoolAllocator& allocator_; + const uint16_t max_buf_size_; - virtual void resetImpl(); + TransferBufferManagerEntry* findFirst(const TransferBufferManagerKey& key); public: - StaticTransferBufferManagerEntryImpl(uint8_t* buf, uint16_t buf_size) - : buf_(buf, buf_size) + TransferBufferManager(uint16_t max_buf_size, IPoolAllocator& allocator) : + allocator_(allocator), + max_buf_size_(max_buf_size) { } - virtual int read(unsigned offset, uint8_t* data, unsigned len) const; - virtual int write(unsigned offset, const uint8_t* data, unsigned len); -}; + ~TransferBufferManager(); -template -class UAVCAN_EXPORT StaticTransferBufferManagerEntry : public StaticTransferBufferManagerEntryImpl -{ - uint8_t buffer_[Size]; -public: - StaticTransferBufferManagerEntry() - : StaticTransferBufferManagerEntryImpl(buffer_, Size) - { } -}; + ITransferBuffer* access(const TransferBufferManagerKey& key); + ITransferBuffer* create(const TransferBufferManagerKey& key); + void remove(const TransferBufferManagerKey& key); + bool isEmpty() const; -/** - * Manages different storage types (static/dynamic) for transfer reception logic. - */ -class UAVCAN_EXPORT ITransferBufferManager -{ -public: - virtual ~ITransferBufferManager() { } - virtual ITransferBuffer* access(const TransferBufferManagerKey& key) = 0; - virtual ITransferBuffer* create(const TransferBufferManagerKey& key) = 0; - virtual void remove(const TransferBufferManagerKey& key) = 0; - virtual bool isEmpty() const = 0; + unsigned getNumBuffers() const; }; /** @@ -224,13 +179,13 @@ public: */ class UAVCAN_EXPORT TransferBufferAccessor { - ITransferBufferManager& bufmgr_; + TransferBufferManager& bufmgr_; const TransferBufferManagerKey key_; public: - TransferBufferAccessor(ITransferBufferManager& bufmgr, TransferBufferManagerKey key) - : bufmgr_(bufmgr) - , key_(key) + TransferBufferAccessor(TransferBufferManager& bufmgr, TransferBufferManagerKey key) : + bufmgr_(bufmgr), + key_(key) { UAVCAN_ASSERT(!key.isEmpty()); } @@ -239,54 +194,6 @@ public: void remove() { bufmgr_.remove(key_); } }; -/** - * Buffer manager implementation. - */ -class TransferBufferManagerImpl : public ITransferBufferManager, Noncopyable -{ - LinkedListRoot dynamic_buffers_; - IPoolAllocator& allocator_; - const uint16_t max_buf_size_; - - DynamicTransferBufferManagerEntry* findFirstDynamic(const TransferBufferManagerKey& key); - -public: - TransferBufferManagerImpl(uint16_t max_buf_size, IPoolAllocator& allocator) - : allocator_(allocator) - , max_buf_size_(max_buf_size) - { } - - virtual ~TransferBufferManagerImpl(); - - virtual ITransferBuffer* access(const TransferBufferManagerKey& key); - virtual ITransferBuffer* create(const TransferBufferManagerKey& key); - virtual void remove(const TransferBufferManagerKey& key); - virtual bool isEmpty() const; - - unsigned getNumBuffers() const; -}; - -template -class UAVCAN_EXPORT TransferBufferManager : public TransferBufferManagerImpl -{ -public: - explicit TransferBufferManager(IPoolAllocator& allocator) : - TransferBufferManagerImpl(MaxBufSize, allocator) - { } -}; - -template <> -class UAVCAN_EXPORT TransferBufferManager<0> : public ITransferBufferManager -{ -public: - TransferBufferManager() { } - TransferBufferManager(IPoolAllocator&) { } - virtual ITransferBuffer* access(const TransferBufferManagerKey&) { return NULL; } - virtual ITransferBuffer* create(const TransferBufferManagerKey&) { return NULL; } - virtual void remove(const TransferBufferManagerKey&) { } - virtual bool isEmpty() const { return true; } -}; - } #endif // UAVCAN_TRANSPORT_TRANSFER_BUFFER_HPP_INCLUDED diff --git a/libuavcan/include/uavcan/transport/transfer_listener.hpp b/libuavcan/include/uavcan/transport/transfer_listener.hpp index 181a2e36b1..b6d28d29f0 100644 --- a/libuavcan/include/uavcan/transport/transfer_listener.hpp +++ b/libuavcan/include/uavcan/transport/transfer_listener.hpp @@ -96,11 +96,11 @@ public: /** * Internal, refer to the transport dispatcher class. */ -class UAVCAN_EXPORT TransferListenerBase : public LinkedListNode, Noncopyable +class UAVCAN_EXPORT TransferListener : public LinkedListNode, Noncopyable { const DataTypeDescriptor& data_type_; - Map& receivers_; - ITransferBufferManager& bufmgr_; + TransferBufferManager bufmgr_; + Map receivers_; TransferPerfCounter& perf_; const TransferCRC crc_base_; ///< Pre-initialized with data type hash, thus constant bool allow_anonymous_transfers_; @@ -108,10 +108,10 @@ class UAVCAN_EXPORT TransferListenerBase : public LinkedListNode& receivers, - ITransferBufferManager& bufmgr) - : data_type_(data_type) - , receivers_(receivers) - , bufmgr_(bufmgr) - , perf_(perf) - , crc_base_(data_type.getSignature().toTransferCRC()) - , allow_anonymous_transfers_(false) - { } - - virtual ~TransferListenerBase() { } - void handleReception(TransferReceiver& receiver, const RxFrame& frame, TransferBufferAccessor& tba); void handleAnonymousTransferReception(const RxFrame& frame); virtual void handleIncomingTransfer(IncomingTransfer& transfer) = 0; public: + TransferListener(TransferPerfCounter& perf, const DataTypeDescriptor& data_type, + uint16_t max_buffer_size, IPoolAllocator& allocator) + : data_type_(data_type) + , bufmgr_(max_buffer_size, allocator) + , receivers_(allocator) + , perf_(perf) + , crc_base_(data_type.getSignature().toTransferCRC()) + , allow_anonymous_transfers_(false) + { } + + virtual ~TransferListener() + { + // Map must be cleared before bufmgr is destroyed + receivers_.clear(); + } + const DataTypeDescriptor& getDataTypeDescriptor() const { return data_type_; } /** @@ -154,29 +157,6 @@ public: virtual void handleFrame(const RxFrame& frame); }; -/** - * This class should be derived by transfer receivers (subscribers, servers). - */ -template -class UAVCAN_EXPORT TransferListener : public TransferListenerBase -{ - TransferBufferManager bufmgr_; - Map receivers_; - -public: - TransferListener(TransferPerfCounter& perf, const DataTypeDescriptor& data_type, IPoolAllocator& allocator) - : TransferListenerBase(perf, data_type, receivers_, bufmgr_) - , bufmgr_(allocator) - , receivers_(allocator) - { } - - virtual ~TransferListener() - { - // Map must be cleared before bufmgr is destroyed - receivers_.clear(); - } -}; - /** * This class is used by transfer listener to decide if the frame should be accepted or ignored. */ @@ -194,19 +174,16 @@ public: /** * This class should be derived by callers. */ -template -class UAVCAN_EXPORT TransferListenerWithFilter : public TransferListener +class UAVCAN_EXPORT TransferListenerWithFilter : public TransferListener { const ITransferAcceptanceFilter* filter_; virtual void handleFrame(const RxFrame& frame); public: - typedef TransferListener BaseType; - TransferListenerWithFilter(TransferPerfCounter& perf, const DataTypeDescriptor& data_type, - IPoolAllocator& allocator) - : BaseType(perf, data_type, allocator) + uint16_t max_buffer_size, IPoolAllocator& allocator) + : TransferListener(perf, data_type, max_buffer_size, allocator) , filter_(NULL) { } @@ -216,27 +193,6 @@ public: } }; -// ---------------------------------------------------------------------------- - -/* - * TransferListenerWithFilter<> - */ -template -void TransferListenerWithFilter::handleFrame(const RxFrame& frame) -{ - if (filter_ != NULL) - { - if (filter_->shouldAcceptFrame(frame)) - { - BaseType::handleFrame(frame); - } - } - else - { - UAVCAN_ASSERT(0); - } -} - } #endif // UAVCAN_TRANSPORT_TRANSFER_LISTENER_HPP_INCLUDED diff --git a/libuavcan/src/node/uc_generic_subscriber.cpp b/libuavcan/src/node/uc_generic_subscriber.cpp index 4ec626cd10..f77cda77b9 100644 --- a/libuavcan/src/node/uc_generic_subscriber.cpp +++ b/libuavcan/src/node/uc_generic_subscriber.cpp @@ -7,8 +7,8 @@ namespace uavcan { -int GenericSubscriberBase::genericStart(TransferListenerBase* listener, - bool (Dispatcher::*registration_method)(TransferListenerBase*)) +int GenericSubscriberBase::genericStart(TransferListener* listener, + bool (Dispatcher::*registration_method)(TransferListener*)) { if (listener == NULL) { @@ -24,7 +24,7 @@ int GenericSubscriberBase::genericStart(TransferListenerBase* listener, return 0; } -void GenericSubscriberBase::stop(TransferListenerBase* listener) +void GenericSubscriberBase::stop(TransferListener* listener) { if (listener != NULL) { diff --git a/libuavcan/src/transport/uc_can_acceptance_filter_configurator.cpp b/libuavcan/src/transport/uc_can_acceptance_filter_configurator.cpp index f82b6a57d0..820c0b22d2 100644 --- a/libuavcan/src/transport/uc_can_acceptance_filter_configurator.cpp +++ b/libuavcan/src/transport/uc_can_acceptance_filter_configurator.cpp @@ -38,8 +38,8 @@ int16_t CanAcceptanceFilterConfigurator::loadInputConfiguration(AnonymousMessage return -ErrMemory; } - const TransferListenerBase* p = node_.getDispatcher().getListOfMessageListeners().get(); - while (p) + const TransferListener* p = node_.getDispatcher().getListOfMessageListeners().get(); + while (p != NULL) { CanFilterConfig cfg; cfg.id = static_cast(p->getDataTypeDescriptor().getID().get()) << 8; diff --git a/libuavcan/src/transport/uc_dispatcher.cpp b/libuavcan/src/transport/uc_dispatcher.cpp index 8e91437f81..0349862ca9 100644 --- a/libuavcan/src/transport/uc_dispatcher.cpp +++ b/libuavcan/src/transport/uc_dispatcher.cpp @@ -72,11 +72,11 @@ void LoopbackFrameListenerRegistry::invokeListeners(RxFrame& frame) /* * Dispatcher::ListenerRegister */ -bool Dispatcher::ListenerRegistry::add(TransferListenerBase* listener, Mode mode) +bool Dispatcher::ListenerRegistry::add(TransferListener* listener, Mode mode) { if (mode == UniqueListener) { - TransferListenerBase* p = list_.get(); + TransferListener* p = list_.get(); while (p) { if (p->getDataTypeDescriptor().getID() == listener->getDataTypeDescriptor().getID()) @@ -91,14 +91,14 @@ bool Dispatcher::ListenerRegistry::add(TransferListenerBase* listener, Mode mode return true; } -void Dispatcher::ListenerRegistry::remove(TransferListenerBase* listener) +void Dispatcher::ListenerRegistry::remove(TransferListener* listener) { list_.remove(listener); } bool Dispatcher::ListenerRegistry::exists(DataTypeID dtid) const { - TransferListenerBase* p = list_.get(); + TransferListener* p = list_.get(); while (p) { if (p->getDataTypeDescriptor().getID() == dtid) @@ -112,10 +112,10 @@ bool Dispatcher::ListenerRegistry::exists(DataTypeID dtid) const void Dispatcher::ListenerRegistry::cleanup(MonotonicTime ts) { - TransferListenerBase* p = list_.get(); + TransferListener* p = list_.get(); while (p) { - TransferListenerBase* const next = p->getNextListNode(); + TransferListener* const next = p->getNextListNode(); p->cleanup(ts); // p may be modified p = next; } @@ -123,10 +123,10 @@ void Dispatcher::ListenerRegistry::cleanup(MonotonicTime ts) void Dispatcher::ListenerRegistry::handleFrame(const RxFrame& frame) { - TransferListenerBase* p = list_.get(); + TransferListener* p = list_.get(); while (p) { - TransferListenerBase* const next = p->getNextListNode(); + TransferListener* const next = p->getNextListNode(); if (p->getDataTypeDescriptor().getID() == frame.getDataTypeID()) { p->handleFrame(frame); // p may be modified @@ -311,7 +311,7 @@ void Dispatcher::cleanup(MonotonicTime ts) lsrv_resp_.cleanup(ts); } -bool Dispatcher::registerMessageListener(TransferListenerBase* listener) +bool Dispatcher::registerMessageListener(TransferListener* listener) { if (listener->getDataTypeDescriptor().getKind() != DataTypeKindMessage) { @@ -321,7 +321,7 @@ bool Dispatcher::registerMessageListener(TransferListenerBase* listener) return lmsg_.add(listener, ListenerRegistry::ManyListeners); // Multiple subscribers are OK } -bool Dispatcher::registerServiceRequestListener(TransferListenerBase* listener) +bool Dispatcher::registerServiceRequestListener(TransferListener* listener) { if (listener->getDataTypeDescriptor().getKind() != DataTypeKindService) { @@ -331,7 +331,7 @@ bool Dispatcher::registerServiceRequestListener(TransferListenerBase* listener) return lsrv_req_.add(listener, ListenerRegistry::UniqueListener); // Only one server per data type } -bool Dispatcher::registerServiceResponseListener(TransferListenerBase* listener) +bool Dispatcher::registerServiceResponseListener(TransferListener* listener) { if (listener->getDataTypeDescriptor().getKind() != DataTypeKindService) { @@ -341,17 +341,17 @@ bool Dispatcher::registerServiceResponseListener(TransferListenerBase* listener) return lsrv_resp_.add(listener, ListenerRegistry::ManyListeners); // Multiple callers may call same srv } -void Dispatcher::unregisterMessageListener(TransferListenerBase* listener) +void Dispatcher::unregisterMessageListener(TransferListener* listener) { lmsg_.remove(listener); } -void Dispatcher::unregisterServiceRequestListener(TransferListenerBase* listener) +void Dispatcher::unregisterServiceRequestListener(TransferListener* listener) { lsrv_req_.remove(listener); } -void Dispatcher::unregisterServiceResponseListener(TransferListenerBase* listener) +void Dispatcher::unregisterServiceResponseListener(TransferListener* listener) { lsrv_resp_.remove(listener); } diff --git a/libuavcan/src/transport/uc_transfer_buffer.cpp b/libuavcan/src/transport/uc_transfer_buffer.cpp index f684d8ecd7..f30c9da675 100644 --- a/libuavcan/src/transport/uc_transfer_buffer.cpp +++ b/libuavcan/src/transport/uc_transfer_buffer.cpp @@ -8,6 +8,58 @@ namespace uavcan { +/* + * StaticTransferBufferImpl + */ +int StaticTransferBufferImpl::read(unsigned offset, uint8_t* data, unsigned len) const +{ + if (!data) + { + UAVCAN_ASSERT(0); + return -ErrInvalidParam; + } + if (offset >= max_write_pos_) + { + return 0; + } + if ((offset + len) > max_write_pos_) + { + len = max_write_pos_ - offset; + } + UAVCAN_ASSERT((offset + len) <= max_write_pos_); + (void)copy(data_ + offset, data_ + offset + len, data); + return int(len); +} + +int StaticTransferBufferImpl::write(unsigned offset, const uint8_t* data, unsigned len) +{ + if (!data) + { + UAVCAN_ASSERT(0); + return -ErrInvalidParam; + } + if (offset >= size_) + { + return 0; + } + if ((offset + len) > size_) + { + len = size_ - offset; + } + UAVCAN_ASSERT((offset + len) <= size_); + (void)copy(data, data + len, data_ + offset); + max_write_pos_ = max(uint16_t(offset + len), uint16_t(max_write_pos_)); + return int(len); +} + +void StaticTransferBufferImpl::reset() +{ + max_write_pos_ = 0; +#if UAVCAN_DEBUG + fill(data_, data_ + size_, uint8_t(0)); +#endif +} + /* * TransferBufferManagerKey */ @@ -23,8 +75,8 @@ std::string TransferBufferManagerKey::toString() const /* * DynamicTransferBuffer::Block */ -DynamicTransferBufferManagerEntry::Block* -DynamicTransferBufferManagerEntry::Block::instantiate(IPoolAllocator& allocator) +TransferBufferManagerEntry::Block* +TransferBufferManagerEntry::Block::instantiate(IPoolAllocator& allocator) { void* const praw = allocator.allocate(sizeof(Block)); if (praw == NULL) @@ -34,7 +86,7 @@ DynamicTransferBufferManagerEntry::Block::instantiate(IPoolAllocator& allocator) return new (praw) Block; } -void DynamicTransferBufferManagerEntry::Block::destroy(Block*& obj, IPoolAllocator& allocator) +void TransferBufferManagerEntry::Block::destroy(Block*& obj, IPoolAllocator& allocator) { if (obj != NULL) { @@ -44,8 +96,8 @@ void DynamicTransferBufferManagerEntry::Block::destroy(Block*& obj, IPoolAllocat } } -void DynamicTransferBufferManagerEntry::Block::read(uint8_t*& outptr, unsigned target_offset, - unsigned& total_offset, unsigned& left_to_read) +void TransferBufferManagerEntry::Block::read(uint8_t*& outptr, unsigned target_offset, + unsigned& total_offset, unsigned& left_to_read) { UAVCAN_ASSERT(outptr); for (unsigned i = 0; (i < Block::Size) && (left_to_read > 0); i++, total_offset++) @@ -58,7 +110,7 @@ void DynamicTransferBufferManagerEntry::Block::read(uint8_t*& outptr, unsigned t } } -void DynamicTransferBufferManagerEntry::Block::write(const uint8_t*& inptr, unsigned target_offset, +void TransferBufferManagerEntry::Block::write(const uint8_t*& inptr, unsigned target_offset, unsigned& total_offset, unsigned& left_to_write) { UAVCAN_ASSERT(inptr); @@ -75,46 +127,28 @@ void DynamicTransferBufferManagerEntry::Block::write(const uint8_t*& inptr, unsi /* * DynamicTransferBuffer */ -DynamicTransferBufferManagerEntry* DynamicTransferBufferManagerEntry::instantiate(IPoolAllocator& allocator, +TransferBufferManagerEntry* TransferBufferManagerEntry::instantiate(IPoolAllocator& allocator, uint16_t max_size) { - void* const praw = allocator.allocate(sizeof(DynamicTransferBufferManagerEntry)); + void* const praw = allocator.allocate(sizeof(TransferBufferManagerEntry)); if (praw == NULL) { return NULL; } - return new (praw) DynamicTransferBufferManagerEntry(allocator, max_size); + return new (praw) TransferBufferManagerEntry(allocator, max_size); } -void DynamicTransferBufferManagerEntry::destroy(DynamicTransferBufferManagerEntry*& obj, IPoolAllocator& allocator) +void TransferBufferManagerEntry::destroy(TransferBufferManagerEntry*& obj, IPoolAllocator& allocator) { if (obj != NULL) { - obj->~DynamicTransferBufferManagerEntry(); + obj->~TransferBufferManagerEntry(); allocator.deallocate(obj); obj = NULL; } } -void DynamicTransferBufferManagerEntry::doReset() -{ - max_write_pos_ = 0; - Block* p = blocks_.get(); - while (p) - { - Block* const next = p->getNextListNode(); - blocks_.remove(p); - Block::destroy(p, allocator_); - p = next; - } -} - -void DynamicTransferBufferManagerEntry::resetImpl() -{ - doReset(); -} - -int DynamicTransferBufferManagerEntry::read(unsigned offset, uint8_t* data, unsigned len) const +int TransferBufferManagerEntry::read(unsigned offset, uint8_t* data, unsigned len) const { if (!data) { @@ -150,7 +184,7 @@ int DynamicTransferBufferManagerEntry::read(unsigned offset, uint8_t* data, unsi return int(len); } -int DynamicTransferBufferManagerEntry::write(unsigned offset, const uint8_t* data, unsigned len) +int TransferBufferManagerEntry::write(unsigned offset, const uint8_t* data, unsigned len) { if (!data) { @@ -221,82 +255,26 @@ int DynamicTransferBufferManagerEntry::write(unsigned offset, const uint8_t* dat return int(actually_written); } -/* - * StaticTransferBufferImpl - */ -int StaticTransferBufferImpl::read(unsigned offset, uint8_t* data, unsigned len) const -{ - if (!data) - { - UAVCAN_ASSERT(0); - return -ErrInvalidParam; - } - if (offset >= max_write_pos_) - { - return 0; - } - if ((offset + len) > max_write_pos_) - { - len = max_write_pos_ - offset; - } - UAVCAN_ASSERT((offset + len) <= max_write_pos_); - (void)copy(data_ + offset, data_ + offset + len, data); - return int(len); -} - -int StaticTransferBufferImpl::write(unsigned offset, const uint8_t* data, unsigned len) -{ - if (!data) - { - UAVCAN_ASSERT(0); - return -ErrInvalidParam; - } - if (offset >= size_) - { - return 0; - } - if ((offset + len) > size_) - { - len = size_ - offset; - } - UAVCAN_ASSERT((offset + len) <= size_); - (void)copy(data, data + len, data_ + offset); - max_write_pos_ = max(uint16_t(offset + len), uint16_t(max_write_pos_)); - return int(len); -} - -void StaticTransferBufferImpl::reset() +void TransferBufferManagerEntry::reset(const TransferBufferManagerKey& key) { + key_ = key; max_write_pos_ = 0; -#if UAVCAN_DEBUG - fill(data_, data_ + size_, uint8_t(0)); -#endif + Block* p = blocks_.get(); + while (p) + { + Block* const next = p->getNextListNode(); + blocks_.remove(p); + Block::destroy(p, allocator_); + p = next; + } } /* - * StaticTransferBufferManagerEntryImpl + * TransferBufferManager */ -void StaticTransferBufferManagerEntryImpl::resetImpl() +TransferBufferManagerEntry* TransferBufferManager::findFirst(const TransferBufferManagerKey& key) { - buf_.reset(); -} - -int StaticTransferBufferManagerEntryImpl::read(unsigned offset, uint8_t* data, unsigned len) const -{ - return buf_.read(offset, data, len); -} - -int StaticTransferBufferManagerEntryImpl::write(unsigned offset, const uint8_t* data, unsigned len) -{ - return buf_.write(offset, data, len); -} - -/* - * TransferBufferManagerImpl - */ -DynamicTransferBufferManagerEntry* TransferBufferManagerImpl::findFirstDynamic(const TransferBufferManagerKey& key) -{ - DynamicTransferBufferManagerEntry* dyn = dynamic_buffers_.get(); + TransferBufferManagerEntry* dyn = buffers_.get(); while (dyn) { UAVCAN_ASSERT(!dyn->isEmpty()); @@ -309,29 +287,29 @@ DynamicTransferBufferManagerEntry* TransferBufferManagerImpl::findFirstDynamic(c return NULL; } -TransferBufferManagerImpl::~TransferBufferManagerImpl() +TransferBufferManager::~TransferBufferManager() { - DynamicTransferBufferManagerEntry* dyn = dynamic_buffers_.get(); + TransferBufferManagerEntry* dyn = buffers_.get(); while (dyn) { - DynamicTransferBufferManagerEntry* const next = dyn->getNextListNode(); - dynamic_buffers_.remove(dyn); - DynamicTransferBufferManagerEntry::destroy(dyn, allocator_); + TransferBufferManagerEntry* const next = dyn->getNextListNode(); + buffers_.remove(dyn); + TransferBufferManagerEntry::destroy(dyn, allocator_); dyn = next; } } -ITransferBuffer* TransferBufferManagerImpl::access(const TransferBufferManagerKey& key) +ITransferBuffer* TransferBufferManager::access(const TransferBufferManagerKey& key) { if (key.isEmpty()) { UAVCAN_ASSERT(0); return NULL; } - return findFirstDynamic(key); + return findFirst(key); } -ITransferBuffer* TransferBufferManagerImpl::create(const TransferBufferManagerKey& key) +ITransferBuffer* TransferBufferManager::create(const TransferBufferManagerKey& key) { if (key.isEmpty()) { @@ -340,13 +318,13 @@ ITransferBuffer* TransferBufferManagerImpl::create(const TransferBufferManagerKe } remove(key); - DynamicTransferBufferManagerEntry* tbme = DynamicTransferBufferManagerEntry::instantiate(allocator_, max_buf_size_); + TransferBufferManagerEntry* tbme = TransferBufferManagerEntry::instantiate(allocator_, max_buf_size_); if (tbme == NULL) { return NULL; // Epic fail. } - dynamic_buffers_.insert(tbme); + buffers_.insert(tbme); UAVCAN_TRACE("TransferBufferManager", "Buffer created [num=%u], %s", getNumBuffers(), key.toString().c_str()); @@ -358,27 +336,27 @@ ITransferBuffer* TransferBufferManagerImpl::create(const TransferBufferManagerKe return tbme; } -void TransferBufferManagerImpl::remove(const TransferBufferManagerKey& key) +void TransferBufferManager::remove(const TransferBufferManagerKey& key) { UAVCAN_ASSERT(!key.isEmpty()); - DynamicTransferBufferManagerEntry* dyn = findFirstDynamic(key); + TransferBufferManagerEntry* dyn = findFirst(key); if (dyn != NULL) { UAVCAN_TRACE("TransferBufferManager", "Buffer deleted, %s", key.toString().c_str()); - dynamic_buffers_.remove(dyn); - DynamicTransferBufferManagerEntry::destroy(dyn, allocator_); + buffers_.remove(dyn); + TransferBufferManagerEntry::destroy(dyn, allocator_); } } -bool TransferBufferManagerImpl::isEmpty() const +bool TransferBufferManager::isEmpty() const { return getNumBuffers() == 0; } -unsigned TransferBufferManagerImpl::getNumBuffers() const +unsigned TransferBufferManager::getNumBuffers() const { - return dynamic_buffers_.getLength(); + return buffers_.getLength(); } } diff --git a/libuavcan/src/transport/uc_transfer_listener.cpp b/libuavcan/src/transport/uc_transfer_listener.cpp index 59ed97dc5c..064f5dae9b 100644 --- a/libuavcan/src/transport/uc_transfer_listener.cpp +++ b/libuavcan/src/transport/uc_transfer_listener.cpp @@ -80,10 +80,10 @@ int MultiFrameIncomingTransfer::read(unsigned offset, uint8_t* data, unsigned le } /* - * TransferListenerBase::TimedOutReceiverPredicate + * TransferListener::TimedOutReceiverPredicate */ -bool TransferListenerBase::TimedOutReceiverPredicate::operator()(const TransferBufferManagerKey& key, - const TransferReceiver& value) const +bool TransferListener::TimedOutReceiverPredicate::operator()(const TransferBufferManagerKey& key, + const TransferReceiver& value) const { if (value.isTimedOut(ts_)) { @@ -101,9 +101,9 @@ bool TransferListenerBase::TimedOutReceiverPredicate::operator()(const TransferB } /* - * TransferListenerBase + * TransferListener */ -bool TransferListenerBase::checkPayloadCrc(const uint16_t compare_with, const ITransferBuffer& tbb) const +bool TransferListener::checkPayloadCrc(const uint16_t compare_with, const ITransferBuffer& tbb) const { TransferCRC crc = crc_base_; unsigned offset = 0; @@ -113,7 +113,7 @@ bool TransferListenerBase::checkPayloadCrc(const uint16_t compare_with, const IT const int res = tbb.read(offset, buf, sizeof(buf)); if (res < 0) { - UAVCAN_TRACE("TransferListenerBase", "Failed to check CRC: Buffer read failure %i", res); + UAVCAN_TRACE("TransferListener", "Failed to check CRC: Buffer read failure %i", res); return false; } if (res == 0) @@ -125,14 +125,14 @@ bool TransferListenerBase::checkPayloadCrc(const uint16_t compare_with, const IT } if (crc.get() != compare_with) { - UAVCAN_TRACE("TransferListenerBase", "CRC mismatch, expected=0x%04x, got=0x%04x", + UAVCAN_TRACE("TransferListener", "CRC mismatch, expected=0x%04x, got=0x%04x", int(compare_with), int(crc.get())); return false; } return true; } -void TransferListenerBase::handleReception(TransferReceiver& receiver, const RxFrame& frame, +void TransferListener::handleReception(TransferReceiver& receiver, const RxFrame& frame, TransferBufferAccessor& tba) { switch (receiver.addFrame(frame, tba)) @@ -155,12 +155,12 @@ void TransferListenerBase::handleReception(TransferReceiver& receiver, const RxF const ITransferBuffer* tbb = tba.access(); if (tbb == NULL) { - UAVCAN_TRACE("TransferListenerBase", "Buffer access failure, last frame: %s", frame.toString().c_str()); + UAVCAN_TRACE("TransferListener", "Buffer access failure, last frame: %s", frame.toString().c_str()); break; } if (!checkPayloadCrc(receiver.getLastTransferCrc(), *tbb)) { - UAVCAN_TRACE("TransferListenerBase", "CRC error, last frame: %s", frame.toString().c_str()); + UAVCAN_TRACE("TransferListener", "CRC error, last frame: %s", frame.toString().c_str()); break; } MultiFrameIncomingTransfer it(receiver.getLastTransferTimestampMonotonic(), @@ -177,7 +177,7 @@ void TransferListenerBase::handleReception(TransferReceiver& receiver, const RxF } } -void TransferListenerBase::handleAnonymousTransferReception(const RxFrame& frame) +void TransferListener::handleAnonymousTransferReception(const RxFrame& frame) { if (allow_anonymous_transfers_) { @@ -187,13 +187,13 @@ void TransferListenerBase::handleAnonymousTransferReception(const RxFrame& frame } } -void TransferListenerBase::cleanup(MonotonicTime ts) +void TransferListener::cleanup(MonotonicTime ts) { receivers_.removeAllWhere(TimedOutReceiverPredicate(ts, bufmgr_)); UAVCAN_ASSERT(receivers_.isEmpty() ? bufmgr_.isEmpty() : 1); } -void TransferListenerBase::handleFrame(const RxFrame& frame) +void TransferListener::handleFrame(const RxFrame& frame) { if (frame.getSrcNodeID().isUnicast()) // Normal transfer { @@ -227,7 +227,25 @@ void TransferListenerBase::handleFrame(const RxFrame& frame) } else { - UAVCAN_TRACE("TransferListenerBase", "Invalid frame: %s", frame.toString().c_str()); // Invalid frame + UAVCAN_TRACE("TransferListener", "Invalid frame: %s", frame.toString().c_str()); // Invalid frame + } +} + +/* + * TransferListenerWithFilter + */ +void TransferListenerWithFilter::handleFrame(const RxFrame& frame) +{ + if (filter_ != NULL) + { + if (filter_->shouldAcceptFrame(frame)) + { + TransferListener::handleFrame(frame); + } + } + else + { + UAVCAN_ASSERT(0); } } diff --git a/libuavcan/test/transport/dispatcher.cpp b/libuavcan/test/transport/dispatcher.cpp index b33ace4931..65724864ae 100644 --- a/libuavcan/test/transport/dispatcher.cpp +++ b/libuavcan/test/transport/dispatcher.cpp @@ -86,17 +86,17 @@ TEST(Dispatcher, Reception) makeDataType(uavcan::DataTypeKindService, 1) }; - typedef TestListener<512> Subscriber; - typedef std::auto_ptr SubscriberPtr; - static const int NUM_SUBSCRIBERS = 6; - SubscriberPtr subscribers[NUM_SUBSCRIBERS] = + typedef std::auto_ptr TestListenerPtr; + static const int MaxBufSize = 512; + static const int NumSubscribers = 6; + TestListenerPtr subscribers[NumSubscribers] = { - SubscriberPtr(new Subscriber(dispatcher.getTransferPerfCounter(), TYPES[0], pool)), // msg - SubscriberPtr(new Subscriber(dispatcher.getTransferPerfCounter(), TYPES[0], pool)), // msg // Two similar - SubscriberPtr(new Subscriber(dispatcher.getTransferPerfCounter(), TYPES[1], pool)), // msg - SubscriberPtr(new Subscriber(dispatcher.getTransferPerfCounter(), TYPES[2], pool)), // srv - SubscriberPtr(new Subscriber(dispatcher.getTransferPerfCounter(), TYPES[3], pool)), // srv - SubscriberPtr(new Subscriber(dispatcher.getTransferPerfCounter(), TYPES[3], pool)) // srv // Repeat again + TestListenerPtr(new TestListener(dispatcher.getTransferPerfCounter(), TYPES[0], MaxBufSize, pool)), // msg + TestListenerPtr(new TestListener(dispatcher.getTransferPerfCounter(), TYPES[0], MaxBufSize, pool)), // msg // Two similar + TestListenerPtr(new TestListener(dispatcher.getTransferPerfCounter(), TYPES[1], MaxBufSize, pool)), // msg + TestListenerPtr(new TestListener(dispatcher.getTransferPerfCounter(), TYPES[2], MaxBufSize, pool)), // srv + TestListenerPtr(new TestListener(dispatcher.getTransferPerfCounter(), TYPES[3], MaxBufSize, pool)), // srv + TestListenerPtr(new TestListener(dispatcher.getTransferPerfCounter(), TYPES[3], MaxBufSize, pool)) // srv // Repeat again }; static const std::string DATA[6] = @@ -139,7 +139,7 @@ TEST(Dispatcher, Reception) /* * Registration */ - for (int i = 0; i < NUM_SUBSCRIBERS; i++) + for (int i = 0; i < NumSubscribers; i++) { ASSERT_FALSE(dispatcher.hasSubscriber(subscribers[i]->getDataTypeDescriptor().getID())); ASSERT_FALSE(dispatcher.hasPublisher(subscribers[i]->getDataTypeDescriptor().getID())); @@ -153,7 +153,7 @@ TEST(Dispatcher, Reception) ASSERT_TRUE(dispatcher.registerServiceResponseListener(subscribers[4].get())); ASSERT_TRUE(dispatcher.registerServiceResponseListener(subscribers[5].get())); - for (int i = 0; i < NUM_SUBSCRIBERS; i++) + for (int i = 0; i < NumSubscribers; i++) { ASSERT_FALSE(dispatcher.hasPublisher(subscribers[i]->getDataTypeDescriptor().getID())); } @@ -177,7 +177,7 @@ TEST(Dispatcher, Reception) ASSERT_EQ(1, dispatcher.getNumServiceRequestListeners()); ASSERT_EQ(2, dispatcher.getNumServiceResponseListeners()); - for (int i = 0; i < NUM_SUBSCRIBERS; i++) + for (int i = 0; i < NumSubscribers; i++) { ASSERT_TRUE(subscribers[i]->isEmpty()); } @@ -214,7 +214,7 @@ TEST(Dispatcher, Reception) ASSERT_TRUE(subscribers[5]->matchAndPop(transfers[3])); - for (int i = 0; i < NUM_SUBSCRIBERS; i++) + for (int i = 0; i < NumSubscribers; i++) { ASSERT_TRUE(subscribers[i]->isEmpty()); } diff --git a/libuavcan/test/transport/incoming_transfer.cpp b/libuavcan/test/transport/incoming_transfer.cpp index 735d0027b6..3e1f55bc58 100644 --- a/libuavcan/test/transport/incoming_transfer.cpp +++ b/libuavcan/test/transport/incoming_transfer.cpp @@ -73,7 +73,7 @@ TEST(MultiFrameIncomingTransfer, Basic) using uavcan::MultiFrameIncomingTransfer; uavcan::PoolAllocator poolmgr; - uavcan::TransferBufferManager<256> bufmgr(poolmgr); + uavcan::TransferBufferManager bufmgr(256, poolmgr); const RxFrame frame = makeFrame(); uavcan::TransferBufferManagerKey bufmgr_key(frame.getSrcNodeID(), frame.getTransferType()); diff --git a/libuavcan/test/transport/transfer_buffer.cpp b/libuavcan/test/transport/transfer_buffer.cpp index 27d798e6fc..e3baf42f64 100644 --- a/libuavcan/test/transport/transfer_buffer.cpp +++ b/libuavcan/test/transport/transfer_buffer.cpp @@ -83,8 +83,8 @@ static const int TEST_BUFFER_SIZE = 200; TEST(StaticTransferBuffer, Basic) { - using uavcan::StaticTransferBufferManagerEntry; - StaticTransferBufferManagerEntry buf; + using uavcan::StaticTransferBuffer; + StaticTransferBuffer buf; uint8_t local_buffer[TEST_BUFFER_SIZE * 2]; const uint8_t* const test_data_ptr = reinterpret_cast(TEST_DATA.c_str()); @@ -126,15 +126,15 @@ TEST(StaticTransferBuffer, Basic) } -TEST(DynamicTransferBufferManagerEntry, Basic) +TEST(TransferBufferManagerEntry, Basic) { - using uavcan::DynamicTransferBufferManagerEntry; + using uavcan::TransferBufferManagerEntry; static const int MAX_SIZE = TEST_BUFFER_SIZE; static const int POOL_BLOCKS = 8; uavcan::PoolAllocator pool; - DynamicTransferBufferManagerEntry buf(pool, MAX_SIZE); + TransferBufferManagerEntry buf(pool, MAX_SIZE); uint8_t local_buffer[TEST_BUFFER_SIZE * 2]; const uint8_t* const test_data_ptr = reinterpret_cast(TEST_DATA.c_str()); @@ -186,7 +186,7 @@ TEST(DynamicTransferBufferManagerEntry, Basic) // Destroying the object; memory should be released ASSERT_LT(0, pool.getNumUsedBlocks()); - buf.~DynamicTransferBufferManagerEntry(); + buf.~TransferBufferManagerEntry(); ASSERT_EQ(0, pool.getNumUsedBlocks()); } @@ -232,8 +232,7 @@ TEST(TransferBufferManager, Basic) static const int POOL_BLOCKS = 100; uavcan::PoolAllocator pool; - typedef TransferBufferManager TransferBufferManagerType; - std::auto_ptr mgr(new TransferBufferManagerType(pool)); + std::auto_ptr mgr(new TransferBufferManager(MGR_MAX_BUFFER_SIZE, pool)); // Empty ASSERT_FALSE(mgr->access(TransferBufferManagerKey(0, uavcan::TransferTypeMessageBroadcast))); @@ -313,11 +312,3 @@ TEST(TransferBufferManager, Basic) mgr.reset(); ASSERT_EQ(0, pool.getNumUsedBlocks()); } - - -TEST(TransferBufferManager, EmptySpecialization) -{ - uavcan::TransferBufferManager<0> mgr; - (void)mgr; - ASSERT_GE(sizeof(void*), sizeof(mgr)); -} diff --git a/libuavcan/test/transport/transfer_listener.cpp b/libuavcan/test/transport/transfer_listener.cpp index a4ea34b2b0..cf3806c7f0 100644 --- a/libuavcan/test/transport/transfer_listener.cpp +++ b/libuavcan/test/transport/transfer_listener.cpp @@ -9,11 +9,11 @@ class TransferListenerEmulator : public IncomingTransferEmulatorBase { - uavcan::TransferListenerBase& target_; + uavcan::TransferListener& target_; const uavcan::DataTypeDescriptor data_type_; public: - TransferListenerEmulator(uavcan::TransferListenerBase& target, const uavcan::DataTypeDescriptor& type, + TransferListenerEmulator(uavcan::TransferListener& target, const uavcan::DataTypeDescriptor& type, uavcan::NodeID dst_node_id = 127) : IncomingTransferEmulatorBase(dst_node_id) , target_(target) @@ -38,7 +38,7 @@ TEST(TransferListener, BasicMFT) uavcan::PoolAllocator pool; uavcan::TransferPerfCounter perf; - TestListener<256> subscriber(perf, type, pool); + TestListener subscriber(perf, type, 256, pool); /* * Test data @@ -96,7 +96,7 @@ TEST(TransferListener, CrcFailure) static const int NUM_POOL_BLOCKS = 100; uavcan::PoolAllocator poolmgr; uavcan::TransferPerfCounter perf; - TestListener<256> subscriber(perf, type, poolmgr); // Static buffer only, 2 entries + TestListener subscriber(perf, type, 256, poolmgr); // Static buffer only, 2 entries /* * Generating transfers with damaged payload (CRC is not valid) @@ -140,7 +140,7 @@ TEST(TransferListener, BasicSFT) static const int NUM_POOL_BLOCKS = 100; uavcan::PoolAllocator poolmgr; uavcan::TransferPerfCounter perf; - TestListener<0> subscriber(perf, type, poolmgr); // Max buf size is 0, i.e. SFT-only + TestListener subscriber(perf, type, 0, poolmgr); // Max buf size is 0, i.e. SFT-only TransferListenerEmulator emulator(subscriber, type); const Transfer transfers[] = @@ -176,7 +176,7 @@ TEST(TransferListener, Cleanup) static const int NUM_POOL_BLOCKS = 100; uavcan::PoolAllocator poolmgr; uavcan::TransferPerfCounter perf; - TestListener<256> subscriber(perf, type, poolmgr); // Static buffer only, 1 entry + TestListener subscriber(perf, type, 256, poolmgr); /* * Generating transfers @@ -208,7 +208,7 @@ TEST(TransferListener, Cleanup) /* * Cleanup with huge timestamp value will remove all entries */ - static_cast(subscriber).cleanup(tsMono(100000000)); + static_cast(subscriber).cleanup(tsMono(100000000)); /* * Sending the same transfers again - they will be accepted since registres were cleared @@ -232,7 +232,7 @@ TEST(TransferListener, AnonymousTransfers) static const int NUM_POOL_BLOCKS = 100; uavcan::PoolAllocator poolmgr; uavcan::TransferPerfCounter perf; - TestListener<0> subscriber(perf, type, poolmgr); + TestListener subscriber(perf, type, 0, poolmgr); TransferListenerEmulator emulator(subscriber, type); const Transfer transfers[] = @@ -264,5 +264,5 @@ TEST(TransferListener, Sizes) { using namespace uavcan; - std::cout << "sizeof(TransferListener<64>): " << sizeof(TransferListener<64>) << std::endl; + std::cout << "sizeof(TransferListener): " << sizeof(TransferListener) << std::endl; } diff --git a/libuavcan/test/transport/transfer_receiver.cpp b/libuavcan/test/transport/transfer_receiver.cpp index 878a41a70f..dd7e85d1d2 100644 --- a/libuavcan/test/transport/transfer_receiver.cpp +++ b/libuavcan/test/transport/transfer_receiver.cpp @@ -76,10 +76,10 @@ struct Context { uavcan::PoolAllocator pool; uavcan::TransferReceiver receiver; // Must be default constructible and copyable - uavcan::TransferBufferManager bufmgr; + uavcan::TransferBufferManager bufmgr; Context() : - bufmgr(pool) + bufmgr(BufSize, pool) { } ~Context() @@ -122,7 +122,7 @@ TEST(TransferReceiver, Basic) Context<32> context; RxFrameGenerator gen(789); uavcan::TransferReceiver& rcv = context.receiver; - uavcan::ITransferBufferManager& bufmgr = context.bufmgr; + uavcan::TransferBufferManager& bufmgr = context.bufmgr; uavcan::TransferBufferAccessor bk(context.bufmgr, RxFrameGenerator::DEFAULT_KEY); std::cout << "sizeof(TransferReceiver): " << sizeof(TransferReceiver) << std::endl; @@ -261,7 +261,7 @@ TEST(TransferReceiver, OutOfBufferSpace_32bytes) Context<32> context; RxFrameGenerator gen(789); uavcan::TransferReceiver& rcv = context.receiver; - uavcan::ITransferBufferManager& bufmgr = context.bufmgr; + uavcan::TransferBufferManager& bufmgr = context.bufmgr; uavcan::TransferBufferAccessor bk(context.bufmgr, RxFrameGenerator::DEFAULT_KEY); /* @@ -299,7 +299,7 @@ TEST(TransferReceiver, OutOfOrderFrames) Context<32> context; RxFrameGenerator gen(789); uavcan::TransferReceiver& rcv = context.receiver; - uavcan::ITransferBufferManager& bufmgr = context.bufmgr; + uavcan::TransferBufferManager& bufmgr = context.bufmgr; uavcan::TransferBufferAccessor bk(context.bufmgr, RxFrameGenerator::DEFAULT_KEY); CHECK_NOT_COMPLETE(rcv.addFrame(gen(1, "1234567", SET100, 7, 100000000), bk)); @@ -323,7 +323,7 @@ TEST(TransferReceiver, IntervalMeasurement) Context<32> context; RxFrameGenerator gen(789); uavcan::TransferReceiver& rcv = context.receiver; - uavcan::ITransferBufferManager& bufmgr = context.bufmgr; + uavcan::TransferBufferManager& bufmgr = context.bufmgr; uavcan::TransferBufferAccessor bk(context.bufmgr, RxFrameGenerator::DEFAULT_KEY); static const int INTERVAL = 1000; @@ -353,7 +353,7 @@ TEST(TransferReceiver, Restart) Context<32> context; RxFrameGenerator gen(789); uavcan::TransferReceiver& rcv = context.receiver; - uavcan::ITransferBufferManager& bufmgr = context.bufmgr; + uavcan::TransferBufferManager& bufmgr = context.bufmgr; uavcan::TransferBufferAccessor bk(context.bufmgr, RxFrameGenerator::DEFAULT_KEY); /* @@ -401,7 +401,7 @@ TEST(TransferReceiver, UtcTransferTimestamping) Context<32> context; RxFrameGenerator gen(789); uavcan::TransferReceiver& rcv = context.receiver; - uavcan::ITransferBufferManager& bufmgr = context.bufmgr; + uavcan::TransferBufferManager& bufmgr = context.bufmgr; uavcan::TransferBufferAccessor bk(context.bufmgr, RxFrameGenerator::DEFAULT_KEY); /* @@ -460,7 +460,7 @@ TEST(TransferReceiver, HeaderParsing) Context<32> context; RxFrameGenerator gen(123); uavcan::TransferReceiver& rcv = context.receiver; - uavcan::ITransferBufferManager& bufmgr = context.bufmgr; + uavcan::TransferBufferManager& bufmgr = context.bufmgr; /* * MFT, message broadcasting @@ -507,7 +507,7 @@ TEST(TransferReceiver, HeaderParsing) Context<32> context; RxFrameGenerator gen(123); uavcan::TransferReceiver& rcv = context.receiver; - uavcan::ITransferBufferManager& bufmgr = context.bufmgr; + uavcan::TransferBufferManager& bufmgr = context.bufmgr; static const uavcan::TransferType ADDRESSED_TRANSFER_TYPES[2] = { diff --git a/libuavcan/test/transport/transfer_sender.cpp b/libuavcan/test/transport/transfer_sender.cpp index ff5595e19a..a156b3039e 100644 --- a/libuavcan/test/transport/transfer_sender.cpp +++ b/libuavcan/test/transport/transfer_sender.cpp @@ -123,9 +123,9 @@ TEST(TransferSender, Basic) } } - TestListener<512> sub_msg(dispatcher_rx.getTransferPerfCounter(), TYPES[0], poolmgr); - TestListener<512> sub_srv_req(dispatcher_rx.getTransferPerfCounter(), TYPES[1], poolmgr); - TestListener<512> sub_srv_resp(dispatcher_rx.getTransferPerfCounter(), TYPES[1], poolmgr); + TestListener sub_msg(dispatcher_rx.getTransferPerfCounter(), TYPES[0], 512, poolmgr); + TestListener sub_srv_req(dispatcher_rx.getTransferPerfCounter(), TYPES[1], 512, poolmgr); + TestListener sub_srv_resp(dispatcher_rx.getTransferPerfCounter(), TYPES[1], 512, poolmgr); dispatcher_rx.registerMessageListener(&sub_msg); dispatcher_rx.registerServiceRequestListener(&sub_srv_req); diff --git a/libuavcan/test/transport/transfer_test_helpers.cpp b/libuavcan/test/transport/transfer_test_helpers.cpp index f7de3410f1..e3da982602 100644 --- a/libuavcan/test/transport/transfer_test_helpers.cpp +++ b/libuavcan/test/transport/transfer_test_helpers.cpp @@ -11,7 +11,7 @@ TEST(TransferTestHelpers, Transfer) { uavcan::PoolAllocator pool; - uavcan::TransferBufferManager<128> mgr(pool); + uavcan::TransferBufferManager mgr(128, pool); uavcan::TransferBufferAccessor tba(mgr, uavcan::TransferBufferManagerKey(0, uavcan::TransferTypeMessageBroadcast)); uavcan::RxFrame frame(uavcan::Frame(123, uavcan::TransferTypeMessageBroadcast, 1, 0, 0), diff --git a/libuavcan/test/transport/transfer_test_helpers.hpp b/libuavcan/test/transport/transfer_test_helpers.hpp index d1b4f76375..aa52154ea4 100644 --- a/libuavcan/test/transport/transfer_test_helpers.hpp +++ b/libuavcan/test/transport/transfer_test_helpers.hpp @@ -117,17 +117,16 @@ struct Transfer * In reality, uavcan::TransferListener should accept only specific transfer types * which are dispatched/filtered by uavcan::Dispatcher. */ -template -class TestListener : public uavcan::TransferListener +class TestListener : public uavcan::TransferListener { - typedef uavcan::TransferListener Base; + typedef uavcan::TransferListener Base; std::queue transfers_; public: TestListener(uavcan::TransferPerfCounter& perf, const uavcan::DataTypeDescriptor& data_type, - uavcan::IPoolAllocator& allocator) - : Base(perf, data_type, allocator) + uavcan::uint16_t max_buffer_size, uavcan::IPoolAllocator& allocator) + : Base(perf, data_type, max_buffer_size, allocator) { } void handleIncomingTransfer(uavcan::IncomingTransfer& transfer) @@ -166,7 +165,7 @@ public: return res; } - int getNumReceivedTransfers() const { return transfers_.size(); } + unsigned getNumReceivedTransfers() const { return static_cast(transfers_.size()); } bool isEmpty() const { return transfers_.empty(); } }; From 0d5fc6590710f56ad597585df8809f2831536279 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Wed, 14 Oct 2015 20:02:25 +0300 Subject: [PATCH 10/25] Transfer listener .hpp --> .cpp --- libuavcan/include/uavcan/transport/transfer_listener.hpp | 6 +----- libuavcan/src/transport/uc_transfer_listener.cpp | 6 ++++++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/libuavcan/include/uavcan/transport/transfer_listener.hpp b/libuavcan/include/uavcan/transport/transfer_listener.hpp index b6d28d29f0..acf7c5999f 100644 --- a/libuavcan/include/uavcan/transport/transfer_listener.hpp +++ b/libuavcan/include/uavcan/transport/transfer_listener.hpp @@ -138,11 +138,7 @@ public: , allow_anonymous_transfers_(false) { } - virtual ~TransferListener() - { - // Map must be cleared before bufmgr is destroyed - receivers_.clear(); - } + virtual ~TransferListener(); const DataTypeDescriptor& getDataTypeDescriptor() const { return data_type_; } diff --git a/libuavcan/src/transport/uc_transfer_listener.cpp b/libuavcan/src/transport/uc_transfer_listener.cpp index 064f5dae9b..d5a7ea6280 100644 --- a/libuavcan/src/transport/uc_transfer_listener.cpp +++ b/libuavcan/src/transport/uc_transfer_listener.cpp @@ -187,6 +187,12 @@ void TransferListener::handleAnonymousTransferReception(const RxFrame& frame) } } +TransferListener::~TransferListener() +{ + // Map must be cleared before bufmgr is destroyed + receivers_.clear(); +} + void TransferListener::cleanup(MonotonicTime ts) { receivers_.removeAllWhere(TimedOutReceiverPredicate(ts, bufmgr_)); From 899aae44f7ce8b077a0bdfa478ce5a45f8c652cf Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Wed, 14 Oct 2015 20:15:19 +0300 Subject: [PATCH 11/25] OTR deinterfaced --- libuavcan/include/uavcan/node/node.hpp | 4 +- libuavcan/include/uavcan/node/scheduler.hpp | 4 +- libuavcan/include/uavcan/node/sub_node.hpp | 4 +- .../include/uavcan/transport/dispatcher.hpp | 8 +-- .../transport/outgoing_transfer_registry.hpp | 62 ++----------------- .../uc_outgoing_transfer_registry.cpp | 32 +++++++++- .../src/transport/uc_transfer_sender.cpp | 2 +- libuavcan/test/node/test_node.hpp | 4 +- libuavcan/test/transport/dispatcher.cpp | 19 ++---- libuavcan/test/transport/transfer_sender.cpp | 14 ++--- 10 files changed, 56 insertions(+), 97 deletions(-) diff --git a/libuavcan/include/uavcan/node/node.hpp b/libuavcan/include/uavcan/node/node.hpp index d9b6502736..4b573bf42a 100644 --- a/libuavcan/include/uavcan/node/node.hpp +++ b/libuavcan/include/uavcan/node/node.hpp @@ -47,7 +47,6 @@ class UAVCAN_EXPORT Node : public INode typedef PoolAllocator Allocator; Allocator pool_allocator_; - OutgoingTransferRegistry outgoing_trans_reg_; Scheduler scheduler_; NodeStatusProvider proto_nsp_; @@ -75,8 +74,7 @@ protected: public: Node(ICanDriver& can_driver, ISystemClock& system_clock) - : outgoing_trans_reg_(pool_allocator_) - , scheduler_(can_driver, pool_allocator_, system_clock, outgoing_trans_reg_) + : scheduler_(can_driver, pool_allocator_, system_clock) , proto_nsp_(*this) #if !UAVCAN_TINY , proto_dtp_(*this) diff --git a/libuavcan/include/uavcan/node/scheduler.hpp b/libuavcan/include/uavcan/node/scheduler.hpp index 01e58eb877..8f008054a8 100644 --- a/libuavcan/include/uavcan/node/scheduler.hpp +++ b/libuavcan/include/uavcan/node/scheduler.hpp @@ -92,8 +92,8 @@ class UAVCAN_EXPORT Scheduler : Noncopyable void pollCleanup(MonotonicTime mono_ts, uint32_t num_frames_processed_with_last_spin); public: - Scheduler(ICanDriver& can_driver, IPoolAllocator& allocator, ISystemClock& sysclock, IOutgoingTransferRegistry& otr) - : dispatcher_(can_driver, allocator, sysclock, otr) + Scheduler(ICanDriver& can_driver, IPoolAllocator& allocator, ISystemClock& sysclock) + : dispatcher_(can_driver, allocator, sysclock) , prev_cleanup_ts_(sysclock.getMonotonic()) , deadline_resolution_(MonotonicDuration::fromMSec(DefaultDeadlineResolutionMs)) , cleanup_period_(MonotonicDuration::fromMSec(DefaultCleanupPeriodMs)) diff --git a/libuavcan/include/uavcan/node/sub_node.hpp b/libuavcan/include/uavcan/node/sub_node.hpp index 2a7de956ef..ef5881cb12 100644 --- a/libuavcan/include/uavcan/node/sub_node.hpp +++ b/libuavcan/include/uavcan/node/sub_node.hpp @@ -31,7 +31,6 @@ class UAVCAN_EXPORT SubNode : public INode typedef PoolAllocator Allocator; Allocator pool_allocator_; - OutgoingTransferRegistry outgoing_trans_reg_; Scheduler scheduler_; uint64_t internal_failure_cnt_; @@ -46,8 +45,7 @@ protected: public: SubNode(ICanDriver& can_driver, ISystemClock& system_clock) : - outgoing_trans_reg_(pool_allocator_), - scheduler_(can_driver, pool_allocator_, system_clock, outgoing_trans_reg_), + scheduler_(can_driver, pool_allocator_, system_clock), internal_failure_cnt_(0) { } diff --git a/libuavcan/include/uavcan/transport/dispatcher.hpp b/libuavcan/include/uavcan/transport/dispatcher.hpp index 33ac1f2e5c..c4f40e00a3 100644 --- a/libuavcan/include/uavcan/transport/dispatcher.hpp +++ b/libuavcan/include/uavcan/transport/dispatcher.hpp @@ -81,7 +81,7 @@ class UAVCAN_EXPORT Dispatcher : Noncopyable { CanIOManager canio_; ISystemClock& sysclock_; - IOutgoingTransferRegistry& outgoing_transfer_reg_; + OutgoingTransferRegistry outgoing_transfer_reg_; TransferPerfCounter perf_; class ListenerRegistry @@ -133,10 +133,10 @@ class UAVCAN_EXPORT Dispatcher : Noncopyable void notifyRxFrameListener(const CanRxFrame& can_frame, CanIOFlags flags); public: - Dispatcher(ICanDriver& driver, IPoolAllocator& allocator, ISystemClock& sysclock, IOutgoingTransferRegistry& otr) + Dispatcher(ICanDriver& driver, IPoolAllocator& allocator, ISystemClock& sysclock) : canio_(driver, allocator, sysclock) , sysclock_(sysclock) - , outgoing_transfer_reg_(otr) + , outgoing_transfer_reg_(allocator) #if !UAVCAN_TINY , rx_listener_(NULL) #endif @@ -201,7 +201,7 @@ public: * @} */ - IOutgoingTransferRegistry& getOutgoingTransferRegistry() { return outgoing_transfer_reg_; } + OutgoingTransferRegistry& getOutgoingTransferRegistry() { return outgoing_transfer_reg_; } #if !UAVCAN_TINY LoopbackFrameListenerRegistry& getLoopbackFrameListenerRegistry() { return loopback_listeners_; } diff --git a/libuavcan/include/uavcan/transport/outgoing_transfer_registry.hpp b/libuavcan/include/uavcan/transport/outgoing_transfer_registry.hpp index c6d2c0c412..f92636e540 100644 --- a/libuavcan/include/uavcan/transport/outgoing_transfer_registry.hpp +++ b/libuavcan/include/uavcan/transport/outgoing_transfer_registry.hpp @@ -60,23 +60,8 @@ public: * Outgoing transfer registry keeps track of Transfer ID values for all currently existing local transfer senders. * If a local transfer sender was inactive for a sufficiently long time, the outgoing transfer registry will * remove the respective Transfer ID tracking object. - * - * TODO: Deinterface this. */ -class UAVCAN_EXPORT IOutgoingTransferRegistry -{ -public: - static const MonotonicDuration MinEntryLifetime; - - virtual ~IOutgoingTransferRegistry() { } - - virtual TransferID* accessOrCreate(const OutgoingTransferRegistryKey& key, MonotonicTime new_deadline) = 0; - virtual bool exists(DataTypeID dtid, TransferType tt) const = 0; - virtual void cleanup(MonotonicTime deadline) = 0; -}; - - -class UAVCAN_EXPORT OutgoingTransferRegistry : public IOutgoingTransferRegistry, Noncopyable +class UAVCAN_EXPORT OutgoingTransferRegistry : Noncopyable { struct Value { @@ -127,54 +112,19 @@ class UAVCAN_EXPORT OutgoingTransferRegistry : public IOutgoingTransferRegistry, Map map_; public: + static const MonotonicDuration MinEntryLifetime; + explicit OutgoingTransferRegistry(IPoolAllocator& allocator) : map_(allocator) { } - virtual TransferID* accessOrCreate(const OutgoingTransferRegistryKey& key, MonotonicTime new_deadline); + TransferID* accessOrCreate(const OutgoingTransferRegistryKey& key, MonotonicTime new_deadline); - virtual bool exists(DataTypeID dtid, TransferType tt) const; + bool exists(DataTypeID dtid, TransferType tt) const; - virtual void cleanup(MonotonicTime ts); + void cleanup(MonotonicTime ts); }; -// ---------------------------------------------------------------------------- - -/* - * OutgoingTransferRegistry - * TODO: deinterface and move to .cpp - */ -inline -TransferID* OutgoingTransferRegistry::accessOrCreate(const OutgoingTransferRegistryKey& key, - MonotonicTime new_deadline) -{ - UAVCAN_ASSERT(!new_deadline.isZero()); - Value* p = map_.access(key); - if (p == NULL) - { - p = map_.insert(key, Value()); - if (p == NULL) - { - return NULL; - } - UAVCAN_TRACE("OutgoingTransferRegistry", "Created %s", key.toString().c_str()); - } - p->deadline = new_deadline; - return &p->tid; -} - -inline -bool OutgoingTransferRegistry::exists(DataTypeID dtid, TransferType tt) const -{ - return NULL != map_.find(ExistenceCheckingPredicate(dtid, tt)); -} - -inline -void OutgoingTransferRegistry::cleanup(MonotonicTime ts) -{ - map_.removeAllWhere(DeadlineExpiredPredicate(ts)); -} - } #endif // UAVCAN_TRANSPORT_OUTGOING_TRANSFER_REGISTRY_HPP_INCLUDED diff --git a/libuavcan/src/transport/uc_outgoing_transfer_registry.cpp b/libuavcan/src/transport/uc_outgoing_transfer_registry.cpp index f840fa8cc6..852e850ccb 100644 --- a/libuavcan/src/transport/uc_outgoing_transfer_registry.cpp +++ b/libuavcan/src/transport/uc_outgoing_transfer_registry.cpp @@ -20,8 +20,36 @@ std::string OutgoingTransferRegistryKey::toString() const #endif /* - * IOutgoingTransferRegistry + * OutgoingTransferRegistry */ -const MonotonicDuration IOutgoingTransferRegistry::MinEntryLifetime = MonotonicDuration::fromMSec(2000); +const MonotonicDuration OutgoingTransferRegistry::MinEntryLifetime = MonotonicDuration::fromMSec(2000); + +TransferID* OutgoingTransferRegistry::accessOrCreate(const OutgoingTransferRegistryKey& key, + MonotonicTime new_deadline) +{ + UAVCAN_ASSERT(!new_deadline.isZero()); + Value* p = map_.access(key); + if (p == NULL) + { + p = map_.insert(key, Value()); + if (p == NULL) + { + return NULL; + } + UAVCAN_TRACE("OutgoingTransferRegistry", "Created %s", key.toString().c_str()); + } + p->deadline = new_deadline; + return &p->tid; +} + +bool OutgoingTransferRegistry::exists(DataTypeID dtid, TransferType tt) const +{ + return NULL != map_.find(ExistenceCheckingPredicate(dtid, tt)); +} + +void OutgoingTransferRegistry::cleanup(MonotonicTime ts) +{ + map_.removeAllWhere(DeadlineExpiredPredicate(ts)); +} } diff --git a/libuavcan/src/transport/uc_transfer_sender.cpp b/libuavcan/src/transport/uc_transfer_sender.cpp index f0829976a6..0a9dc4a700 100644 --- a/libuavcan/src/transport/uc_transfer_sender.cpp +++ b/libuavcan/src/transport/uc_transfer_sender.cpp @@ -153,7 +153,7 @@ int TransferSender::send(const uint8_t* payload, unsigned payload_len, Monotonic UAVCAN_ASSERT(!tx_deadline.isZero()); const MonotonicTime otr_deadline = tx_deadline + max(max_transfer_interval_ * 2, - IOutgoingTransferRegistry::MinEntryLifetime); + OutgoingTransferRegistry::MinEntryLifetime); TransferID* const tid = dispatcher_.getOutgoingTransferRegistry().accessOrCreate(otr_key, otr_deadline); if (tid == NULL) diff --git a/libuavcan/test/node/test_node.hpp b/libuavcan/test/node/test_node.hpp index ea187c809f..bd71b89ee3 100644 --- a/libuavcan/test/node/test_node.hpp +++ b/libuavcan/test/node/test_node.hpp @@ -20,13 +20,11 @@ struct TestNode : public uavcan::INode { uavcan::PoolAllocator pool; - uavcan::OutgoingTransferRegistry otr; uavcan::Scheduler scheduler; uint64_t internal_failure_count; TestNode(uavcan::ICanDriver& can_driver, uavcan::ISystemClock& clock_driver, uavcan::NodeID self_node_id) - : otr(pool) - , scheduler(can_driver, pool, clock_driver, otr) + : scheduler(can_driver, pool, clock_driver) , internal_failure_count(0) { setNodeID(self_node_id); diff --git a/libuavcan/test/transport/dispatcher.cpp b/libuavcan/test/transport/dispatcher.cpp index 65724864ae..621ccf8972 100644 --- a/libuavcan/test/transport/dispatcher.cpp +++ b/libuavcan/test/transport/dispatcher.cpp @@ -57,9 +57,7 @@ TEST(Dispatcher, Reception) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry out_trans_reg(pool); - - uavcan::Dispatcher dispatcher(driver, pool, clockmock, out_trans_reg); + uavcan::Dispatcher dispatcher(driver, pool, clockmock); ASSERT_TRUE(dispatcher.setNodeID(SELF_NODE_ID)); // Can be set only once ASSERT_FALSE(dispatcher.setNodeID(SELF_NODE_ID)); ASSERT_EQ(SELF_NODE_ID, dispatcher.getNodeID()); @@ -255,9 +253,7 @@ TEST(Dispatcher, Transmission) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry out_trans_reg(pool); - - uavcan::Dispatcher dispatcher(driver, pool, clockmock, out_trans_reg); + uavcan::Dispatcher dispatcher(driver, pool, clockmock); ASSERT_TRUE(dispatcher.setNodeID(SELF_NODE_ID)); // Can be set only once ASSERT_FALSE(dispatcher.setNodeID(SELF_NODE_ID)); @@ -280,7 +276,8 @@ TEST(Dispatcher, Transmission) ASSERT_FALSE(dispatcher.hasPublisher(123)); ASSERT_FALSE(dispatcher.hasPublisher(456)); const uavcan::OutgoingTransferRegistryKey otr_key(123, uavcan::TransferTypeMessageBroadcast, 0); - ASSERT_TRUE(out_trans_reg.accessOrCreate(otr_key, uavcan::MonotonicTime::fromMSec(1000000))); + ASSERT_TRUE(dispatcher.getOutgoingTransferRegistry().accessOrCreate(otr_key, + uavcan::MonotonicTime::fromMSec(1000000))); ASSERT_TRUE(dispatcher.hasPublisher(123)); ASSERT_FALSE(dispatcher.hasPublisher(456)); @@ -319,9 +316,7 @@ TEST(Dispatcher, Spin) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry out_trans_reg(poolmgr); - - uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock, out_trans_reg); + uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock); ASSERT_TRUE(dispatcher.setNodeID(SELF_NODE_ID)); // Can be set only once ASSERT_FALSE(dispatcher.setNodeID(SELF_NODE_ID)); @@ -365,9 +360,7 @@ TEST(Dispatcher, Loopback) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry out_trans_reg(poolmgr); - - uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock, out_trans_reg); + uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock); ASSERT_TRUE(dispatcher.setNodeID(SELF_NODE_ID)); { diff --git a/libuavcan/test/transport/transfer_sender.cpp b/libuavcan/test/transport/transfer_sender.cpp index a156b3039e..ff27b6fe98 100644 --- a/libuavcan/test/transport/transfer_sender.cpp +++ b/libuavcan/test/transport/transfer_sender.cpp @@ -34,12 +34,10 @@ TEST(TransferSender, Basic) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry out_trans_reg(poolmgr); - static const uavcan::NodeID TX_NODE_ID(64); static const uavcan::NodeID RX_NODE_ID(65); - uavcan::Dispatcher dispatcher_tx(driver, poolmgr, clockmock, out_trans_reg); - uavcan::Dispatcher dispatcher_rx(driver, poolmgr, clockmock, out_trans_reg); + uavcan::Dispatcher dispatcher_tx(driver, poolmgr, clockmock); + uavcan::Dispatcher dispatcher_rx(driver, poolmgr, clockmock); ASSERT_TRUE(dispatcher_tx.setNodeID(TX_NODE_ID)); ASSERT_TRUE(dispatcher_rx.setNodeID(RX_NODE_ID)); @@ -195,10 +193,8 @@ TEST(TransferSender, Loopback) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry out_trans_reg(poolmgr); - static const uavcan::NodeID TX_NODE_ID(64); - uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock, out_trans_reg); + uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock); ASSERT_TRUE(dispatcher.setNodeID(TX_NODE_ID)); uavcan::DataTypeDescriptor desc = makeDataType(uavcan::DataTypeKindMessage, 1, "Foobar"); @@ -236,9 +232,7 @@ TEST(TransferSender, PassiveMode) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry out_trans_reg(poolmgr); - - uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock, out_trans_reg); + uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock); uavcan::TransferSender sender(dispatcher, makeDataType(uavcan::DataTypeKindMessage, 123), uavcan::CanTxQueue::Volatile); From 23352746cda0dee319485d3062c970b906c91d32 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Wed, 14 Oct 2015 20:30:24 +0300 Subject: [PATCH 12/25] Speed-optimized versions of bitarrayCopy() removed --- .../include/uavcan/marshal/bit_stream.hpp | 28 ---- libuavcan/src/marshal/uc_bit_array_copy.cpp | 148 ------------------ 2 files changed, 176 deletions(-) diff --git a/libuavcan/include/uavcan/marshal/bit_stream.hpp b/libuavcan/include/uavcan/marshal/bit_stream.hpp index 1ac9fe400c..17ed92f2ea 100644 --- a/libuavcan/include/uavcan/marshal/bit_stream.hpp +++ b/libuavcan/include/uavcan/marshal/bit_stream.hpp @@ -13,8 +13,6 @@ namespace uavcan { - -#if UAVCAN_TINY /** * This function implements fast copy of unaligned bit arrays. It isn't part of the library API, so it is not exported. * @param src_org Source array @@ -25,16 +23,6 @@ namespace uavcan */ void bitarrayCopy(const unsigned char* src_org, unsigned src_offset, unsigned src_len, unsigned char* dst_org, unsigned dst_offset); -#else -/** - * Special cases of @ref bitarrayCopy() - either source or destination must be aligned. - * These functions aren't part of the library API, so they are not exported. - */ -void bitarrayCopyAlignedToUnaligned(const unsigned char* src_org, unsigned src_len, - unsigned char* dst_org, unsigned dst_offset); -void bitarrayCopyUnalignedToAligned(const unsigned char* src_org, unsigned src_offset, unsigned src_len, - unsigned char* dst_org); -#endif /** * This class treats a chunk of memory as an array of bits. @@ -50,7 +38,6 @@ class UAVCAN_EXPORT BitStream static inline unsigned bitlenToBytelen(unsigned bits) { return (bits + 7) / 8; } -#if UAVCAN_TINY static inline void copyBitArrayAlignedToUnaligned(const uint8_t* src_org, unsigned src_len, uint8_t* dst_org, unsigned dst_offset) { @@ -64,21 +51,6 @@ class UAVCAN_EXPORT BitStream bitarrayCopy(reinterpret_cast(src_org), src_offset, src_len, reinterpret_cast(dst_org), 0); } -#else - static inline void copyBitArrayAlignedToUnaligned(const uint8_t* src_org, unsigned src_len, - uint8_t* dst_org, unsigned dst_offset) - { - bitarrayCopyAlignedToUnaligned(reinterpret_cast(src_org), src_len, - reinterpret_cast(dst_org), dst_offset); - } - - static inline void copyBitArrayUnalignedToAligned(const uint8_t* src_org, unsigned src_offset, unsigned src_len, - uint8_t* dst_org) - { - bitarrayCopyUnalignedToAligned(reinterpret_cast(src_org), src_offset, src_len, - reinterpret_cast(dst_org)); - } -#endif public: static const unsigned MaxBitsPerRW = MaxBytesPerRW * 8; diff --git a/libuavcan/src/marshal/uc_bit_array_copy.cpp b/libuavcan/src/marshal/uc_bit_array_copy.cpp index 034b752584..461d007ca0 100644 --- a/libuavcan/src/marshal/uc_bit_array_copy.cpp +++ b/libuavcan/src/marshal/uc_bit_array_copy.cpp @@ -14,8 +14,6 @@ namespace uavcan static const unsigned char reverse_mask[] = { 0x00U, 0x80U, 0xC0U, 0xE0U, 0xF0U, 0xF8U, 0xFCU, 0xFEU, 0xFFU }; static const unsigned char reverse_mask_xor[] = { 0xFFU, 0x7FU, 0x3FU, 0x1FU, 0x0FU, 0x07U, 0x03U, 0x01U, 0x00U }; -#if UAVCAN_TINY - #define PREPARE_FIRST_COPY() \ do { \ if (src_len >= (CHAR_BIT - dst_offset_modulo)) { \ @@ -120,150 +118,4 @@ void bitarrayCopy(const unsigned char* src_org, unsigned src_offset, unsigned sr } } -#else - -/* - * Functions below were manually optimized in the most horrible way. - */ - -void bitarrayCopyAlignedToUnaligned(const unsigned char* src_org, unsigned src_len, - unsigned char* dst_org, unsigned dst_offset) -{ - if (src_len > 0U) - { - unsigned char* dst = dst_org + (dst_offset / CHAR_BIT); - const unsigned dst_offset_modulo = dst_offset % CHAR_BIT; - - if (0U == dst_offset_modulo) - { - const unsigned byte_len = src_len / CHAR_BIT; - const unsigned src_len_modulo = src_len % CHAR_BIT; - - if (byte_len > 0U) - { - (void)std::memcpy(dst, src_org, byte_len); - src_org += byte_len; - dst += byte_len; - } - if (src_len_modulo > 0U) - { - *dst &= reverse_mask_xor[src_len_modulo]; - *dst |= reverse_mask[src_len_modulo] & *src_org; - } - } - else - { - const unsigned bit_diff_ls = CHAR_BIT - dst_offset_modulo; - unsigned char c = - static_cast(*src_org >> dst_offset_modulo & reverse_mask_xor[dst_offset_modulo]); - - if (src_len >= (CHAR_BIT - dst_offset_modulo)) - { - *dst &= reverse_mask[dst_offset_modulo]; - src_len -= CHAR_BIT - dst_offset_modulo; - } - else - { - *dst &= reverse_mask[dst_offset_modulo] | reverse_mask_xor[dst_offset_modulo + src_len + 1]; - c &= reverse_mask[dst_offset_modulo + src_len]; - src_len = 0; - } - - *dst++ |= c; - - int byte_len = int(src_len / CHAR_BIT); - - while (--byte_len >= 0) - { - c = static_cast(*src_org++ << bit_diff_ls); - c = static_cast(c | (*src_org >> dst_offset_modulo)); - *dst++ = c; - } - - const unsigned src_len_modulo = src_len % CHAR_BIT; - if (src_len_modulo > 0U) - { - c = static_cast(*src_org++ << bit_diff_ls); - c = static_cast(c | (*src_org >> dst_offset_modulo)); - c &= reverse_mask[src_len_modulo]; - - *dst &= reverse_mask_xor[src_len_modulo]; - *dst |= c; - } - } - } -} - -void bitarrayCopyUnalignedToAligned(const unsigned char* src_org, unsigned src_offset, unsigned src_len, - unsigned char* dst_org) -{ - if (src_len > 0U) - { - const unsigned char* src = src_org + (src_offset / CHAR_BIT); - - const unsigned src_offset_modulo = src_offset % CHAR_BIT; - - if (src_offset_modulo == 0U) - { - const unsigned byte_len = src_len / CHAR_BIT; - const unsigned src_len_modulo = src_len % CHAR_BIT; - - if (byte_len > 0U) - { - (void)std::memcpy(dst_org, src, byte_len); - src += byte_len; - dst_org += byte_len; - } - if (src_len_modulo > 0U) - { - *dst_org &= reverse_mask_xor[src_len_modulo]; - *dst_org |= reverse_mask[src_len_modulo] & *src; - } - } - else - { - const unsigned bit_diff_rs = CHAR_BIT - src_offset_modulo; - - unsigned char c = static_cast(*src++ << src_offset_modulo); - c = static_cast(c | (*src >> bit_diff_rs)); - - if (src_len >= CHAR_BIT) - { - *dst_org &= 0x55U; - src_len -= CHAR_BIT; - } - else - { - *dst_org &= 0x55U | reverse_mask_xor[src_len + 1]; - c &= reverse_mask[src_len]; - src_len = 0; - } - - *dst_org++ |= c; - - int byte_len = int(src_len / CHAR_BIT); - - while (--byte_len >= 0) - { - c = static_cast(*src++ << src_offset_modulo); - c = static_cast(c | (*src >> bit_diff_rs)); - *dst_org++ = c; - } - - const unsigned src_len_modulo = src_len % CHAR_BIT; - if (src_len_modulo > 0U) - { - c = static_cast(*src++ << src_offset_modulo); - c = static_cast(c | (*src >> bit_diff_rs)); - c &= reverse_mask[src_len_modulo]; - - *dst_org &= reverse_mask_xor[src_len_modulo]; - *dst_org |= c; - } - } - } -} - -#endif - } From d1511bed5cd486d9c9b2951a014bc7bd1c64d2e6 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Wed, 14 Oct 2015 21:36:13 +0300 Subject: [PATCH 13/25] Node<> and SubNode<> support custom allocators --- libuavcan/include/uavcan/node/node.hpp | 64 +++++++++++++++------- libuavcan/include/uavcan/node/sub_node.hpp | 32 ++++++++--- 2 files changed, 68 insertions(+), 28 deletions(-) diff --git a/libuavcan/include/uavcan/node/node.hpp b/libuavcan/include/uavcan/node/node.hpp index 4b573bf42a..d270c8adf9 100644 --- a/libuavcan/include/uavcan/node/node.hpp +++ b/libuavcan/include/uavcan/node/node.hpp @@ -30,21 +30,18 @@ namespace uavcan * This is the top-level node API. * A custom node class can be implemented if needed, in which case it shall inherit INode. * - * @tparam MemPoolSize_ Size of memory pool for this node, in bytes. - * Minimum recommended size is 4K * (number of CAN ifaces + 1). - * For simple nodes this number can be reduced. - * For high-traffic nodes the recommended minimum is - * like 16K * (number of CAN ifaces + 1). + * @tparam MemPoolSize Size of memory pool for this node, in bytes. + * Please refer to the documentation for details. + * If this value is zero, the constructor will accept a reference to user-provided allocator. */ -template +template class UAVCAN_EXPORT Node : public INode { - enum - { - MemPoolSize = (MemPoolSize_ < std::size_t(MemPoolBlockSize)) ? std::size_t(MemPoolBlockSize) : MemPoolSize_ - }; - - typedef PoolAllocator Allocator; + typedef typename + Select<(MemPoolSize > 0), + PoolAllocator, // If pool size is specified, use default allocator + IPoolAllocator& // Otherwise use reference to user-provided allocator + >::Result Allocator; Allocator pool_allocator_; Scheduler scheduler_; @@ -60,6 +57,12 @@ class UAVCAN_EXPORT Node : public INode uint64_t internal_failure_cnt_; bool started_; + void commonInit() + { + internal_failure_cnt_ = 0; + started_ = false; + } + protected: virtual void registerInternalFailure(const char* msg) { @@ -73,20 +76,43 @@ protected: } public: - Node(ICanDriver& can_driver, ISystemClock& system_clock) - : scheduler_(can_driver, pool_allocator_, system_clock) - , proto_nsp_(*this) + /** + * This overload is only valid if MemPoolSize > 0. + */ + Node(ICanDriver& can_driver, + ISystemClock& system_clock) : + scheduler_(can_driver, pool_allocator_, system_clock), + proto_nsp_(*this) #if !UAVCAN_TINY , proto_dtp_(*this) , proto_logger_(*this) , proto_rrs_(*this) , proto_tsp_(*this) #endif - , internal_failure_cnt_(0) - , started_(false) - { } + { + commonInit(); + } - virtual Allocator& getAllocator() { return pool_allocator_; } + /** + * This overload is only valid if MemPoolSize == 0. + */ + Node(ICanDriver& can_driver, + ISystemClock& system_clock, + IPoolAllocator& allocator) : + pool_allocator_(allocator), + scheduler_(can_driver, pool_allocator_, system_clock), + proto_nsp_(*this) +#if !UAVCAN_TINY + , proto_dtp_(*this) + , proto_logger_(*this) + , proto_rrs_(*this) + , proto_tsp_(*this) +#endif + { + commonInit(); + } + + virtual typename RemoveReference::Type& getAllocator() { return pool_allocator_; } virtual Scheduler& getScheduler() { return scheduler_; } virtual const Scheduler& getScheduler() const { return scheduler_; } diff --git a/libuavcan/include/uavcan/node/sub_node.hpp b/libuavcan/include/uavcan/node/sub_node.hpp index ef5881cb12..438d1d6815 100644 --- a/libuavcan/include/uavcan/node/sub_node.hpp +++ b/libuavcan/include/uavcan/node/sub_node.hpp @@ -20,15 +20,14 @@ namespace uavcan * Please refer to the @ref Node<> for documentation concerning the template arguments; refer to the tutorials * to lean how to use libuavcan in multiprocess applications. */ -template +template class UAVCAN_EXPORT SubNode : public INode { - enum - { - MemPoolSize = (MemPoolSize_ < std::size_t(MemPoolBlockSize)) ? std::size_t(MemPoolBlockSize) : MemPoolSize_ - }; - - typedef PoolAllocator Allocator; + typedef typename + Select<(MemPoolSize > 0), + PoolAllocator, // If pool size is specified, use default allocator + IPoolAllocator& // Otherwise use reference to user-provided allocator + >::Result Allocator; Allocator pool_allocator_; Scheduler scheduler_; @@ -44,12 +43,27 @@ protected: } public: - SubNode(ICanDriver& can_driver, ISystemClock& system_clock) : + /** + * This overload is only valid if MemPoolSize > 0. + */ + SubNode(ICanDriver& can_driver, + ISystemClock& system_clock) : scheduler_(can_driver, pool_allocator_, system_clock), internal_failure_cnt_(0) { } - virtual Allocator& getAllocator() { return pool_allocator_; } + /** + * This overload is only valid if MemPoolSize == 0. + */ + SubNode(ICanDriver& can_driver, + ISystemClock& system_clock, + IPoolAllocator& allocator) : + pool_allocator_(allocator), + scheduler_(can_driver, pool_allocator_, system_clock), + internal_failure_cnt_(0) + { } + + virtual typename RemoveReference::Type& getAllocator() { return pool_allocator_; } virtual Scheduler& getScheduler() { return scheduler_; } virtual const Scheduler& getScheduler() const { return scheduler_; } From a5d3895cbf99f9e038864bc52bbeacb2b2561862 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Thu, 15 Oct 2015 03:54:48 +0300 Subject: [PATCH 14/25] HeapBasedPoolAllocator<> with a basic test --- .../helpers/heap_based_pool_allocator.hpp | 159 ++++++++++++++++++ .../helpers/heap_based_pool_allocator.cpp | 50 ++++++ 2 files changed, 209 insertions(+) create mode 100644 libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp create mode 100644 libuavcan/test/helpers/heap_based_pool_allocator.cpp diff --git a/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp b/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp new file mode 100644 index 0000000000..a012ee2c9a --- /dev/null +++ b/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp @@ -0,0 +1,159 @@ +/* + * Copyright (C) 2015 Pavel Kirienko + */ + +#ifndef UAVCAN_HELPERS_HEAP_BASED_POOL_ALLOCATOR_HPP_INCLUDED +#define UAVCAN_HELPERS_HEAP_BASED_POOL_ALLOCATOR_HPP_INCLUDED + +#include +#include + +namespace uavcan +{ +/** + * A special-purpose implementation of a pool allocator that keeps the pool in the heap using malloc()/free(). + * The pool grows dynamically, ad-hoc, thus using as little memory as possible. + * + * Allocated blocks will not be freed back automatically, but there are two ways to force their deallocation: + * - Call @ref shrink() - this method frees all blocks that are unused at the moment. + * - Destroy the object - the desctructor calls @ref shrink(). + * + * All operations are thread-safe, the safety is obtained through the standard atomic compare-and-swap operator (CAS). + * If thread safety is not required, the CAS operator can be replaced with a dummy version that simply performs the + * assignment without any checks. + * If thread safety is required, but the hardware does not support atomic exchange operations, CAS can be emulated + * with a mutex or a critical section roughly as follows: + * + * bool softwareAtomicCompareAndSwap(void** address, void* expected, void* replacement) + * { + * RaiiSynchronizationPrimitive lock; + * if (*address != expected) + * { + * return false; + * } + * *address = replacement; + * return true; + * } + */ +template +class UAVCAN_EXPORT HeapBasedPoolAllocator : public IPoolAllocator, Noncopyable +{ + union Node + { + Node* next; + private: + uint8_t data[BlockSize]; + long double _aligner1; + long long _aligner2; + }; + + Node* volatile cache_; + + Node* popCache() + { + // http://www.ibm.com/developerworks/aix/library/au-multithreaded_structures2/ + for (;;) + { + Node* volatile const result = cache_; + if (result == NULL) + { + break; + } + if ((cache_ != NULL) && + AtomicCompareAndSwap(reinterpret_cast(const_cast(&cache_)), result, result->next)) + { + return result; + } + } + return NULL; + } + +public: + /** + * The allocator initializes with empty cache, so first allocations will be served from heap. + */ + HeapBasedPoolAllocator() : cache_(NULL) { } + + /** + * The destructor de-allocates all blocks that are currently in the cache. + * BLOCKS THAT ARE CURRENTLY HELD BY THE APPLICATION WILL LEAK. + */ + ~HeapBasedPoolAllocator() { shrink(); } + + /** + * Takes a block from the cache, unless it's empty. + * In the latter case, allocates a new block in the heap. + */ + virtual void* allocate(std::size_t size) + { + if (size > BlockSize) + { + return NULL; + } + if (Node* n = popCache()) + { + return n; + } + else + { + return std::malloc(sizeof(Node)); + } + } + + /** + * Puts the block back to cache. + * The block will not be free()d automatically; see @ref shrink(). + */ + virtual void deallocate(const void* ptr) + { + if (ptr == NULL) + { + return; + } + + Node* volatile const n = static_cast(const_cast(ptr)); + do + { + n->next = cache_; + } + while (!AtomicCompareAndSwap(reinterpret_cast(const_cast(&cache_)), n->next, n)); + } + + /** + * Heap-based pool is virutally infinite in size, so this method just returns maximum possible number of blocks. + */ + virtual uint16_t getNumBlocks() const { return NumericTraits::max(); } + + /** + * Frees all blocks that are not in use at the moment. + */ + void shrink() + { + while (Node* p = popCache()) + { + std::free(p); + } + } + + /** + * This function naively counts the number of cached blocks. + * It is not thread-safe and is mostly designed for testing and debugging purposes. + * Don't use it in production code. + */ + unsigned getNumCachedBlocks() + { + unsigned out = 0; + Node* p = cache_; + while (p != NULL) + { + out++; + p = p->next; + } + return out; + } +}; + +} + +#endif diff --git a/libuavcan/test/helpers/heap_based_pool_allocator.cpp b/libuavcan/test/helpers/heap_based_pool_allocator.cpp new file mode 100644 index 0000000000..93da37130f --- /dev/null +++ b/libuavcan/test/helpers/heap_based_pool_allocator.cpp @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2015 Pavel Kirienko + */ + +#include +#include + +inline bool atomicCompareAndSwap(void** address, void* expected, void* replacement) +{ + return __sync_bool_compare_and_swap(address, expected, replacement); +} + +TEST(ThreadSafeHeapBasedPoolAllocator, Basic) +{ + uavcan::HeapBasedPoolAllocator allocator; + + ASSERT_EQ(0, allocator.getNumCachedBlocks()); + + void* a = allocator.allocate(10); + void* b = allocator.allocate(10); + void* c = allocator.allocate(10); + void* d = allocator.allocate(10); + + ASSERT_EQ(0, allocator.getNumCachedBlocks()); + + allocator.deallocate(a); + ASSERT_EQ(1, allocator.getNumCachedBlocks()); + + allocator.deallocate(b); + ASSERT_EQ(2, allocator.getNumCachedBlocks()); + + allocator.deallocate(c); + ASSERT_EQ(3, allocator.getNumCachedBlocks()); + + a = allocator.allocate(10); + ASSERT_EQ(2, allocator.getNumCachedBlocks()); + ASSERT_EQ(c, a); + + allocator.deallocate(a); + ASSERT_EQ(3, allocator.getNumCachedBlocks()); + + allocator.shrink(); + ASSERT_EQ(0, allocator.getNumCachedBlocks()); + + allocator.deallocate(d); + ASSERT_EQ(1, allocator.getNumCachedBlocks()); + + allocator.shrink(); + ASSERT_EQ(0, allocator.getNumCachedBlocks()); +} From edadf58a914951201278d52ae05e515164b6f8b7 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Thu, 15 Oct 2015 05:06:30 +0300 Subject: [PATCH 15/25] Some tests for HeapBasedPoolAllocator<>; the concurrency test is failing with heap corruption --- .../helpers/heap_based_pool_allocator.cpp | 132 ++++++++++++++---- 1 file changed, 108 insertions(+), 24 deletions(-) diff --git a/libuavcan/test/helpers/heap_based_pool_allocator.cpp b/libuavcan/test/helpers/heap_based_pool_allocator.cpp index 93da37130f..61daf378d8 100644 --- a/libuavcan/test/helpers/heap_based_pool_allocator.cpp +++ b/libuavcan/test/helpers/heap_based_pool_allocator.cpp @@ -4,47 +4,131 @@ #include #include +#include inline bool atomicCompareAndSwap(void** address, void* expected, void* replacement) { return __sync_bool_compare_and_swap(address, expected, replacement); } -TEST(ThreadSafeHeapBasedPoolAllocator, Basic) +TEST(HeapBasedPoolAllocator, Basic) { - uavcan::HeapBasedPoolAllocator allocator; + std::cout << ">>> HEAP BEFORE:" << std::endl; + malloc_stats(); - ASSERT_EQ(0, allocator.getNumCachedBlocks()); + uavcan::HeapBasedPoolAllocator al; - void* a = allocator.allocate(10); - void* b = allocator.allocate(10); - void* c = allocator.allocate(10); - void* d = allocator.allocate(10); + ASSERT_EQ(0, al.getNumCachedBlocks()); - ASSERT_EQ(0, allocator.getNumCachedBlocks()); + void* a = al.allocate(10); + void* b = al.allocate(10); + void* c = al.allocate(10); + void* d = al.allocate(10); - allocator.deallocate(a); - ASSERT_EQ(1, allocator.getNumCachedBlocks()); + ASSERT_EQ(0, al.getNumCachedBlocks()); - allocator.deallocate(b); - ASSERT_EQ(2, allocator.getNumCachedBlocks()); + al.deallocate(a); + ASSERT_EQ(1, al.getNumCachedBlocks()); - allocator.deallocate(c); - ASSERT_EQ(3, allocator.getNumCachedBlocks()); + al.deallocate(b); + ASSERT_EQ(2, al.getNumCachedBlocks()); - a = allocator.allocate(10); - ASSERT_EQ(2, allocator.getNumCachedBlocks()); + al.deallocate(c); + ASSERT_EQ(3, al.getNumCachedBlocks()); + + a = al.allocate(10); + ASSERT_EQ(2, al.getNumCachedBlocks()); ASSERT_EQ(c, a); - allocator.deallocate(a); - ASSERT_EQ(3, allocator.getNumCachedBlocks()); + al.deallocate(a); + ASSERT_EQ(3, al.getNumCachedBlocks()); - allocator.shrink(); - ASSERT_EQ(0, allocator.getNumCachedBlocks()); + al.shrink(); + ASSERT_EQ(0, al.getNumCachedBlocks()); - allocator.deallocate(d); - ASSERT_EQ(1, allocator.getNumCachedBlocks()); + al.deallocate(d); + ASSERT_EQ(1, al.getNumCachedBlocks()); - allocator.shrink(); - ASSERT_EQ(0, allocator.getNumCachedBlocks()); + al.shrink(); + ASSERT_EQ(0, al.getNumCachedBlocks()); + + std::cout << ">>> HEAP AFTER:" << std::endl; + malloc_stats(); } + +#if UAVCAN_CPP_VERSION >= UAVCAN_CPP11 + +#include + +inline bool atomicCompareAndSwapWithRescheduling(void** address, void* expected, void* replacement) +{ + /* + * Yield is added for testing reasons, making CAS failure more likely + */ + if ((float(std::rand()) / float(RAND_MAX)) < 0.05) + { + std::this_thread::yield(); + } + return __sync_bool_compare_and_swap(address, expected, replacement); +} + +TEST(HeapBasedPoolAllocator, Concurrency) +{ + std::cout << ">>> HEAP BEFORE:" << std::endl; + malloc_stats(); + + uavcan::HeapBasedPoolAllocator al; + + volatile bool terminate = false; + + /* + * Starting the testing threads + */ + std::thread threads[3]; + + for (auto& x : threads) + { + x = std::thread([&al, &terminate]() + { + while (!terminate) + { + auto a = al.allocate(1); + auto b = al.allocate(1); + auto c = al.allocate(1); + al.deallocate(al.allocate(1)); + al.deallocate(a); + al.deallocate(b); + al.deallocate(c); + } + }); + } + + /* + * Running the threads for some time, then terminating + */ + std::this_thread::sleep_for(std::chrono::seconds(1)); + + terminate = true; + std::cout << "Terminating workers..." << std::endl; + + for (auto& x : threads) + { + x.join(); + } + std::cout << "All workers joined" << std::endl; + + /* + * Now, there must not be any leaked memory, because the worker threads deallocate everything before completion. + */ + //std::cout << "Cached blocks: " << al.getNumCachedBlocks() << std::endl; + + std::cout << ">>> HEAP BEFORE SHRINK:" << std::endl; + malloc_stats(); + + al.shrink(); + + std::cout << ">>> HEAP AFTER SHRINK:" << std::endl; + malloc_stats(); +} + +#endif From ef93f1b1e8bc4294e01ffe402417e93bfbc05a7c Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Thu, 15 Oct 2015 10:50:42 +0300 Subject: [PATCH 16/25] Heap based pool allocator - configurable getNumBlocks() --- .../uavcan/helpers/heap_based_pool_allocator.hpp | 11 ++++++++--- libuavcan/test/helpers/heap_based_pool_allocator.cpp | 8 ++++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp b/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp index a012ee2c9a..566e21f2d3 100644 --- a/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp +++ b/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp @@ -49,6 +49,7 @@ class UAVCAN_EXPORT HeapBasedPoolAllocator : public IPoolAllocator, Noncopyable }; Node* volatile cache_; + uint16_t reported_num_blocks_; Node* popCache() { @@ -73,7 +74,10 @@ public: /** * The allocator initializes with empty cache, so first allocations will be served from heap. */ - HeapBasedPoolAllocator() : cache_(NULL) { } + HeapBasedPoolAllocator(uint16_t reported_num_blocks) : + cache_(NULL), + reported_num_blocks_(reported_num_blocks) + { } /** * The destructor de-allocates all blocks that are currently in the cache. @@ -121,9 +125,10 @@ public: } /** - * Heap-based pool is virutally infinite in size, so this method just returns maximum possible number of blocks. + * Heap-based pool is virutally infinite in size, so this method just returns some pre-defined value. */ - virtual uint16_t getNumBlocks() const { return NumericTraits::max(); } + virtual uint16_t getNumBlocks() const { return reported_num_blocks_; } + void setReportedNumBlocks(uint16_t x) { reported_num_blocks_ = x; } /** * Frees all blocks that are not in use at the moment. diff --git a/libuavcan/test/helpers/heap_based_pool_allocator.cpp b/libuavcan/test/helpers/heap_based_pool_allocator.cpp index 61daf378d8..ae250be287 100644 --- a/libuavcan/test/helpers/heap_based_pool_allocator.cpp +++ b/libuavcan/test/helpers/heap_based_pool_allocator.cpp @@ -16,10 +16,14 @@ TEST(HeapBasedPoolAllocator, Basic) std::cout << ">>> HEAP BEFORE:" << std::endl; malloc_stats(); - uavcan::HeapBasedPoolAllocator al; + uavcan::HeapBasedPoolAllocator al(64); ASSERT_EQ(0, al.getNumCachedBlocks()); + ASSERT_EQ(64, al.getNumBlocks()); + al.setReportedNumBlocks(123); + ASSERT_EQ(123, al.getNumBlocks()); + void* a = al.allocate(10); void* b = al.allocate(10); void* c = al.allocate(10); @@ -77,7 +81,7 @@ TEST(HeapBasedPoolAllocator, Concurrency) std::cout << ">>> HEAP BEFORE:" << std::endl; malloc_stats(); - uavcan::HeapBasedPoolAllocator al; + uavcan::HeapBasedPoolAllocator al(1); volatile bool terminate = false; From 913f6ea03467149bc2fa00e4ede9422038e9e916 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Thu, 15 Oct 2015 16:49:03 +0300 Subject: [PATCH 17/25] Traditional lock-based thread safety for HeapBasedPoolAllocator --- .../helpers/heap_based_pool_allocator.hpp | 58 ++++++------------- .../helpers/heap_based_pool_allocator.cpp | 25 +++----- 2 files changed, 27 insertions(+), 56 deletions(-) diff --git a/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp b/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp index 566e21f2d3..ad217a241a 100644 --- a/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp +++ b/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp @@ -18,25 +18,9 @@ namespace uavcan * - Call @ref shrink() - this method frees all blocks that are unused at the moment. * - Destroy the object - the desctructor calls @ref shrink(). * - * All operations are thread-safe, the safety is obtained through the standard atomic compare-and-swap operator (CAS). - * If thread safety is not required, the CAS operator can be replaced with a dummy version that simply performs the - * assignment without any checks. - * If thread safety is required, but the hardware does not support atomic exchange operations, CAS can be emulated - * with a mutex or a critical section roughly as follows: - * - * bool softwareAtomicCompareAndSwap(void** address, void* expected, void* replacement) - * { - * RaiiSynchronizationPrimitive lock; - * if (*address != expected) - * { - * return false; - * } - * *address = replacement; - * return true; - * } + * TODO: notes on thread-safety. */ -template +template class UAVCAN_EXPORT HeapBasedPoolAllocator : public IPoolAllocator, Noncopyable { union Node @@ -53,21 +37,22 @@ class UAVCAN_EXPORT HeapBasedPoolAllocator : public IPoolAllocator, Noncopyable Node* popCache() { - // http://www.ibm.com/developerworks/aix/library/au-multithreaded_structures2/ - for (;;) + RaiiSynchronizer lock; + (void)lock; + Node* const p = cache_; + if (p != NULL) { - Node* volatile const result = cache_; - if (result == NULL) - { - break; - } - if ((cache_ != NULL) && - AtomicCompareAndSwap(reinterpret_cast(const_cast(&cache_)), result, result->next)) - { - return result; - } + cache_ = cache_->next; } - return NULL; + return p; + } + + void pushCache(Node* node) + { + RaiiSynchronizer lock; + (void)lock; + node->next = cache_; + cache_ = node; } public: @@ -111,17 +96,10 @@ public: */ virtual void deallocate(const void* ptr) { - if (ptr == NULL) + if (ptr != NULL) { - return; + pushCache(static_cast(const_cast(ptr))); } - - Node* volatile const n = static_cast(const_cast(ptr)); - do - { - n->next = cache_; - } - while (!AtomicCompareAndSwap(reinterpret_cast(const_cast(&cache_)), n->next, n)); } /** diff --git a/libuavcan/test/helpers/heap_based_pool_allocator.cpp b/libuavcan/test/helpers/heap_based_pool_allocator.cpp index ae250be287..2f029241d3 100644 --- a/libuavcan/test/helpers/heap_based_pool_allocator.cpp +++ b/libuavcan/test/helpers/heap_based_pool_allocator.cpp @@ -6,17 +6,13 @@ #include #include -inline bool atomicCompareAndSwap(void** address, void* expected, void* replacement) -{ - return __sync_bool_compare_and_swap(address, expected, replacement); -} TEST(HeapBasedPoolAllocator, Basic) { std::cout << ">>> HEAP BEFORE:" << std::endl; malloc_stats(); - uavcan::HeapBasedPoolAllocator al(64); + uavcan::HeapBasedPoolAllocator al(64); ASSERT_EQ(0, al.getNumCachedBlocks()); @@ -63,25 +59,22 @@ TEST(HeapBasedPoolAllocator, Basic) #if UAVCAN_CPP_VERSION >= UAVCAN_CPP11 #include +#include -inline bool atomicCompareAndSwapWithRescheduling(void** address, void* expected, void* replacement) +struct RaiiSynchronizer { - /* - * Yield is added for testing reasons, making CAS failure more likely - */ - if ((float(std::rand()) / float(RAND_MAX)) < 0.05) - { - std::this_thread::yield(); - } - return __sync_bool_compare_and_swap(address, expected, replacement); -} + static std::mutex mutex; + std::lock_guard guard{mutex}; +}; + +std::mutex RaiiSynchronizer::mutex; TEST(HeapBasedPoolAllocator, Concurrency) { std::cout << ">>> HEAP BEFORE:" << std::endl; malloc_stats(); - uavcan::HeapBasedPoolAllocator al(1); + uavcan::HeapBasedPoolAllocator al(1); volatile bool terminate = false; From 660e84e17f8dbcfec03d4c3bd5b8c67321a7f552 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Fri, 16 Oct 2015 14:31:42 +0300 Subject: [PATCH 18/25] Allocator interface: getNumBlocks() --> getBlockCapacity() --- libuavcan/include/uavcan/dynamic_memory.hpp | 9 ++++-- .../helpers/heap_based_pool_allocator.hpp | 28 +++++++++++++++---- libuavcan/src/transport/uc_can_io.cpp | 4 +-- libuavcan/src/uc_dynamic_memory.cpp | 4 +-- libuavcan/test/dynamic_memory.cpp | 2 +- .../helpers/heap_based_pool_allocator.cpp | 12 ++++---- .../test/transport/transfer_test_helpers.hpp | 2 +- .../linux/apps/test_multithreading.cpp | 4 +-- 8 files changed, 43 insertions(+), 22 deletions(-) diff --git a/libuavcan/include/uavcan/dynamic_memory.hpp b/libuavcan/include/uavcan/dynamic_memory.hpp index 5d440724d6..425d7ca6b4 100644 --- a/libuavcan/include/uavcan/dynamic_memory.hpp +++ b/libuavcan/include/uavcan/dynamic_memory.hpp @@ -26,7 +26,10 @@ public: virtual void* allocate(std::size_t size) = 0; virtual void deallocate(const void* ptr) = 0; - virtual uint16_t getNumBlocks() const = 0; + /** + * Returns the maximum number of blocks this allocator can allocate. + */ + virtual uint16_t getBlockCapacity() const = 0; }; /** @@ -61,7 +64,7 @@ public: virtual void* allocate(std::size_t size); virtual void deallocate(const void* ptr); - virtual uint16_t getNumBlocks() const { return NumBlocks; } + virtual uint16_t getBlockCapacity() const { return NumBlocks; } /** * Return the number of blocks that are currently allocated/unallocated. @@ -96,7 +99,7 @@ public: virtual void* allocate(std::size_t size); virtual void deallocate(const void* ptr); - virtual uint16_t getNumBlocks() const; + virtual uint16_t getBlockCapacity() const; }; // ---------------------------------------------------------------------------- diff --git a/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp b/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp index ad217a241a..0034ec9b31 100644 --- a/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp +++ b/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp @@ -33,7 +33,9 @@ class UAVCAN_EXPORT HeapBasedPoolAllocator : public IPoolAllocator, Noncopyable }; Node* volatile cache_; - uint16_t reported_num_blocks_; + + const uint16_t capacity_soft_limit_; + const uint16_t capacity_hard_limit_; Node* popCache() { @@ -58,10 +60,20 @@ class UAVCAN_EXPORT HeapBasedPoolAllocator : public IPoolAllocator, Noncopyable public: /** * The allocator initializes with empty cache, so first allocations will be served from heap. + * + * @param block_capacity_soft_limit Block capacity that will be reported via @ref getBlockCapacity(). + * + * @param block_capacity_hard_limit Real block capacity limit; the number of allocated blocks will never + * exceed this value. Hard limit should be higher than soft limit. + * Default value is two times the soft limit. */ - HeapBasedPoolAllocator(uint16_t reported_num_blocks) : + HeapBasedPoolAllocator(uint16_t block_capacity_soft_limit, + uint16_t block_capacity_hard_limit = 0) : cache_(NULL), - reported_num_blocks_(reported_num_blocks) + capacity_soft_limit_(block_capacity_soft_limit), + capacity_hard_limit_((block_capacity_hard_limit > 0) ? block_capacity_hard_limit : + static_cast(min(static_cast(block_capacity_soft_limit) * 2U, + static_cast(NumericTraits::max())))) { } /** @@ -103,10 +115,14 @@ public: } /** - * Heap-based pool is virutally infinite in size, so this method just returns some pre-defined value. + * The soft limit. */ - virtual uint16_t getNumBlocks() const { return reported_num_blocks_; } - void setReportedNumBlocks(uint16_t x) { reported_num_blocks_ = x; } + virtual uint16_t getBlockCapacity() const { return capacity_soft_limit_; } + + /** + * The hard limit. + */ + uint16_t getBlockCapacityHardLimit() const { return capacity_hard_limit_; } /** * Frees all blocks that are not in use at the moment. diff --git a/libuavcan/src/transport/uc_can_io.cpp b/libuavcan/src/transport/uc_can_io.cpp index 2fb36ec69c..d6417a670c 100644 --- a/libuavcan/src/transport/uc_can_io.cpp +++ b/libuavcan/src/transport/uc_can_io.cpp @@ -295,10 +295,10 @@ CanIOManager::CanIOManager(ICanDriver& driver, IPoolAllocator& allocator, ISyste if (mem_blocks_per_iface == 0) { - mem_blocks_per_iface = allocator.getNumBlocks() / (num_ifaces_ + 1U) + 1U; + mem_blocks_per_iface = allocator.getBlockCapacity() / (num_ifaces_ + 1U) + 1U; } UAVCAN_TRACE("CanIOManager", "Memory blocks per iface: %u, total: %u", - unsigned(mem_blocks_per_iface), unsigned(allocator.getNumBlocks())); + unsigned(mem_blocks_per_iface), unsigned(allocator.getBlockCapacity())); for (int i = 0; i < num_ifaces_; i++) { diff --git a/libuavcan/src/uc_dynamic_memory.cpp b/libuavcan/src/uc_dynamic_memory.cpp index 68925f1d11..1b682f6b1b 100644 --- a/libuavcan/src/uc_dynamic_memory.cpp +++ b/libuavcan/src/uc_dynamic_memory.cpp @@ -33,9 +33,9 @@ void LimitedPoolAllocator::deallocate(const void* ptr) } } -uint16_t LimitedPoolAllocator::getNumBlocks() const +uint16_t LimitedPoolAllocator::getBlockCapacity() const { - return min(max_blocks_, allocator_.getNumBlocks()); + return min(max_blocks_, allocator_.getBlockCapacity()); } } diff --git a/libuavcan/test/dynamic_memory.cpp b/libuavcan/test/dynamic_memory.cpp index 21ba7e4487..cbe3650614 100644 --- a/libuavcan/test/dynamic_memory.cpp +++ b/libuavcan/test/dynamic_memory.cpp @@ -49,7 +49,7 @@ TEST(DynamicMemory, LimitedPoolAllocator) uavcan::PoolAllocator<128, 32> pool32; uavcan::LimitedPoolAllocator lim(pool32, 2); - EXPECT_EQ(2, lim.getNumBlocks()); + EXPECT_EQ(2, lim.getBlockCapacity()); EXPECT_EQ(0, pool32.getPeakNumUsedBlocks()); const void* ptr1 = lim.allocate(1); diff --git a/libuavcan/test/helpers/heap_based_pool_allocator.cpp b/libuavcan/test/helpers/heap_based_pool_allocator.cpp index 2f029241d3..96783acfd8 100644 --- a/libuavcan/test/helpers/heap_based_pool_allocator.cpp +++ b/libuavcan/test/helpers/heap_based_pool_allocator.cpp @@ -12,13 +12,12 @@ TEST(HeapBasedPoolAllocator, Basic) std::cout << ">>> HEAP BEFORE:" << std::endl; malloc_stats(); - uavcan::HeapBasedPoolAllocator al(64); + uavcan::HeapBasedPoolAllocator al(0xEEEE); ASSERT_EQ(0, al.getNumCachedBlocks()); - ASSERT_EQ(64, al.getNumBlocks()); - al.setReportedNumBlocks(123); - ASSERT_EQ(123, al.getNumBlocks()); + ASSERT_EQ(0xEEEE, al.getBlockCapacity()); + ASSERT_EQ(0xFFFF, al.getBlockCapacityHardLimit()); void* a = al.allocate(10); void* b = al.allocate(10); @@ -74,7 +73,10 @@ TEST(HeapBasedPoolAllocator, Concurrency) std::cout << ">>> HEAP BEFORE:" << std::endl; malloc_stats(); - uavcan::HeapBasedPoolAllocator al(1); + uavcan::HeapBasedPoolAllocator al(1000); + + ASSERT_EQ(1000, al.getBlockCapacity()); + ASSERT_EQ(2000, al.getBlockCapacityHardLimit()); volatile bool terminate = false; diff --git a/libuavcan/test/transport/transfer_test_helpers.hpp b/libuavcan/test/transport/transfer_test_helpers.hpp index aa52154ea4..55dda5a35f 100644 --- a/libuavcan/test/transport/transfer_test_helpers.hpp +++ b/libuavcan/test/transport/transfer_test_helpers.hpp @@ -316,5 +316,5 @@ class NullAllocator : public uavcan::IPoolAllocator public: virtual void* allocate(std::size_t) { return NULL; } virtual void deallocate(const void*) { } - virtual uint16_t getNumBlocks() const { return 0; } + virtual uint16_t getBlockCapacity() const { return 0; } }; diff --git a/libuavcan_drivers/linux/apps/test_multithreading.cpp b/libuavcan_drivers/linux/apps/test_multithreading.cpp index f46d5a46cf..fb143e3cf2 100644 --- a/libuavcan_drivers/linux/apps/test_multithreading.cpp +++ b/libuavcan_drivers/linux/apps/test_multithreading.cpp @@ -373,11 +373,11 @@ public: { assert(num_ifaces_ > 0 && num_ifaces_ <= uavcan::MaxCanIfaces); - const unsigned quota_per_iface = allocator_.getNumBlocks() / num_ifaces_; + const unsigned quota_per_iface = allocator_.getBlockCapacity() / num_ifaces_; const unsigned quota_per_queue = quota_per_iface; // 2x overcommit UAVCAN_TRACE("VirtualCanDriver", "Total blocks: %u, quota per queue: %u", - unsigned(allocator_.getNumBlocks()), unsigned(quota_per_queue)); + unsigned(allocator_.getBlockCapacity()), unsigned(quota_per_queue)); for (unsigned i = 0; i < num_ifaces_; i++) { From 6d29f0e40597d851738b1ceacef8116d1f7bd33a Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Fri, 16 Oct 2015 15:50:33 +0300 Subject: [PATCH 19/25] UAVCAN_LIKELY(), UAVCAN_UNLIKELY() --- libuavcan/include/uavcan/build_config.hpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/libuavcan/include/uavcan/build_config.hpp b/libuavcan/include/uavcan/build_config.hpp index fedea852b7..c1ee6dc1e0 100644 --- a/libuavcan/include/uavcan/build_config.hpp +++ b/libuavcan/include/uavcan/build_config.hpp @@ -167,6 +167,22 @@ # endif #endif +#ifndef UAVCAN_LIKELY +# if __GNUC__ +# define UAVCAN_LIKELY(x) __builtin_expect(!!(x), true) +# else +# define UAVCAN_LIKELY(x) (x) +# endif +#endif + +#ifndef UAVCAN_UNLIKELY +# if __GNUC__ +# define UAVCAN_UNLIKELY(x) __builtin_expect(!!(x), false) +# else +# define UAVCAN_UNLIKELY(x) (x) +# endif +#endif + namespace uavcan { /** From b0d0c607a550e44534429b1a4e34b00ae2d24b01 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Fri, 16 Oct 2015 16:24:22 +0300 Subject: [PATCH 20/25] Updated HeapBasedPoolAllocator --- .../helpers/heap_based_pool_allocator.hpp | 148 ++++++++++++------ .../helpers/heap_based_pool_allocator.cpp | 68 ++++++-- 2 files changed, 160 insertions(+), 56 deletions(-) diff --git a/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp b/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp index 0034ec9b31..d6683c3871 100644 --- a/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp +++ b/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp @@ -6,6 +6,8 @@ #define UAVCAN_HELPERS_HEAP_BASED_POOL_ALLOCATOR_HPP_INCLUDED #include +#include +#include #include namespace uavcan @@ -32,34 +34,18 @@ class UAVCAN_EXPORT HeapBasedPoolAllocator : public IPoolAllocator, Noncopyable long long _aligner2; }; - Node* volatile cache_; - const uint16_t capacity_soft_limit_; const uint16_t capacity_hard_limit_; - Node* popCache() - { - RaiiSynchronizer lock; - (void)lock; - Node* const p = cache_; - if (p != NULL) - { - cache_ = cache_->next; - } - return p; - } + uint16_t num_reserved_blocks_; + uint16_t num_allocated_blocks_; + uint16_t peak_allocated_blocks_; - void pushCache(Node* node) - { - RaiiSynchronizer lock; - (void)lock; - node->next = cache_; - cache_ = node; - } + Node* reserve_; public: /** - * The allocator initializes with empty cache, so first allocations will be served from heap. + * The allocator initializes with empty reserve, so first allocations will be served from heap. * * @param block_capacity_soft_limit Block capacity that will be reported via @ref getBlockCapacity(). * @@ -69,21 +55,33 @@ public: */ HeapBasedPoolAllocator(uint16_t block_capacity_soft_limit, uint16_t block_capacity_hard_limit = 0) : - cache_(NULL), capacity_soft_limit_(block_capacity_soft_limit), capacity_hard_limit_((block_capacity_hard_limit > 0) ? block_capacity_hard_limit : static_cast(min(static_cast(block_capacity_soft_limit) * 2U, - static_cast(NumericTraits::max())))) + static_cast(NumericTraits::max())))), + num_reserved_blocks_(0), + num_allocated_blocks_(0), + peak_allocated_blocks_(0), + reserve_(NULL) { } /** - * The destructor de-allocates all blocks that are currently in the cache. + * The destructor de-allocates all blocks that are currently in the reserve. * BLOCKS THAT ARE CURRENTLY HELD BY THE APPLICATION WILL LEAK. */ - ~HeapBasedPoolAllocator() { shrink(); } + ~HeapBasedPoolAllocator() + { + shrink(); +#if UAVCAN_DEBUG + if (num_allocated_blocks_ > 0) + { + UAVCAN_TRACE("HeapBasedPoolAllocator", "%u BLOCKS LEAKED", num_allocated_blocks_); + } +#endif + } /** - * Takes a block from the cache, unless it's empty. + * Takes a block from the reserve, unless it's empty. * In the latter case, allocates a new block in the heap. */ virtual void* allocate(std::size_t size) @@ -92,25 +90,64 @@ public: { return NULL; } - if (Node* n = popCache()) + { - return n; + RaiiSynchronizer lock; + (void)lock; + + Node* const p = reserve_; + + if (UAVCAN_LIKELY(p != NULL)) + { + reserve_ = reserve_->next; + num_allocated_blocks_++; + + if (UAVCAN_UNLIKELY(num_allocated_blocks_ > peak_allocated_blocks_)) + { + peak_allocated_blocks_ = num_allocated_blocks_; + } + return p; + } + + if (num_reserved_blocks_ >= capacity_hard_limit_) // Hard limit reached, no further allocations + { + return NULL; + } } - else + + // Unlikely branch + void* const m = std::malloc(sizeof(Node)); + if (m != NULL) { - return std::malloc(sizeof(Node)); + RaiiSynchronizer lock; + (void)lock; + + num_reserved_blocks_++; + num_allocated_blocks_++; + if (num_allocated_blocks_ > peak_allocated_blocks_) + { + peak_allocated_blocks_ = num_allocated_blocks_; + } } + return m; } /** - * Puts the block back to cache. + * Puts the block back to reserve. * The block will not be free()d automatically; see @ref shrink(). */ virtual void deallocate(const void* ptr) { if (ptr != NULL) { - pushCache(static_cast(const_cast(ptr))); + RaiiSynchronizer lock; + (void)lock; + + Node* const node = static_cast(const_cast(ptr)); + node->next = reserve_; + reserve_ = node; + + num_allocated_blocks_--; } } @@ -126,30 +163,51 @@ public: /** * Frees all blocks that are not in use at the moment. + * Post-condition is getNumAllocatedBlocks() == getNumReservedBlocks(). */ void shrink() { - while (Node* p = popCache()) + Node* p = NULL; + for (;;) { + { + RaiiSynchronizer lock; + (void)lock; + // Removing from reserve and updating the counter. + p = reserve_; + if (p != NULL) + { + reserve_ = reserve_->next; + num_reserved_blocks_--; + } + else + { + break; + } + } + // Then freeing, having left the critical section. std::free(p); } } /** - * This function naively counts the number of cached blocks. - * It is not thread-safe and is mostly designed for testing and debugging purposes. - * Don't use it in production code. + * Number of blocks that are currently in use by the application. */ - unsigned getNumCachedBlocks() + uint16_t getNumAllocatedBlocks() const { - unsigned out = 0; - Node* p = cache_; - while (p != NULL) - { - out++; - p = p->next; - } - return out; + RaiiSynchronizer lock; + (void)lock; + return num_allocated_blocks_; + } + + /** + * Number of blocks that are acquired from the heap. + */ + uint16_t getNumReservedBlocks() const + { + RaiiSynchronizer lock; + (void)lock; + return num_reserved_blocks_; } }; diff --git a/libuavcan/test/helpers/heap_based_pool_allocator.cpp b/libuavcan/test/helpers/heap_based_pool_allocator.cpp index 96783acfd8..b1588675ea 100644 --- a/libuavcan/test/helpers/heap_based_pool_allocator.cpp +++ b/libuavcan/test/helpers/heap_based_pool_allocator.cpp @@ -14,7 +14,8 @@ TEST(HeapBasedPoolAllocator, Basic) uavcan::HeapBasedPoolAllocator al(0xEEEE); - ASSERT_EQ(0, al.getNumCachedBlocks()); + ASSERT_EQ(0, al.getNumReservedBlocks()); + ASSERT_EQ(0, al.getNumAllocatedBlocks()); ASSERT_EQ(0xEEEE, al.getBlockCapacity()); ASSERT_EQ(0xFFFF, al.getBlockCapacityHardLimit()); @@ -24,37 +25,81 @@ TEST(HeapBasedPoolAllocator, Basic) void* c = al.allocate(10); void* d = al.allocate(10); - ASSERT_EQ(0, al.getNumCachedBlocks()); + ASSERT_EQ(4, al.getNumReservedBlocks()); + ASSERT_EQ(4, al.getNumAllocatedBlocks()); al.deallocate(a); - ASSERT_EQ(1, al.getNumCachedBlocks()); + ASSERT_EQ(4, al.getNumReservedBlocks()); + ASSERT_EQ(3, al.getNumAllocatedBlocks()); al.deallocate(b); - ASSERT_EQ(2, al.getNumCachedBlocks()); + ASSERT_EQ(4, al.getNumReservedBlocks()); + ASSERT_EQ(2, al.getNumAllocatedBlocks()); al.deallocate(c); - ASSERT_EQ(3, al.getNumCachedBlocks()); + ASSERT_EQ(4, al.getNumReservedBlocks()); + ASSERT_EQ(1, al.getNumAllocatedBlocks()); a = al.allocate(10); - ASSERT_EQ(2, al.getNumCachedBlocks()); + ASSERT_EQ(4, al.getNumReservedBlocks()); + ASSERT_EQ(2, al.getNumAllocatedBlocks()); ASSERT_EQ(c, a); al.deallocate(a); - ASSERT_EQ(3, al.getNumCachedBlocks()); + ASSERT_EQ(4, al.getNumReservedBlocks()); + ASSERT_EQ(1, al.getNumAllocatedBlocks()); al.shrink(); - ASSERT_EQ(0, al.getNumCachedBlocks()); + ASSERT_EQ(1, al.getNumReservedBlocks()); + ASSERT_EQ(1, al.getNumAllocatedBlocks()); al.deallocate(d); - ASSERT_EQ(1, al.getNumCachedBlocks()); + ASSERT_EQ(1, al.getNumReservedBlocks()); + ASSERT_EQ(0, al.getNumAllocatedBlocks()); al.shrink(); - ASSERT_EQ(0, al.getNumCachedBlocks()); + ASSERT_EQ(0, al.getNumReservedBlocks()); + ASSERT_EQ(0, al.getNumAllocatedBlocks()); std::cout << ">>> HEAP AFTER:" << std::endl; malloc_stats(); } + +TEST(HeapBasedPoolAllocator, Limits) +{ + uavcan::HeapBasedPoolAllocator al(2); + + ASSERT_EQ(2, al.getBlockCapacity()); + ASSERT_EQ(4, al.getBlockCapacityHardLimit()); + + ASSERT_EQ(0, al.getNumReservedBlocks()); + ASSERT_EQ(0, al.getNumAllocatedBlocks()); + + void* a = al.allocate(10); + void* b = al.allocate(10); + void* c = al.allocate(10); + void* d = al.allocate(10); + + ASSERT_TRUE(a); + ASSERT_TRUE(b); + ASSERT_TRUE(c); + ASSERT_TRUE(d); + + ASSERT_FALSE(al.allocate(10)); + + ASSERT_EQ(4, al.getNumReservedBlocks()); + ASSERT_EQ(4, al.getNumAllocatedBlocks()); + + al.deallocate(a); + al.deallocate(b); + al.deallocate(c); + al.deallocate(d); + + ASSERT_EQ(4, al.getNumReservedBlocks()); + ASSERT_EQ(0, al.getNumAllocatedBlocks()); +} + #if UAVCAN_CPP_VERSION >= UAVCAN_CPP11 #include @@ -119,7 +164,8 @@ TEST(HeapBasedPoolAllocator, Concurrency) /* * Now, there must not be any leaked memory, because the worker threads deallocate everything before completion. */ - //std::cout << "Cached blocks: " << al.getNumCachedBlocks() << std::endl; + std::cout << "Allocated: " << al.getNumAllocatedBlocks() << std::endl; + std::cout << "Reserved: " << al.getNumReservedBlocks() << std::endl; std::cout << ">>> HEAP BEFORE SHRINK:" << std::endl; malloc_stats(); From 12c5fc82563c62994b72b1099d3a60717a9e757d Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Fri, 16 Oct 2015 16:32:56 +0300 Subject: [PATCH 21/25] Heap based block allocator docs --- .../helpers/heap_based_pool_allocator.hpp | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp b/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp index d6683c3871..dd2c9c8331 100644 --- a/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp +++ b/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp @@ -20,10 +20,25 @@ namespace uavcan * - Call @ref shrink() - this method frees all blocks that are unused at the moment. * - Destroy the object - the desctructor calls @ref shrink(). * - * TODO: notes on thread-safety. + * The pool can be limited in growth with hard and soft limits. + * The soft limit defines the value that will be reported via @ref IPoolAllocator::getBlockCapacity(). + * The hard limit defines the maximum number of blocks that can be allocated from heap. + * Typically, the hard limit should be equal or greater than the soft limit. + * + * The allocator can be made thread-safe (optional) by means of providing a RAII-lock type via the second template + * argument. The allocator uses the lock only to access the shared state, therefore critical sections are only a few + * cycles long, which implies that it should be acceptable to use hardware IRQ disabling instead of a mutex for + * performance reasons. For example, an IRQ-based RAII-lock type can be implemented as follows: + * struct RaiiSynchronizer + * { + * RaiiSynchronizer() { __disable_irq(); } + * ~RaiiSynchronizer() { __enable_irq(); } + * }; */ -template -class UAVCAN_EXPORT HeapBasedPoolAllocator : public IPoolAllocator, Noncopyable +template +class UAVCAN_EXPORT HeapBasedPoolAllocator : public IPoolAllocator, + Noncopyable { union Node { From 04381686bda88d1c2ee48217908887f30194f210 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Fri, 16 Oct 2015 16:46:10 +0300 Subject: [PATCH 22/25] Optional thread-safety for default allocator --- libuavcan/include/uavcan/dynamic_memory.hpp | 60 ++++++++++++++++----- 1 file changed, 47 insertions(+), 13 deletions(-) diff --git a/libuavcan/include/uavcan/dynamic_memory.hpp b/libuavcan/include/uavcan/dynamic_memory.hpp index 425d7ca6b4..51e0fb912f 100644 --- a/libuavcan/include/uavcan/dynamic_memory.hpp +++ b/libuavcan/include/uavcan/dynamic_memory.hpp @@ -34,9 +34,22 @@ public: /** * Classic implementation of a pool allocator (Meyers). + * + * The allocator can be made thread-safe (optional) by means of providing a RAII-lock type via the second template + * argument. The allocator uses the lock only to access the shared state, therefore critical sections are only a few + * cycles long, which implies that it should be acceptable to use hardware IRQ disabling instead of a mutex for + * performance reasons. For example, an IRQ-based RAII-lock type can be implemented as follows: + * struct RaiiSynchronizer + * { + * RaiiSynchronizer() { __disable_irq(); } + * ~RaiiSynchronizer() { __enable_irq(); } + * }; */ -template -class UAVCAN_EXPORT PoolAllocator : public IPoolAllocator, Noncopyable +template +class UAVCAN_EXPORT PoolAllocator : public IPoolAllocator, + Noncopyable { union Node { @@ -69,13 +82,28 @@ public: /** * Return the number of blocks that are currently allocated/unallocated. */ - uint16_t getNumUsedBlocks() const { return used_; } - uint16_t getNumFreeBlocks() const { return static_cast(NumBlocks - used_); } + uint16_t getNumUsedBlocks() const + { + RaiiSynchronizer lock; + (void)lock; + return used_; + } + uint16_t getNumFreeBlocks() const + { + RaiiSynchronizer lock; + (void)lock; + return static_cast(NumBlocks - used_); + } /** * Returns the maximum number of blocks that were ever allocated at the same time. */ - uint16_t getPeakNumUsedBlocks() const { return max_used_; } + uint16_t getPeakNumUsedBlocks() const + { + RaiiSynchronizer lock; + (void)lock; + return max_used_; + } }; /** @@ -107,11 +135,11 @@ public: /* * PoolAllocator<> */ -template -const uint16_t PoolAllocator::NumBlocks; +template +const uint16_t PoolAllocator::NumBlocks; -template -PoolAllocator::PoolAllocator() : +template +PoolAllocator::PoolAllocator() : free_list_(reinterpret_cast(pool_.bytes)), used_(0), max_used_(0) @@ -128,14 +156,17 @@ PoolAllocator::PoolAllocator() : free_list_[NumBlocks - 1].next = NULL; } -template -void* PoolAllocator::allocate(std::size_t size) +template +void* PoolAllocator::allocate(std::size_t size) { if (free_list_ == NULL || size > BlockSize) { return NULL; } + RaiiSynchronizer lock; + (void)lock; + void* pmem = free_list_; free_list_ = free_list_->next; @@ -150,14 +181,17 @@ void* PoolAllocator::allocate(std::size_t size) return pmem; } -template -void PoolAllocator::deallocate(const void* ptr) +template +void PoolAllocator::deallocate(const void* ptr) { if (ptr == NULL) { return; } + RaiiSynchronizer lock; + (void)lock; + Node* p = static_cast(const_cast(ptr)); p->next = free_list_; free_list_ = p; From 4e4d9b7854b7821f42fd83d89885cc489cc18c33 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Fri, 16 Oct 2015 16:57:02 +0300 Subject: [PATCH 23/25] Node<>, SubNode<>: MemPoolSize defaults to zero --- libuavcan/include/uavcan/node/node.hpp | 2 +- libuavcan/include/uavcan/node/sub_node.hpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libuavcan/include/uavcan/node/node.hpp b/libuavcan/include/uavcan/node/node.hpp index d270c8adf9..3f34d27593 100644 --- a/libuavcan/include/uavcan/node/node.hpp +++ b/libuavcan/include/uavcan/node/node.hpp @@ -34,7 +34,7 @@ namespace uavcan * Please refer to the documentation for details. * If this value is zero, the constructor will accept a reference to user-provided allocator. */ -template +template class UAVCAN_EXPORT Node : public INode { typedef typename diff --git a/libuavcan/include/uavcan/node/sub_node.hpp b/libuavcan/include/uavcan/node/sub_node.hpp index 438d1d6815..35b80492e5 100644 --- a/libuavcan/include/uavcan/node/sub_node.hpp +++ b/libuavcan/include/uavcan/node/sub_node.hpp @@ -20,7 +20,7 @@ namespace uavcan * Please refer to the @ref Node<> for documentation concerning the template arguments; refer to the tutorials * to lean how to use libuavcan in multiprocess applications. */ -template +template class UAVCAN_EXPORT SubNode : public INode { typedef typename From 9a432c03231781403f2a4808846183623cbfc49e Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Fri, 16 Oct 2015 20:36:56 +0300 Subject: [PATCH 24/25] Heap based allocator - peak usage removed --- .../uavcan/helpers/heap_based_pool_allocator.hpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp b/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp index dd2c9c8331..f2e9e9e0d6 100644 --- a/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp +++ b/libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp @@ -54,7 +54,6 @@ class UAVCAN_EXPORT HeapBasedPoolAllocator : public IPoolAllocator, uint16_t num_reserved_blocks_; uint16_t num_allocated_blocks_; - uint16_t peak_allocated_blocks_; Node* reserve_; @@ -76,7 +75,6 @@ public: static_cast(NumericTraits::max())))), num_reserved_blocks_(0), num_allocated_blocks_(0), - peak_allocated_blocks_(0), reserve_(NULL) { } @@ -116,11 +114,6 @@ public: { reserve_ = reserve_->next; num_allocated_blocks_++; - - if (UAVCAN_UNLIKELY(num_allocated_blocks_ > peak_allocated_blocks_)) - { - peak_allocated_blocks_ = num_allocated_blocks_; - } return p; } @@ -139,10 +132,6 @@ public: num_reserved_blocks_++; num_allocated_blocks_++; - if (num_allocated_blocks_ > peak_allocated_blocks_) - { - peak_allocated_blocks_ = num_allocated_blocks_; - } } return m; } From 9b092509c93794ec60484b2864be7a2995aefac6 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Sat, 17 Oct 2015 00:45:04 +0300 Subject: [PATCH 25/25] TestNode uses heap based allocator now --- libuavcan/test/node/test_node.hpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/libuavcan/test/node/test_node.hpp b/libuavcan/test/node/test_node.hpp index bd71b89ee3..8032d06419 100644 --- a/libuavcan/test/node/test_node.hpp +++ b/libuavcan/test/node/test_node.hpp @@ -10,6 +10,7 @@ #endif #include +#include #include #include #include @@ -19,13 +20,20 @@ struct TestNode : public uavcan::INode { - uavcan::PoolAllocator pool; + /* + * This class used to use the simple pool allocator instead: + * uavcan::PoolAllocator pool; + * It has been replaced because unlike the simple allocator, heap-based one is not tested as extensively. + * Moreover, heap based allocator prints and error message upon destruction if some memory has not been freed. + */ + uavcan::HeapBasedPoolAllocator pool; uavcan::Scheduler scheduler; uint64_t internal_failure_count; - TestNode(uavcan::ICanDriver& can_driver, uavcan::ISystemClock& clock_driver, uavcan::NodeID self_node_id) - : scheduler(can_driver, pool, clock_driver) - , internal_failure_count(0) + TestNode(uavcan::ICanDriver& can_driver, uavcan::ISystemClock& clock_driver, uavcan::NodeID self_node_id) : + pool(1024), + scheduler(can_driver, pool, clock_driver), + internal_failure_count(0) { setNodeID(self_node_id); }