Skip to content

Commit

Permalink
remove memcache::Trie from PrefixSelectorRoute (#441)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #441

migrating to LowerBoundPrefixMap

Reviewed By: udippant

Differential Revision: D53852214

fbshipit-source-id: c3646dd2dc1c7c5cbde256e9963706139d89417c
  • Loading branch information
DenisYaroshevskiy authored and facebook-github-bot committed Feb 22, 2024
1 parent d579bfb commit 49e54da
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 8 deletions.
11 changes: 8 additions & 3 deletions mcrouter/routes/PrefixSelectorRoute.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#include <folly/json/dynamic.h>

#include "mcrouter/lib/config/RouteHandleFactory.h"
#include "mcrouter/lib/fbi/cpp/Trie.h"
#include "mcrouter/lib/fbi/cpp/LowerBoundPrefixMap.h"
#include "mcrouter/lib/fbi/cpp/util.h"

namespace facebook {
Expand All @@ -31,7 +31,7 @@ template <class RouteHandleIf>
class PrefixSelectorRoute {
public:
/// Trie that acts like map from key prefix to corresponding RouteHandle.
Trie<std::shared_ptr<RouteHandleIf>> policies;
LowerBoundPrefixMap<std::shared_ptr<RouteHandleIf>> policies;
/// Used when no RouteHandle found in policies
std::shared_ptr<RouteHandleIf> wildcard;

Expand Down Expand Up @@ -64,9 +64,14 @@ class PrefixSelectorRoute {
}
// order is important
std::sort(items.begin(), items.end());

typename LowerBoundPrefixMap<std::shared_ptr<RouteHandleIf>>::Builder
builder;
builder.reserve(items.size());
for (const auto& it : items) {
policies.emplace(it.first, factory.create(*it.second));
builder.insert({std::string(it.first), factory.create(*it.second)});
}
policies = std::move(builder).build();
}

if (jWildcard) {
Expand Down
7 changes: 4 additions & 3 deletions mcrouter/routes/RoutePolicyMap-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,10 @@ RoutePolicyMap<RouteHandleIf>::RoutePolicyMap(
// Combining routes for each key
folly::F14FastSet<std::string_view> seen;
for (const auto& cluster : clusters) {
for (const auto& [key, _] : cluster->policies) {
for (const auto& policy : cluster->policies) {
std::string_view key = policy.key();
if (seen.insert(key).second) {
builder.insert({key, populateRoutesForKey(key, clusters)});
builder.insert({std::string(key), populateRoutesForKey(key, clusters)});
}
}
}
Expand Down Expand Up @@ -104,7 +105,7 @@ auto RoutePolicyMap<RouteHandleIf>::populateRoutesForKey(
for (const auto& cluster : clusters) {
auto found = cluster->policies.findPrefix(key);
const SharedRoutePtr& ptr =
found == cluster->policies.end() ? cluster->wildcard : found->second;
found == cluster->policies.end() ? cluster->wildcard : found->value();
if (ptr && seen.insert(ptr.get()).second) {
res.push_back(ptr);
}
Expand Down
2 changes: 1 addition & 1 deletion mcrouter/routes/test/PrefixSelectorRouteTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ void constructionTest(
};
std::vector<std::pair<std::string, RouteHandle>> policies;
for (const auto& p : route.policies) {
policies.emplace_back(std::string(p.first), toInt(p.second));
policies.emplace_back(std::string(p.key()), toInt(p.value()));
}

EXPECT_EQ(expected.wildcard, toInt(route.wildcard));
Expand Down
4 changes: 3 additions & 1 deletion mcrouter/routes/test/RoutePolicyMapTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,11 @@ struct MockPrefixSelectorRoute {
res->wildcard = kRouteHandles[*wildcard];
}

LowerBoundPrefixMap<std::shared_ptr<RouteHandle>>::Builder builder;
for (const auto& [k, v] : policies) {
res->policies.emplace(k, kRouteHandles[v]);
builder.insert({k, kRouteHandles[v]});
}
res->policies = std::move(builder).build();

return res;
}
Expand Down

0 comments on commit 49e54da

Please sign in to comment.