-
Notifications
You must be signed in to change notification settings - Fork 894
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: several improvements around sorted map #1699
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,35 +22,14 @@ using namespace std; | |
|
||
namespace dfly { | ||
|
||
class BPTreeSetTest : public ::testing::Test { | ||
using Node = detail::BPTreeNode<uint64_t>; | ||
|
||
protected: | ||
static constexpr size_t kNumElems = 7000; | ||
|
||
BPTreeSetTest() : mi_alloc_(mi_heap_get_backing()), bptree_(&mi_alloc_) { | ||
} | ||
static void SetUpTestSuite() { | ||
} | ||
namespace { | ||
|
||
void FillTree(unsigned factor = 1) { | ||
for (unsigned i = 0; i < kNumElems; ++i) { | ||
bptree_.Insert(i * factor); | ||
} | ||
} | ||
|
||
bool Validate(); | ||
|
||
static bool Validate(const Node* node, uint64_t ubound); | ||
|
||
MiMemoryResource mi_alloc_; | ||
BPTree<uint64_t> bptree_; | ||
mt19937 generator_{1}; | ||
}; | ||
template <typename Node, typename Policy> | ||
bool ValidateNode(const Node* node, typename Node::KeyT ubound) { | ||
typename Policy::KeyCompareTo cmp; | ||
|
||
bool BPTreeSetTest::Validate(const Node* node, uint64_t ubound) { | ||
for (unsigned i = 1; i < node->NumItems(); ++i) { | ||
if (node->Key(i - 1) >= node->Key(i)) | ||
if (cmp(node->Key(i - 1), node->Key(i)) > -1) | ||
return false; | ||
} | ||
|
||
|
@@ -71,25 +50,72 @@ bool BPTreeSetTest::Validate(const Node* node, uint64_t ubound) { | |
} | ||
} | ||
|
||
return node->Key(node->NumItems() - 1) < ubound; | ||
return cmp(node->Key(node->NumItems() - 1), ubound) == -1; | ||
} | ||
|
||
struct ZsetPolicy { | ||
struct KeyT { | ||
double d; | ||
sds s; | ||
}; | ||
|
||
struct KeyCompareTo { | ||
int operator()(const KeyT& left, const KeyT& right) { | ||
if (left.d < right.d) | ||
return -1; | ||
if (left.d > right.d) | ||
return 1; | ||
|
||
// Note that sdscmp can return values outside of [-1, 1] range. | ||
return sdscmp(left.s, right.s); | ||
} | ||
}; | ||
}; | ||
|
||
using SDSTree = BPTree<ZsetPolicy::KeyT, ZsetPolicy>; | ||
|
||
} // namespace | ||
|
||
class BPTreeSetTest : public ::testing::Test { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this class existed before. |
||
using Node = detail::BPTreeNode<uint64_t>; | ||
|
||
protected: | ||
static constexpr size_t kNumElems = 7000; | ||
|
||
BPTreeSetTest() : mi_alloc_(mi_heap_get_backing()), bptree_(&mi_alloc_) { | ||
} | ||
static void SetUpTestSuite() { | ||
} | ||
|
||
void FillTree(unsigned factor = 1) { | ||
for (unsigned i = 0; i < kNumElems; ++i) { | ||
bptree_.Insert(i * factor); | ||
} | ||
} | ||
|
||
bool Validate(); | ||
|
||
MiMemoryResource mi_alloc_; | ||
BPTree<uint64_t> bptree_; | ||
mt19937 generator_{1}; | ||
}; | ||
|
||
bool BPTreeSetTest::Validate() { | ||
auto* root = bptree_.DEBUG_root(); | ||
if (!root) | ||
return true; | ||
|
||
// node, upper bound | ||
std::vector<pair<Node*, uint64_t>> stack; | ||
vector<pair<const Node*, uint64_t>> stack; | ||
|
||
stack.emplace_back(root, UINT64_MAX); | ||
|
||
while (!stack.empty()) { | ||
Node* node = stack.back().first; | ||
const Node* node = stack.back().first; | ||
uint64_t ubound = stack.back().second; | ||
stack.pop_back(); | ||
|
||
if (!Validate(node, ubound)) | ||
if (!ValidateNode<Node, BPTreePolicy<uint64_t>>(node, ubound)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. made ValidateNode generic - to be able to use it for differrent types of trees |
||
return false; | ||
|
||
if (!node->IsLeaf()) { | ||
|
@@ -353,25 +379,24 @@ TEST_F(BPTreeSetTest, MemoryUsage) { | |
LOG(INFO) << "btree after: " << mi_alloc.used() << " bytes"; | ||
} | ||
|
||
struct ZsetPolicy { | ||
struct KeyT { | ||
double d; | ||
sds s; | ||
}; | ||
TEST_F(BPTreeSetTest, InsertSDS) { | ||
vector<ZsetPolicy::KeyT> vals; | ||
for (unsigned i = 0; i < 256; ++i) { | ||
sds s = sdsempty(); | ||
|
||
struct KeyCompareTo { | ||
int operator()(const KeyT& left, const KeyT& right) { | ||
if (left.d < right.d) | ||
return -1; | ||
if (left.d > right.d) | ||
return 1; | ||
s = sdscatfmt(s, "a%u", i); | ||
vals.emplace_back(ZsetPolicy::KeyT{.d = 1000, .s = s}); | ||
} | ||
|
||
return sdscmp(left.s, right.s); | ||
} | ||
}; | ||
}; | ||
SDSTree tree(&mi_alloc_); | ||
for (size_t i = 0; i < vals.size(); ++i) { | ||
ASSERT_TRUE(tree.Insert(vals[i])); | ||
} | ||
|
||
using SDSTree = BPTree<ZsetPolicy::KeyT, ZsetPolicy>; | ||
for (auto v : vals) { | ||
sdsfree(v.s); | ||
} | ||
} | ||
|
||
static string RandomString(mt19937& rand, unsigned len) { | ||
const string_view alpanum = "1234567890abcdefghijklmnopqrstuvwxyz"; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,28 +19,13 @@ namespace dfly { | |
|
||
namespace { | ||
|
||
union DoubleUnion { | ||
double d; | ||
uint64_t u; | ||
}; | ||
|
||
inline double GetValue(sds key) { | ||
char* valptr = key + sdslen(key) + 1; | ||
DoubleUnion u; | ||
u.u = absl::little_endian::Load64(valptr); | ||
return u.d; | ||
} | ||
|
||
} // namespace | ||
|
||
ScoreMap::~ScoreMap() { | ||
Clear(); | ||
return absl::bit_cast<double>(absl::little_endian::Load64(valptr)); | ||
} | ||
|
||
bool ScoreMap::AddOrUpdate(string_view field, double value) { | ||
void* AllocateScored(string_view field, double value) { | ||
size_t meta_offset = field.size() + 1; | ||
DoubleUnion u; | ||
u.d = value; | ||
|
||
// The layout is: | ||
// key, '\0', 8-byte double value | ||
|
@@ -50,23 +35,35 @@ bool ScoreMap::AddOrUpdate(string_view field, double value) { | |
memcpy(newkey, field.data(), field.size()); | ||
} | ||
|
||
absl::little_endian::Store64(newkey + meta_offset, u.u); | ||
absl::little_endian::Store64(newkey + meta_offset, absl::bit_cast<uint64_t>(value)); | ||
|
||
return newkey; | ||
} | ||
|
||
} // namespace | ||
|
||
ScoreMap::~ScoreMap() { | ||
Clear(); | ||
} | ||
|
||
pair<void*, bool> ScoreMap::AddOrUpdate(string_view field, double value) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. expose internal objects managed by ScoreMap so we could |
||
void* newkey = AllocateScored(field, value); | ||
|
||
// Replace the whole entry. | ||
sds prev_entry = (sds)AddOrReplaceObj(newkey, false); | ||
if (prev_entry) { | ||
ObjDelete(prev_entry, false); | ||
return false; | ||
return {newkey, false}; | ||
} | ||
|
||
return true; | ||
return {newkey, true}; | ||
} | ||
|
||
bool ScoreMap::AddOrSkip(std::string_view field, double value) { | ||
std::pair<void*, bool> ScoreMap::AddOrSkip(std::string_view field, double value) { | ||
void* obj = FindInternal(&field, 1); // 1 - string_view | ||
|
||
if (obj) | ||
return false; | ||
return {obj, false}; | ||
|
||
return AddOrUpdate(field, value); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i just moved this policy up to be able to use it in tests