From 7ec055a25b39f89b31619c7376bef5133dcff5ce Mon Sep 17 00:00:00 2001 From: Balazs Racz Date: Thu, 17 Sep 2020 22:17:27 +0200 Subject: [PATCH 1/2] Refactors the known nodes' registry from TrainService into a separate class. (#431) * Refactors the node registry from the TrainService into a separate class. * fix whitespace. * Adds missing include guards. --- src/openlcb/DefaultNodeRegistry.hxx | 78 +++++++++++++++++++++++++++++ src/openlcb/NodeRegistry.hxx | 64 +++++++++++++++++++++++ src/openlcb/TractionTrain.cxx | 13 +++-- src/openlcb/TractionTrain.hxx | 14 ++++-- 4 files changed, 158 insertions(+), 11 deletions(-) create mode 100644 src/openlcb/DefaultNodeRegistry.hxx create mode 100644 src/openlcb/NodeRegistry.hxx diff --git a/src/openlcb/DefaultNodeRegistry.hxx b/src/openlcb/DefaultNodeRegistry.hxx new file mode 100644 index 000000000..2b10b6b88 --- /dev/null +++ b/src/openlcb/DefaultNodeRegistry.hxx @@ -0,0 +1,78 @@ +/** \copyright + * Copyright (c) 2020, Balazs Racz + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * - Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * + * \file DefaultNodeRegistry.hxx + * + * Default implementations for data structures keyed by virtual nodes. + * + * @author Balazs Racz + * @date 12 Sep 2020 + */ + +#ifndef _OPENLCB_DEFAULTNODEREGISTRY_HXX_ +#define _OPENLCB_DEFAULTNODEREGISTRY_HXX_ + +#include + +#include "openlcb/NodeRegistry.hxx" + +namespace openlcb +{ + +class Node; + +class DefaultNodeRegistry : public NodeRegistry +{ +public: + /// Adds a node to the list of registered nodes. + /// @param node a virtual node. + void register_node(openlcb::Node *node) override + { + nodes_.insert(node); + } + + /// Removes a node from the list of registered nodes. + /// @param node a virtual node. + void unregister_node(openlcb::Node *node) override + { + nodes_.erase(node); + } + + /// Checks if a node is registered. + /// @param node a virtual node. + /// @return true if this node has been registered. + bool is_node_registered(openlcb::Node *node) override + { + return nodes_.find(node) != nodes_.end(); + } + +private: + std::set nodes_; +}; + +} // namespace openlcb + +#endif // _OPENLCB_DEFAULTNODEREGISTRY_HXX_ diff --git a/src/openlcb/NodeRegistry.hxx b/src/openlcb/NodeRegistry.hxx new file mode 100644 index 000000000..868bbf9ef --- /dev/null +++ b/src/openlcb/NodeRegistry.hxx @@ -0,0 +1,64 @@ +/** \copyright + * Copyright (c) 2020, Balazs Racz + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * - Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * + * \file NodeRegistry.hxx + * + * Abstract class for data structures keyed by virtual nodes. + * + * @author Balazs Racz + * @date 12 Sep 2020 + */ + +#ifndef _OPENLCB_NODEREGISTRY_HXX_ +#define _OPENLCB_NODEREGISTRY_HXX_ + +#include "utils/Destructable.hxx" + +namespace openlcb +{ + +class Node; + +class NodeRegistry : public Destructable +{ +public: + /// Adds a node to the list of registered nodes. + /// @param node a virtual node. + virtual void register_node(openlcb::Node *node) = 0; + + /// Removes a node from the list of registered nodes. + /// @param node a virtual node. + virtual void unregister_node(openlcb::Node *node) = 0; + + /// Checks if a node is registered. + /// @param node a virtual node. + /// @return true if this node has been registered. + virtual bool is_node_registered(openlcb::Node *node) = 0; +}; + +} // namespace openlcb + +#endif // _OPENLCB_NODEREGISTRY_HXX_ diff --git a/src/openlcb/TractionTrain.cxx b/src/openlcb/TractionTrain.cxx index 68c371bec..908b046de 100644 --- a/src/openlcb/TractionTrain.cxx +++ b/src/openlcb/TractionTrain.cxx @@ -174,8 +174,7 @@ struct TrainService::Impl return release_and_exit(); } // Checks if destination is a local traction-enabled node. - if (trainService_->nodes_.find(train_node()) == - trainService_->nodes_.end()) + if (!trainService_->nodes_->is_node_registered(train_node())) { LOG(VERBOSE, "Traction message for node %p that is not " "traction enabled.", @@ -629,9 +628,10 @@ struct TrainService::Impl TractionRequestFlow traction_; }; -TrainService::TrainService(If *iface) +TrainService::TrainService(If *iface, NodeRegistry *train_node_registry) : Service(iface->executor()) , iface_(iface) + , nodes_(train_node_registry) { impl_ = new Impl(this); } @@ -647,17 +647,16 @@ void TrainService::register_train(TrainNode *node) extern void StartInitializationFlow(Node * node); StartInitializationFlow(node); AtomicHolder h(this); - nodes_.insert(node); + nodes_->register_node(node); LOG(VERBOSE, "Registered node %p for traction.", node); - HASSERT(nodes_.find(node) != nodes_.end()); } void TrainService::unregister_train(TrainNode *node) { - HASSERT(nodes_.find(node) != nodes_.end()); + HASSERT(nodes_->is_node_registered(node)); iface_->delete_local_node(node); AtomicHolder h(this); - nodes_.erase(node); + nodes_->unregister_node(node); } } // namespace openlcb diff --git a/src/openlcb/TractionTrain.hxx b/src/openlcb/TractionTrain.hxx index c6729076b..8c1d08ac2 100644 --- a/src/openlcb/TractionTrain.hxx +++ b/src/openlcb/TractionTrain.hxx @@ -38,6 +38,7 @@ #include #include "executor/Service.hxx" +#include "openlcb/DefaultNodeRegistry.hxx" #include "openlcb/Node.hxx" #include "openlcb/TractionDefs.hxx" #include "openlcb/TrainInterface.hxx" @@ -322,7 +323,12 @@ private: class TrainService : public Service, private Atomic { public: - TrainService(If *iface); + /// Constructor. + /// @param iface the OpenLCB interface to which the train nodes are bound. + /// @param train_node_registry implementation of the + /// NodeRegistry. Ownership is transferred. + TrainService( + If *iface, NodeRegistry *train_node_registry = new DefaultNodeRegistry); ~TrainService(); If *iface() @@ -344,17 +350,17 @@ public: /// @return true if this is a known train node. bool is_known_train_node(Node *node) { - return nodes_.find((TrainNode*)node) != nodes_.end(); + return nodes_->is_node_registered(node); } private: struct Impl; /** Implementation flows. */ Impl *impl_; - + /** OpenLCB interface */ If *iface_; /** List of train nodes managed by this Service. */ - std::set nodes_; + std::unique_ptr nodes_; }; } // namespace openlcb From e1b0c2d1e61918288ec169763b050e19d36f94d8 Mon Sep 17 00:00:00 2001 From: Balazs Racz Date: Thu, 17 Sep 2020 22:38:36 +0200 Subject: [PATCH 2/2] Optimizes memory usage of AliasCache (#432) Changes the storage datastructure in AliasCache to use less memory. The new structure uses 16-bit indexes instead of pointers, and a sorted array of 16-bit indexes instead of red-black trees for the lookup methods. The LRU double-linked list is kept. With this change the memory cost goes from 88 bytes per entry to 16 bytes per entry. The compiled code size is also smaller. === * Changes the storage datastructure in AliasCache to use less memory. The new structure uses 16-bit indexes instead of pointers, and a sorted array of 16-bit indexes instead of red-black trees for the lookup methods. The LRU double-linked list is kept. With this change the memory cost goes from 88 bytes per entry to 16 bytes per entry. The compiled code size is also smaller. * fix whitespaces * Add more comments. * fix comment typos. --- src/openlcb/AliasCache.cxx | 159 +++++++++++---------- src/openlcb/AliasCache.cxxtest | 72 +++++----- src/openlcb/AliasCache.hxx | 246 +++++++++++++++++++++++++++------ src/utils/SortedListMap.hxx | 28 ++-- 4 files changed, 342 insertions(+), 163 deletions(-) diff --git a/src/openlcb/AliasCache.cxx b/src/openlcb/AliasCache.cxx index a637b3d6b..32400b2e8 100644 --- a/src/openlcb/AliasCache.cxx +++ b/src/openlcb/AliasCache.cxx @@ -46,15 +46,15 @@ void AliasCache::clear() { idMap.clear(); aliasMap.clear(); - oldest = nullptr; - newest = nullptr; - freeList = nullptr; + oldest.idx_ = NONE_ENTRY; + newest.idx_ = NONE_ENTRY; + freeList.idx_ = NONE_ENTRY; /* initialize the freeList */ for (size_t i = 0; i < entries; ++i) { - pool[i].prev = NULL; - pool[i].next = freeList; - freeList = pool + i; + pool[i].newer_.idx_ = NONE_ENTRY; + pool[i].older_ = freeList; + freeList.idx_ = i; } } @@ -69,78 +69,78 @@ void AliasCache::add(NodeID id, NodeAlias alias) Metadata *insert; - AliasMap::Iterator it = aliasMap.find(alias); + auto it = aliasMap.find(alias); if (it != aliasMap.end()) { /* we already have a mapping for this alias, so lets remove it */ - insert = (*it).second; - remove(alias); - + insert = it->deref(this); + remove(insert->alias_); + if (removeCallback) { /* tell the interface layer that we removed this mapping */ - (*removeCallback)(insert->id, insert->alias, context); + (*removeCallback)(insert->get_node_id(), insert->alias_, context); } } - if (freeList) + if (!freeList.empty()) { /* found an empty slot */ - insert = freeList; - freeList = insert->next; + insert = freeList.deref(this); + freeList = insert->older_; } else { - HASSERT(oldest != NULL && newest != NULL); + HASSERT(!oldest.empty() && !newest.empty()); /* kick out the oldest mapping and re-link the oldest endpoint */ - insert = oldest; - if (oldest->newer) + insert = oldest.deref(this); + auto second = insert->newer_; + if (!second.empty()) { - oldest->newer->older = NULL; + second.deref(this)->older_.idx_ = NONE_ENTRY; } - if (insert == newest) + if (insert == newest.deref(this)) { - newest = NULL; + newest.idx_ = NONE_ENTRY; } - oldest = oldest->newer; + oldest = second; - aliasMap.erase(insert->alias); - idMap.erase(insert->id); + aliasMap.erase(aliasMap.find(insert->alias_)); + idMap.erase(idMap.find(insert->get_node_id())); if (removeCallback) { /* tell the interface layer that we removed this mapping */ - (*removeCallback)(insert->id, insert->alias, context); + (*removeCallback)(insert->get_node_id(), insert->alias_, context); } } - - insert->timestamp = OSTime::get_monotonic(); - insert->id = id; - insert->alias = alias; - aliasMap[alias] = insert; - idMap[id] = insert; + insert->set_node_id(id); + insert->alias_ = alias; + + PoolIdx n; + n.idx_ = insert - pool; + aliasMap.insert(PoolIdx(n)); + idMap.insert(PoolIdx(n)); /* update the time based list */ - insert->newer = NULL; - if (newest == NULL) + insert->newer_.idx_ = NONE_ENTRY; + if (newest.empty()) { /* if newest == NULL, then oldest must also be NULL */ - HASSERT(oldest == NULL); + HASSERT(oldest.empty()); - insert->older = NULL; - oldest = insert; + insert->older_.idx_ = NONE_ENTRY; + oldest = n; } else { - insert->older = newest; - newest->newer = insert; + insert->older_ = newest; + newest.deref(this)->newer_ = n; } - newest = insert; - - return; + newest = n; } /** Remove an alias from an alias cache. This method does not call the @@ -150,33 +150,33 @@ void AliasCache::add(NodeID id, NodeAlias alias) */ void AliasCache::remove(NodeAlias alias) { - AliasMap::Iterator it = aliasMap.find(alias); + auto it = aliasMap.find(alias); if (it != aliasMap.end()) { - Metadata *metadata = (*it).second; + Metadata *metadata = it->deref(this); aliasMap.erase(it); - idMap.erase(metadata->id); - - if (metadata->newer) + idMap.erase(idMap.find(metadata->get_node_id())); + + if (!metadata->newer_.empty()) { - metadata->newer->older = metadata->older; + metadata->newer_.deref(this)->older_ = metadata->older_; } - if (metadata->older) + if (!metadata->older_.empty()) { - metadata->older->newer = metadata->newer; + metadata->older_.deref(this)->newer_ = metadata->newer_; } - if (metadata == newest) + if (metadata == newest.deref(this)) { - newest = metadata->older; + newest = metadata->older_; } - if (metadata == oldest) + if (metadata == oldest.deref(this)) { - oldest = metadata->newer; + oldest = metadata->newer_; } - - metadata->next = freeList; - freeList = metadata; + + metadata->older_ = freeList; + freeList.idx_ = metadata - pool; } } @@ -185,9 +185,9 @@ bool AliasCache::retrieve(unsigned entry, NodeID* node, NodeAlias* alias) { HASSERT(entry < size()); Metadata* md = pool + entry; - if (!md->alias) return false; - if (node) *node = md->id; - if (alias) *alias = md->alias; + if (!md->alias_) return false; + if (node) *node = md->get_node_id(); + if (alias) *alias = md->alias_; return true; } @@ -199,15 +199,15 @@ NodeAlias AliasCache::lookup(NodeID id) { HASSERT(id != 0); - IdMap::Iterator it = idMap.find(id); + auto it = idMap.find(id); if (it != idMap.end()) { - Metadata *metadata = (*it).second; - + Metadata *metadata = it->deref(this); + /* update timestamp */ touch(metadata); - return metadata->alias; + return metadata->alias_; } /* no match found */ @@ -222,15 +222,15 @@ NodeID AliasCache::lookup(NodeAlias alias) { HASSERT(alias != 0); - AliasMap::Iterator it = aliasMap.find(alias); + auto it = aliasMap.find(alias); if (it != aliasMap.end()) { - Metadata *metadata = (*it).second; - + Metadata *metadata = it->deref(this); + /* update timestamp */ touch(metadata); - return metadata->id; + return metadata->get_node_id(); } /* no match found */ @@ -246,9 +246,10 @@ void AliasCache::for_each(void (*callback)(void*, NodeID, NodeAlias), void *cont { HASSERT(callback != NULL); - for (Metadata *metadata = newest; metadata != NULL; metadata = metadata->older) + for (PoolIdx idx = newest; !idx.empty(); idx = idx.deref(this)->older_) { - (*callback)(context, metadata->id, metadata->alias); + Metadata *metadata = idx.deref(this); + (*callback)(context, metadata->get_node_id(), metadata->alias_); } } @@ -277,25 +278,23 @@ NodeAlias AliasCache::generate() */ void AliasCache::touch(Metadata* metadata) { - metadata->timestamp = OSTime::get_monotonic(); - - if (metadata != newest) + if (metadata != newest.deref(this)) { - if (metadata == oldest) + if (metadata == oldest.deref(this)) { - oldest = metadata->newer; - oldest->older = NULL; + oldest = metadata->newer_; + oldest.deref(this)->older_.idx_ = NONE_ENTRY; } else { /* we have someone older */ - metadata->older->newer = metadata->newer; + metadata->older_.deref(this)->newer_ = metadata->newer_; } - metadata->newer->older = metadata->older; - metadata->newer = NULL; - metadata->older = newest; - newest->newer = metadata; - newest = metadata; + metadata->newer_.deref(this)->older_ = metadata->older_; + metadata->newer_.idx_ = NONE_ENTRY; + metadata->older_ = newest; + newest.deref(this)->newer_.idx_ = metadata - pool; + newest.idx_ = metadata - pool; } } diff --git a/src/openlcb/AliasCache.cxxtest b/src/openlcb/AliasCache.cxxtest index 9998d8cb4..6a648a242 100644 --- a/src/openlcb/AliasCache.cxxtest +++ b/src/openlcb/AliasCache.cxxtest @@ -467,16 +467,18 @@ namespace openlcb { int AliasCache::check_consistency() { if (idMap.size() != aliasMap.size()) return 1; if (aliasMap.size() == entries) { - if (freeList != nullptr) return 2; + if (!freeList.empty()) return 2; } else { - if (freeList == nullptr) return 3; + if (freeList.empty()) return 3; } - if (aliasMap.size() == 0 && - (oldest != nullptr || newest != nullptr)) { + if (aliasMap.size() == 0 && (!oldest.empty() || !newest.empty())) + { return 4; } std::set free_entries; - for (Metadata* m = freeList; m; m=m->next) { + for (PoolIdx p = freeList; !p.empty(); p = p.deref(this)->older_) + { + Metadata *m = p.deref(this); if (free_entries.count(m)) { return 5; // duplicate entry on freelist } @@ -486,66 +488,74 @@ int AliasCache::check_consistency() { return 6; // lost some metadata entries } for (auto kv : aliasMap) { - if (free_entries.count(kv.second)) { + if (free_entries.count(kv.deref(this))) + { return 19; } } for (auto kv : idMap) { - if (free_entries.count(kv.second)) { + if (free_entries.count(kv.deref(this))) + { return 20; } } if (aliasMap.size() == 0) { - if (oldest != nullptr) return 7; - if (newest != nullptr) return 8; + if (!oldest.empty()) return 7; + if (!newest.empty()) return 8; } else { - if (oldest == nullptr) return 9; - if (newest == nullptr) return 10; + if (oldest.empty()) return 9; + if (newest.empty()) return 10; } - if (free_entries.count(oldest)) { + if (free_entries.count(oldest.deref(this))) + { return 11; // oldest is free } - if (free_entries.count(newest)) { + if (free_entries.count(newest.deref(this))) + { return 12; // newest is free } if (aliasMap.size() == 0) return 0; // Check linking. { - Metadata* prev = oldest; + PoolIdx prev = oldest; unsigned count = 1; - if (prev->older) return 13; - while (prev->newer) { - auto* next = prev->newer; + if (!prev.deref(this)->older_.empty()) return 13; + while (!prev.deref(this)->newer_.empty()) + { + auto next = prev.deref(this)->newer_; ++count; - if (free_entries.count(next)) { + if (free_entries.count(next.deref(this))) + { return 21; } - if (next->older != prev) return 14; + if (next.deref(this)->older_.idx_ != prev.idx_) return 14; prev = next; } - if (prev != newest) return 18; + if (prev.idx_ != newest.idx_) return 18; if (count != aliasMap.size()) return 27; } { - Metadata* next = newest; - if (next->newer) return 15; - while (next->older) { - auto* prev = next->older; - if (free_entries.count(prev)) { + PoolIdx next = newest; + if (!next.deref(this)->newer_.empty()) return 15; + while (!next.deref(this)->older_.empty()) + { + auto prev = next.deref(this)->older_; + if (free_entries.count(prev.deref(this))) + { return 22; } - if (prev->newer != next) return 16; + if (prev.deref(this)->newer_.idx_ != next.idx_) return 16; next = prev; } - if (next != oldest) return 17; + if (next.idx_ != oldest.idx_) return 17; } for (unsigned i = 0; i < entries; ++i) { if (free_entries.count(pool+i)) continue; auto* e = pool+i; - if (idMap.find(e->id) == idMap.end()) return 23; - if (idMap[e->id] != e) return 24; - if (aliasMap.find(e->alias) == aliasMap.end()) return 25; - if (aliasMap[e->alias] != e) return 26; + if (idMap.find(e->get_node_id()) == idMap.end()) return 23; + if (idMap.find(e->get_node_id())->idx_ != i) return 24; + if (aliasMap.find(e->alias_) == aliasMap.end()) return 25; + if (aliasMap.find(e->alias_)->idx_ != i) return 26; } return 0; } diff --git a/src/openlcb/AliasCache.hxx b/src/openlcb/AliasCache.hxx index b59ddc55d..fdbafbe57 100644 --- a/src/openlcb/AliasCache.hxx +++ b/src/openlcb/AliasCache.hxx @@ -35,9 +35,9 @@ #define _OPENLCB_ALIASCACHE_HXX_ #include "openlcb/Defs.hxx" -#include "utils/macros.h" #include "utils/Map.hxx" -#include "utils/RBTree.hxx" +#include "utils/SortedListMap.hxx" +#include "utils/macros.h" namespace openlcb { @@ -48,8 +48,40 @@ namespace openlcb * is no mutual exclusion locking mechanism built into this class. Mutual * exclusion must be handled by the user as needed. * - * @todo the class uses RBTree, consider a version that is a linear search for - * a small number of entries. + * This data structure is sometimes used with very large entry count + * (hundreds), therefore we must be careful about memory efficiency! + * + * Theory of operation: + * + * The data structure has three access patterns: + * - lookup of alias -> ID + * - lookup of ID -> alias + * - eviction of oldest entry from the cache + * + * We have three data structures to match these use-cases: + * + * The struct Metadata stores the actual NodeID and NodeAlias values. This is + * laid out in the pre-allocated C array `pool`. A freeList shows where unused + * entries are. + * + * To support the eviction of oldest entry, an LRU doubly-linked list is + * created in these Metadata entries. The links are represented by indexes into + * the `pool` array. + * + * Indexes into the `pool` are encapsulated into the PoolIdx struct to make + * them a unique C++ type. This is needed for template disambiguation. We also + * have a dereference function on PoolIdx that turns it into a Metadata + * pointer. + * + * To support lookup by alias, we have a SortedListSet which contains all used + * indexes as PoolIdx, sorted by the alias property in the respective entry in + * `pool`. This is achieved by a custom comparator that dereferences the + * PoolIdx object and fetches the alias from the Metadata struct. The sorted + * vector is maintained using the SortedListSet<> template, and takes a total + * of only 2 bytes per entry. + * + * A similar sorted vector is kept sorted by the NodeID values. This also takes + * only 2 bytes per entry. */ class AliasCache { @@ -62,24 +94,25 @@ public: * @param context context pointer to pass to remove_callback */ AliasCache(NodeID seed, size_t _entries, - void (*remove_callback)(NodeID id, NodeAlias alias, void *) = NULL, - void *context = NULL) - : pool(new Metadata[_entries]), - freeList(NULL), - aliasMap(_entries), - idMap(_entries), - oldest(NULL), - newest(NULL), - seed(seed), - entries(_entries), - removeCallback(remove_callback), - context(context) + void (*remove_callback)(NodeID id, NodeAlias alias, void *) = NULL, + void *context = NULL) + : pool(new Metadata[_entries]) + , aliasMap(this) + , idMap(this) + , seed(seed) + , entries(_entries) + , removeCallback(remove_callback) + , context(context) { + aliasMap.reserve(_entries); + idMap.reserve(_entries); clear(); } /** This NodeID will be used for reserved but unused local aliases. */ static const NodeID RESERVED_ALIAS_NODE_ID; + /// Sentinel entry for empty lists. + static constexpr uint16_t NONE_ENTRY = 0xFFFFu; /** Reinitializes the entire map. */ void clear(); @@ -126,12 +159,12 @@ public: * changes. * @param entry is between 0 and size() - 1. * @param node will be filled with the node ID. May be null. - * @param aliad will be filles with the alias. May be null. + * @param alias will be filled with the alias. May be null. * @return true if the entry is valid, and node and alias were filled, otherwise false if the entry is not allocated. */ bool retrieve(unsigned entry, NodeID* node, NodeAlias* alias); - /** Generate a 12-bit pseudo-random alias for a givin alias cache. + /** Generate a 12-bit pseudo-random alias for a given alias cache. * @return pseudo-random 12-bit alias, an alias of zero is invalid */ NodeAlias generate(); @@ -152,47 +185,172 @@ private: UNUSED_MASK = 0x10000000 }; + struct Metadata; + class PoolIdx; + friend class PoolIdx; + + /// Encapsulation of a pointer into the pool array. + class PoolIdx + { + public: + /// Constructor. Sets the pointer to invalid. + PoolIdx() + : idx_(NONE_ENTRY) + { + } + /// @return true if this entry does not point anywhere. + bool empty() + { + return idx_ == NONE_ENTRY; + } + /// Indexes the pool of the AliasCache. + uint16_t idx_; + /// Dereferences a pool index as if it was a pointer. + /// @param parent the AliasCache whose pool to index. + /// @return referenced Metadata pointer. + Metadata *deref(AliasCache *parent) + { + DASSERT(idx_ != NONE_ENTRY); + return parent->pool + idx_; + } + }; + /** Interesting information about a given cache entry. */ struct Metadata { - NodeID id = 0; /**< 48-bit NMRAnet Node ID */ - NodeAlias alias = 0; /**< NMRAnet alias */ - long long timestamp; /**< time stamp of last usage */ - union + /// Sets the node ID field. + /// @param id the node ID to set. + void set_node_id(NodeID id) { - Metadata *prev; /**< unused */ - Metadata *newer; /**< pointer to the next newest entry */ - }; - union + nodeIdLow_ = id & 0xFFFFFFFFu; + nodeIdHigh_ = (id >> 32) & 0xFFFFu; + } + + /// @return the node ID field. + NodeID get_node_id() { - Metadata *next; /**< pointer to next freeList entry */ - Metadata *older; /**< pointer to the next oldest entry */ - }; + uint64_t h = nodeIdHigh_; + h <<= 32; + h |= nodeIdLow_; + return h; + } + + /// OpenLCB Node ID low 32 bits. + uint32_t nodeIdLow_ = 0; + /// OpenLCB Node ID high 16 bits. + uint16_t nodeIdHigh_ = 0; + /// OpenLCB-CAN alias + NodeAlias alias_ = 0; + /// Index of next-newer entry according to the LRU linked list. + PoolIdx newer_; + /// Index of next-older entry according to the LRU linked list. + PoolIdx older_; }; /** pointer to allocated Metadata pool */ Metadata *pool; - - /** list of unused mapping entries */ - Metadata *freeList; - + + /// Comparator object comparing the aliases stored in the pool. + class AliasComparator + { + public: + /// Constructor + /// @param parent owning AliasCache. + AliasComparator(AliasCache *parent) + : parent_(parent) + { + } + + /// Less-than action. + /// @param e left hand side + /// @param alias right hand side + bool operator()(PoolIdx e, uint16_t alias) const + { + return e.deref(parent_)->alias_ < alias; + } + + /// Less-than action. + /// @param alias left hand side + /// @param e right hand side + bool operator()(uint16_t alias, PoolIdx e) const + { + return alias < e.deref(parent_)->alias_; + } + + /// Less-than action. + /// @param a left hand side + /// @param b right hand side + bool operator()(PoolIdx a, PoolIdx b) const + { + return a.deref(parent_)->alias_ < b.deref(parent_)->alias_; + } + + private: + /// AliasCache whose pool we are indexing into. + AliasCache *parent_; + }; + + /// Comparator object comparing the aliases stored in the pool. + class IdComparator + { + public: + /// Constructor + /// @param parent owning AliasCache. + IdComparator(AliasCache *parent) + : parent_(parent) + { + } + + /// Less-than action. + /// @param e left hand side + /// @param id right hand side + bool operator()(PoolIdx e, NodeID id) const + { + return e.deref(parent_)->get_node_id() < id; + } + + /// Less-than action. + /// @param id left hand side + /// @param e right hand side + bool operator()(NodeID id, PoolIdx e) const + { + return id < e.deref(parent_)->get_node_id(); + } + + /// Less-than action. + /// @param a left hand side + /// @param b right hand side + bool operator()(PoolIdx a, PoolIdx b) const + { + return a.deref(parent_)->get_node_id() < + b.deref(parent_)->get_node_id(); + } + + private: + /// AliasCache whose pool we are indexing into. + AliasCache *parent_; + }; + /** Short hand for the alias Map type */ - typedef Map AliasMap; - + typedef SortedListSet AliasMap; + /** Short hand for the ID Map type */ - typedef Map IdMap; + typedef SortedListSet IdMap; /** Map of alias to corresponding Metadata */ AliasMap aliasMap; /** Map of Node ID to corresponding Metadata */ IdMap idMap; - - /** oldest untouched entry */ - Metadata *oldest; - - /** newest, most recently touched entry */ - Metadata *newest; + + /** list of unused mapping entries (index into pool) */ + PoolIdx freeList; + + /** oldest untouched entry (index into pool) */ + PoolIdx oldest; + + /** newest, most recently touched entry (index into pool) */ + PoolIdx newest; /** Seed for the generation of the next alias */ NodeID seed; @@ -214,6 +372,6 @@ private: DISALLOW_COPY_AND_ASSIGN(AliasCache); }; -} /* namepace NMRAnet */ +} /* namespace openlcb */ -#endif /* _NMRAnetAliasCache_hxx */ +#endif // _OPENLCB_ALIASCACHE_HXX_ diff --git a/src/utils/SortedListMap.hxx b/src/utils/SortedListMap.hxx index 293727d58..598e934d5 100644 --- a/src/utils/SortedListMap.hxx +++ b/src/utils/SortedListMap.hxx @@ -59,10 +59,19 @@ public: /// Const Iterator type. typedef typename container_type::const_iterator const_iterator; - SortedListSet() + template + SortedListSet(Args &&...args) + : cmp_(std::forward(args)...) { } + /// Ensures that a given size can be reached without memory allocation. + /// @param sz the number of entries to prepare for. + void reserve(size_t sz) + { + container_.reserve(sz); + } + /// @return first iterator. iterator begin() { @@ -86,8 +95,8 @@ public: iterator lower_bound(key_type key) { lazy_init(); - return std::lower_bound(container_.begin(), container_.end(), key, - CMP()); + return std::lower_bound( + container_.begin(), container_.end(), key, cmp_); } /// @param key what to search for @return iterator, see std::upper_bound. @@ -95,8 +104,8 @@ public: iterator upper_bound(key_type key) { lazy_init(); - return std::upper_bound(container_.begin(), container_.end(), key, - CMP()); + return std::upper_bound( + container_.begin(), container_.end(), key, cmp_); } /// Searches for a single entry. @param key is what to search for. @return @@ -116,8 +125,8 @@ public: std::pair equal_range(key_type key) { lazy_init(); - return std::equal_range(container_.begin(), container_.end(), key, - CMP()); + return std::equal_range( + container_.begin(), container_.end(), key, cmp_); } /// Adds new entry to the vector. @@ -154,7 +163,7 @@ private: { if (sortedCount_ != container_.size()) { - sort(container_.begin(), container_.end(), CMP()); + sort(container_.begin(), container_.end(), cmp_); sortedCount_ = container_.size(); } } @@ -162,6 +171,9 @@ private: /// Holds the actual data elements. container_type container_; + /// Comparator instance. + CMP cmp_; + /// The first this many elements in the container are already sorted. size_t sortedCount_{0}; };