From 8564980847124346c5e7ada01babbd93aafd9255 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 2 Jun 2021 19:41:09 +0000 Subject: Use a hash map for specs. If the request is a point lookup then just use a hash lookup. If it is a wildcard lookup iterate as earlier on. Also use vespalib::stringref in interface to avoid conversion. Use vespalib:string in the hash map to locate string in object aswe are still on old abi. --- slobrok/src/tests/mirrorapi/match_test.cpp | 2 +- slobrok/src/vespa/slobrok/backoff.h | 9 ++--- slobrok/src/vespa/slobrok/imirrorapi.h | 11 ++---- slobrok/src/vespa/slobrok/sbmirror.cpp | 59 +++++++++++++----------------- slobrok/src/vespa/slobrok/sbmirror.h | 13 ++++--- slobrok/src/vespa/slobrok/sbregister.cpp | 1 - 6 files changed, 41 insertions(+), 54 deletions(-) (limited to 'slobrok') diff --git a/slobrok/src/tests/mirrorapi/match_test.cpp b/slobrok/src/tests/mirrorapi/match_test.cpp index d350718a671..0686390565a 100644 --- a/slobrok/src/tests/mirrorapi/match_test.cpp +++ b/slobrok/src/tests/mirrorapi/match_test.cpp @@ -4,7 +4,7 @@ class MatchTester : public slobrok::api::IMirrorAPI { - SpecList lookup(const std::string &) const override { + SpecList lookup(vespalib::stringref ) const override { return SpecList(); } uint32_t updates() const override { return 0; } diff --git a/slobrok/src/vespa/slobrok/backoff.h b/slobrok/src/vespa/slobrok/backoff.h index 53193a9de6e..5d410635088 100644 --- a/slobrok/src/vespa/slobrok/backoff.h +++ b/slobrok/src/vespa/slobrok/backoff.h @@ -1,10 +1,9 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -#include +#include -namespace slobrok { -namespace api { +namespace slobrok::api { class BackOff { @@ -20,6 +19,4 @@ public: bool shouldWarn(); }; -} // namespace api -} // namespace slobrok - +} diff --git a/slobrok/src/vespa/slobrok/imirrorapi.h b/slobrok/src/vespa/slobrok/imirrorapi.h index 5cce9f1f49e..508847cbfeb 100644 --- a/slobrok/src/vespa/slobrok/imirrorapi.h +++ b/slobrok/src/vespa/slobrok/imirrorapi.h @@ -1,11 +1,10 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -#include +#include #include -namespace slobrok { -namespace api { +namespace slobrok::api { /** * @brief Defines an interface for the name server lookup. @@ -48,7 +47,7 @@ public: * @return a list of all matching services, with corresponding connect specs * @param pattern The pattern used for matching **/ - virtual SpecList lookup(const std::string & pattern) const = 0; + virtual SpecList lookup(vespalib::stringref pattern) const = 0; /** * Obtain the number of updates seen by this mirror. The value may wrap, but will never become 0 again. This can be @@ -62,6 +61,4 @@ public: virtual bool ready() const = 0; }; -} // namespace api -} // namespace slobrok - +} diff --git a/slobrok/src/vespa/slobrok/sbmirror.cpp b/slobrok/src/vespa/slobrok/sbmirror.cpp index 13680d16e68..5f6a54504e5 100644 --- a/slobrok/src/vespa/slobrok/sbmirror.cpp +++ b/slobrok/src/vespa/slobrok/sbmirror.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include LOG_SETUP(".slobrok.mirror"); @@ -53,14 +54,22 @@ MirrorAPI::~MirrorAPI() MirrorAPI::SpecList -MirrorAPI::lookup(const std::string & pattern) const +MirrorAPI::lookup(vespalib::stringref pattern) const { SpecList ret; + ret.reserve(1); + bool exact = pattern.find('*') == std::string::npos; std::lock_guard guard(_lock); - SpecList::const_iterator end = _specs.end(); - for (SpecList::const_iterator it = _specs.begin(); it != end; ++it) { - if (match(it->first.c_str(), pattern.c_str())) { - ret.push_back(*it); + if (exact) { + auto found = _specs.find(pattern); + if (found != _specs.end()) { + ret.emplace_back(found->first, found->second); + } + } else { + for (const auto & spec : _specs) { + if (match(spec.first.c_str(), pattern.data())) { + ret.emplace_back(spec.first, spec.second); + } } } return ret; @@ -77,8 +86,6 @@ MirrorAPI::lookup(const std::string & pattern) const bool IMirrorAPI::match(const char *name, const char *pattern) { - LOG_ASSERT(name != NULL); - LOG_ASSERT(pattern != NULL); while (*pattern != '\0') { if (*name == *pattern) { ++name; @@ -102,11 +109,11 @@ IMirrorAPI::match(const char *name, const char *pattern) void -MirrorAPI::updateTo(SpecList& newSpecs, uint32_t newGen) +MirrorAPI::updateTo(SpecMap newSpecs, uint32_t newGen) { { std::lock_guard guard(_lock); - std::swap(newSpecs, _specs); + _specs = std::move(newSpecs); _updates.add(); } _specsGen.setFromInt(newGen); @@ -167,34 +174,26 @@ MirrorAPI::handleIncrementalFetch() LOG(spam, "incremental diff [%u;%u] full dump, but numRemove=%u, numNames=%u", diff_from, diff_to, numRemove, numNames); } - SpecList specs; + SpecMap specs; for (uint32_t idx = 0; idx < numNames; idx++) { - specs.push_back( - std::make_pair(std::string(n[idx]._str), - std::string(s[idx]._str))); + specs[n[idx]._str] = s[idx]._str; } - updateTo(specs, diff_to); + updateTo(std::move(specs), diff_to); } else if (_specsGen == diff_from) { // incremental update - SpecList specs; - SpecList::const_iterator end = _specs.end(); - for (SpecList::const_iterator it = _specs.begin(); - it != end; - ++it) - { + SpecMap specs; + for (const auto & spec : _specs) { bool keep = true; for (uint32_t idx = 0; idx < numRemove; idx++) { - if (it->first == r[idx]._str) keep = false; + if (spec.first == r[idx]._str) keep = false; } for (uint32_t idx = 0; idx < numNames; idx++) { - if (it->first == n[idx]._str) keep = false; + if (spec.first == n[idx]._str) keep = false; } - if (keep) specs.push_back(*it); + if (keep) specs[spec.first] = spec.second; } for (uint32_t idx = 0; idx < numNames; idx++) { - specs.push_back( - std::make_pair(std::string(n[idx]._str), - std::string(s[idx]._str))); + specs[n[idx]._str] = s[idx]._str; } updateTo(specs, diff_to); } @@ -222,13 +221,7 @@ MirrorAPI::handleReqDone() if (_reqDone) { _reqDone = false; _reqPending = false; - bool reconn = (_target == 0); - - if (_req->IsError()) { - reconn = true; - } else { - reconn = handleIncrementalFetch(); - } + bool reconn = _req->IsError() ? true : handleIncrementalFetch(); if (reconn) { if (_target != 0) { diff --git a/slobrok/src/vespa/slobrok/sbmirror.h b/slobrok/src/vespa/slobrok/sbmirror.h index c1c7009ce12..ad86daa56bb 100644 --- a/slobrok/src/vespa/slobrok/sbmirror.h +++ b/slobrok/src/vespa/slobrok/sbmirror.h @@ -5,6 +5,7 @@ #include "backoff.h" #include "sblist.h" #include +#include #include class FRT_Target; @@ -40,6 +41,8 @@ public: * @param config how to get the connect spec list **/ MirrorAPI(FRT_Supervisor &orb, const ConfiguratorFactory & config); + MirrorAPI(const MirrorAPI &) = delete; + MirrorAPI &operator=(const MirrorAPI &) = delete; /** * @brief Clean up. @@ -47,7 +50,7 @@ public: ~MirrorAPI(); // Inherit doc from IMirrorAPI. - SpecList lookup(const std::string & pattern) const override; + SpecList lookup(vespalib::stringref pattern) const override; // Inherit doc from IMirrorAPI. uint32_t updates() const override { return _updates.getAsInt(); } @@ -67,16 +70,14 @@ public: bool ready() const override; private: - MirrorAPI(const MirrorAPI &); - MirrorAPI &operator=(const MirrorAPI &); - + using SpecMap = vespalib::hash_map; /** from FNET_Task, polls slobrok **/ void PerformTask() override; /** from FRT_IRequestWait **/ void RequestDone(FRT_RPCRequest *req) override; - void updateTo(SpecList& newSpecs, uint32_t newGen); + void updateTo(SpecMap newSpecs, uint32_t newGen); bool handleIncrementalFetch(); @@ -93,7 +94,7 @@ private: bool _scheduled; bool _reqDone; bool _logOnSuccess; - SpecList _specs; + SpecMap _specs; vespalib::GenCnt _specsGen; vespalib::GenCnt _updates; SlobrokList _slobrokSpecs; diff --git a/slobrok/src/vespa/slobrok/sbregister.cpp b/slobrok/src/vespa/slobrok/sbregister.cpp index 06a9fb2d79e..357e3fbd1ce 100644 --- a/slobrok/src/vespa/slobrok/sbregister.cpp +++ b/slobrok/src/vespa/slobrok/sbregister.cpp @@ -6,7 +6,6 @@ #include #include #include -#include #include LOG_SETUP(".slobrok.register"); -- cgit v1.2.3