diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2023-08-09 17:00:01 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-08-09 17:00:01 +0200 |
commit | e8c966890fbc22d6ce5556b0ba708bf0dec4c739 (patch) | |
tree | 74d0ca9e073fb0fdb42599f0ca34f3e40466c3fb | |
parent | eed1ed31709908ec5317369af9955a9f2862fbfe (diff) | |
parent | 3e37e84a207f52fd542a0afcf514080e8587b970 (diff) |
Merge pull request #28001 from vespa-engine/balder/use-hash-map-for-faster-lookups
- Use a hashmap for string Node/NodeState in ClusterState for fast lo…
6 files changed, 37 insertions, 34 deletions
diff --git a/persistence/src/vespa/persistence/spi/clusterstate.cpp b/persistence/src/vespa/persistence/spi/clusterstate.cpp index ad5039fade1..e6708192d47 100644 --- a/persistence/src/vespa/persistence/spi/clusterstate.cpp +++ b/persistence/src/vespa/persistence/spi/clusterstate.cpp @@ -97,7 +97,7 @@ void ClusterState::serialize(vespalib::nbostream& o) const { assert(_distribution); assert(_state); vespalib::asciistream tmp; - _state->serialize(tmp, false); + _state->serialize(tmp); o << tmp.str() << _nodeIndex; o << _distribution->serialize(); } diff --git a/storage/src/vespa/storage/storageserver/rpc/slime_cluster_state_bundle_codec.cpp b/storage/src/vespa/storage/storageserver/rpc/slime_cluster_state_bundle_codec.cpp index 8c994991b9b..ea049493348 100644 --- a/storage/src/vespa/storage/storageserver/rpc/slime_cluster_state_bundle_codec.cpp +++ b/storage/src/vespa/storage/storageserver/rpc/slime_cluster_state_bundle_codec.cpp @@ -44,7 +44,7 @@ OutputBuf::~OutputBuf() = default; vespalib::string serialize_state(const lib::ClusterState& state) { vespalib::asciistream as; - state.serialize(as, false); + state.serialize(as); return as.str(); } diff --git a/vdslib/src/tests/state/clusterstatetest.cpp b/vdslib/src/tests/state/clusterstatetest.cpp index a08ec007f55..0b278177453 100644 --- a/vdslib/src/tests/state/clusterstatetest.cpp +++ b/vdslib/src/tests/state/clusterstatetest.cpp @@ -13,10 +13,10 @@ using ::testing::ContainsRegex; namespace storage::lib { -#define VERIFY3(source, result, type, typestr) { \ +#define VERIFY3(source, result, typestr) { \ vespalib::asciistream ost; \ try { \ - state->serialize(ost, type); \ + state->serialize(ost); \ } catch (std::exception& e) { \ FAIL() << ("Failed to serialize system state " \ + state->toString(true) + " in " + std::string(typestr) \ @@ -26,24 +26,18 @@ namespace storage::lib { vespalib::string(typestr) + " \"" + ost.str() + "\"") << state->toString(true); \ } -#define VERIFY2(serialized, result, testOld, testNew) { \ +#define VERIFY2(serialized, result) { \ std::unique_ptr<ClusterState> state; \ try { \ state.reset(new ClusterState(serialized)); \ } catch (std::exception& e) { \ - FAIL() << ("Failed to parse '" + std::string(serialized) \ - + "': " + e.what()); \ + FAIL() << ("Failed to parse '" + std::string(serialized) + "': " + e.what()); \ } \ - if (testOld) VERIFY3(serialized, result, true, "Old") \ - if (testNew) VERIFY3(serialized, result, false, "New") \ + VERIFY3(serialized, result, "New") \ } -#define VERIFYSAMEOLD(serialized) VERIFY2(serialized, serialized, true, false) -#define VERIFYOLD(serialized, result) VERIFY2(serialized, result, true, false) -#define VERIFYSAMENEW(serialized) VERIFY2(serialized, serialized, false, true) -#define VERIFYNEW(serialized, result) VERIFY2(serialized, result, false, true) -#define VERIFYSAME(serialized) VERIFY2(serialized, serialized, true, true) -#define VERIFY(serialized, result) VERIFY2(serialized, result, true, true) +#define VERIFYSAMENEW(serialized) VERIFY2(serialized, serialized) +#define VERIFYNEW(serialized, result) VERIFY2(serialized, result) #define VERIFY_FAIL(serialized, error) { \ try{ \ diff --git a/vdslib/src/vespa/vdslib/state/clusterstate.cpp b/vdslib/src/vespa/vdslib/state/clusterstate.cpp index b796e357d24..f4314a6624b 100644 --- a/vdslib/src/vespa/vdslib/state/clusterstate.cpp +++ b/vdslib/src/vespa/vdslib/state/clusterstate.cpp @@ -7,6 +7,8 @@ #include <vespa/vdslib/distribution/distribution.h> #include <vespa/vespalib/util/exceptions.h> #include <vespa/vespalib/stllike/asciistream.h> +#include <vespa/vespalib/stllike/hash_map.hpp> +#include <vespa/vespalib/stllike/hash_map_equal.hpp> #include <sstream> #include <cassert> @@ -193,11 +195,11 @@ struct SeparatorPrinter { namespace { void -serialize_node(vespalib::asciistream & out, const std::pair<Node, NodeState> & entry) { +serialize_node(vespalib::asciistream & out, const Node & node, const NodeState & state) { vespalib::asciistream prefix; - prefix << "." << entry.first.getIndex() << "."; + prefix << "." << node.getIndex() << "."; vespalib::asciistream ost; - entry.second.serialize(ost, prefix.str(), false); + state.serialize(ost, prefix.str(), false); vespalib::stringref content = ost.str(); if ( !content.empty()) { out << " " << content; @@ -207,36 +209,40 @@ serialize_node(vespalib::asciistream & out, const std::pair<Node, NodeState> & e } void -ClusterState::serialize_nodes(vespalib::asciistream & out, bool ignoreNewFeatures, - SeparatorPrinter & sep, const NodeType & nodeType) const +ClusterState::serialize_nodes(vespalib::asciistream & out, SeparatorPrinter & sep, const NodeType & nodeType, + const std::vector<NodeStatePair> & nodeStates) const { uint16_t nodeCount = getNodeCount(nodeType); - if (ignoreNewFeatures || nodeCount > 0) { + if (nodeCount > 0) { out << sep.toString() << nodeType.serialize() << ":" << nodeCount; - for (const auto & entry : _nodeStates) { + for (const auto & entry : nodeStates) { if (entry.first.getType() == nodeType) { - serialize_node(out, entry); + serialize_node(out, entry.first, entry.second); } } } } void -ClusterState::serialize(vespalib::asciistream & out, bool ignoreNewFeatures) const +ClusterState::serialize(vespalib::asciistream & out) const { SeparatorPrinter sep; - if (!ignoreNewFeatures && _version != 0) { + if (_version != 0) { out << sep.toString() << "version:" << _version; } - if (!ignoreNewFeatures && *_clusterState != State::UP) { + if (*_clusterState != State::UP) { out << sep.toString() << "cluster:" << _clusterState->serialize(); } - if (!ignoreNewFeatures && _distributionBits != 16) { + if (_distributionBits != 16) { out << sep.toString() << "bits:" << _distributionBits; } - serialize_nodes(out, ignoreNewFeatures, sep, NodeType::DISTRIBUTOR); - serialize_nodes(out, ignoreNewFeatures, sep, NodeType::STORAGE); + if ((getNodeCount(NodeType::DISTRIBUTOR) + getNodeCount(NodeType::STORAGE)) == 0u) return; + + std::vector<NodeStatePair> nodeStates(_nodeStates.cbegin(), _nodeStates.cend()); + std::sort(nodeStates.begin(), nodeStates.end(), [](const NodeStatePair &a, const NodeStatePair &b) { return a.first < b.first; }); + serialize_nodes(out, sep, NodeType::DISTRIBUTOR, nodeStates); + serialize_nodes(out, sep, NodeType::STORAGE, nodeStates); } bool @@ -320,7 +326,7 @@ ClusterState::print(std::ostream& out, bool verbose, const std::string&) const { (void) verbose; vespalib::asciistream tmp; - serialize(tmp, false); + serialize(tmp); out << tmp.str(); } diff --git a/vdslib/src/vespa/vdslib/state/clusterstate.h b/vdslib/src/vespa/vdslib/state/clusterstate.h index 70f4d59977a..90ec7c1aa65 100644 --- a/vdslib/src/vespa/vdslib/state/clusterstate.h +++ b/vdslib/src/vespa/vdslib/state/clusterstate.h @@ -10,7 +10,7 @@ #include "node.h" #include "nodestate.h" -#include <map> +#include <vespa/vespalib/stllike/hash_map.h> #include <array> namespace storage::lib { @@ -22,7 +22,8 @@ struct SeparatorPrinter; class ClusterState : public document::Printable { public: - using NodeMap = std::map<Node, NodeState>; + using NodeStatePair = std::pair<Node, NodeState>; + using NodeMap = vespalib::hash_map<Node, NodeState>; using NodeCounts = std::array<uint16_t, 2>; using CSP = std::shared_ptr<const ClusterState>; using SP = std::shared_ptr<ClusterState>; @@ -37,7 +38,7 @@ public: ~ClusterState(); std::string getTextualDifference(const ClusterState& other) const; - void serialize(vespalib::asciistream & out, bool ignoreNewFeatures) const; + void serialize(vespalib::asciistream & out) const; bool operator==(const ClusterState& other) const noexcept; bool operator!=(const ClusterState& other) const noexcept; @@ -71,7 +72,8 @@ private: void printStateGroupwise(std::ostream& out, const Group&, bool verbose, const std::string& indent, bool rootGroup) const; void getTextualDifference(std::ostringstream& builder, const NodeType& type, const ClusterState& other) const; size_t printStateGroupwise(std::ostream& out, const Group&, bool verbose, const std::string& indent, const NodeType& type) const; - void serialize_nodes(vespalib::asciistream & out, bool ignoreNewFeatures, SeparatorPrinter & sep, const NodeType & nodeType) const; + void serialize_nodes(vespalib::asciistream & out, SeparatorPrinter & sep, const NodeType & nodeType, + const std::vector<NodeStatePair> & nodeStates) const; uint32_t _version; NodeCounts _nodeCount; const State* _clusterState; diff --git a/vdslib/src/vespa/vdslib/state/node.h b/vdslib/src/vespa/vdslib/state/node.h index 5dfd6a2ccf4..49c8f0e641b 100644 --- a/vdslib/src/vespa/vdslib/state/node.h +++ b/vdslib/src/vespa/vdslib/state/node.h @@ -22,6 +22,7 @@ public: const NodeType& getType() const noexcept { return *_type; } uint16_t getIndex() const noexcept { return _index; } + uint32_t hash() const noexcept { return (_index << 1) | *_type; } bool operator==(const Node& other) const noexcept { return (other._index == _index && *other._type == *_type); |