caleb-brown/redict
1
0
Fork 0
mirror of https://codeberg.org/redict/redict.git synced 2025-01-22 16:18:28 -05:00
Code Issues Actions Packages Projects Releases Wiki Activity

improve quicklist insert head/tail while head/tail node is full. (#9113)

Browse Source 
In _quicklistInsert when `at_head` / `at_tail` is true, but `prev` / `next` is NULL,
the code was reaching the last if-else block at the bottom of the function,
and would have unnecessarily executed _quicklistSplitNode, instead of just creating a new node.
This was because the penultimate if-else was checking `node->next && full_next`.
but in fact it was unnecessary to check if `node->next` exists, if we're gonna create one anyway,
we only care that it's not full, or doesn't exist, so the condition could have been changed to `!node->next || full_next`.

Instead, this PR makes a small refactory to negate `full_next` to a more meaningful variable
`avail_next` that indicates that the next node is available for pushing additional elements or
not (this would be true only if it exists and it is non-full)
This commit is contained in:
cmemory 2021-08-02 13:30:25 +08:00 committed by GitHub
parent 8ea18fa8d2
commit 27a68a4d1b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 38 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
src/quicklist.c
View File

@ -843,7 +843,7 @@ REDIS_STATIC quicklistNode *_quicklistSplitNode(quicklistNode *node, int offset,
* the new value is inserted before 'entry'. */
REDIS_STATIC void _quicklistInsert(quicklist *quicklist, quicklistEntry *entry,
void *value, const size_t sz, int after) {
int full = 0, at_tail = 0, at_head = 0, full_next = 0, full_prev = 0;
int full = 0, at_tail = 0, at_head = 0, avail_next = 0, avail_prev = 0;
int fill = quicklist->fill;
quicklistNode *node = entry->node;
quicklistNode *new_node = NULL;
@ -869,18 +869,18 @@ REDIS_STATIC void _quicklistInsert(quicklist *quicklist, quicklistEntry *entry,
if (after && (entry->offset == node->count)) {
D("At Tail of current ziplist");
at_tail = 1;
if (!_quicklistNodeAllowInsert(node->next, fill, sz)) {
D("Next node is full too.");
full_next = 1;
if (_quicklistNodeAllowInsert(node->next, fill, sz)) {
D("Next node is available.");
avail_next = 1;
}
}
if (!after && (entry->offset == 0)) {
D("At Head");
at_head = 1;
if (!_quicklistNodeAllowInsert(node->prev, fill, sz)) {
D("Prev node is full too.");
full_prev = 1;
if (_quicklistNodeAllowInsert(node->prev, fill, sz)) {
D("Prev node is available.");
avail_prev = 1;
}
}
@ -904,7 +904,7 @@ REDIS_STATIC void _quicklistInsert(quicklist *quicklist, quicklistEntry *entry,
node->count++;
quicklistNodeUpdateSz(node);
quicklistRecompressOnly(quicklist, node);
} else if (full && at_tail && node->next && !full_next && after) {
} else if (full && at_tail && avail_next && after) {
/* If we are: at tail, next has free space, and inserting after:
* - insert entry at head of next node. */
D("Full and tail, but next isn't full; inserting next node head");
@ -914,7 +914,7 @@ REDIS_STATIC void _quicklistInsert(quicklist *quicklist, quicklistEntry *entry,
new_node->count++;
quicklistNodeUpdateSz(new_node);
quicklistRecompressOnly(quicklist, new_node);
} else if (full && at_head && node->prev && !full_prev && !after) {
} else if (full && at_head && avail_prev && !after) {
/* If we are: at head, previous has free space, and inserting before:
* - insert entry at tail of previous node. */
D("Full and head, but prev isn't full, inserting prev node tail");
@ -924,9 +924,9 @@ REDIS_STATIC void _quicklistInsert(quicklist *quicklist, quicklistEntry *entry,
new_node->count++;
quicklistNodeUpdateSz(new_node);
quicklistRecompressOnly(quicklist, new_node);
} else if (full && ((at_tail && node->next && full_next && after) ||
(at_head && node->prev && full_prev && !after))) {
/* If we are: full, and our prev/next is full, then:
} else if (full && ((at_tail && !avail_next && after) ||
(at_head && !avail_prev && !after))) {
/* If we are: full, and our prev/next has no available space, then:
* - create new node and attach to quicklist */
D("\tprovisioning new node...");
new_node = quicklistCreateNode();
@ -2008,6 +2008,32 @@ int quicklistTest(int argc, char *argv[], int accurate) {
quicklistRelease(ql);
}
TEST("insert head while head node is full") {
quicklist *ql = quicklistNew(4, options[_i]);
for (int i = 0; i < 10; i++)
quicklistPushTail(ql, genstr("hello", i), 6);
quicklistSetFill(ql, -1);
quicklistEntry entry;
quicklistIndex(ql, 0, &entry);
char buf[4096] = {0};
quicklistInsertBefore(ql, &entry, buf, 4096);
ql_verify(ql, 4, 11, 1, 2);
quicklistRelease(ql);
}
TEST("insert tail while tail node is full") {
quicklist *ql = quicklistNew(4, options[_i]);
for (int i = 0; i < 10; i++)
quicklistPushHead(ql, genstr("hello", i), 6);
quicklistSetFill(ql, -1);
quicklistEntry entry;
quicklistIndex(ql, -1, &entry);
char buf[4096] = {0};
quicklistInsertAfter(ql, &entry, buf, 4096);
ql_verify(ql, 4, 11, 2, 1);
quicklistRelease(ql);
}
TEST_DESC("insert once in elements while iterating at compress %d",
options[_i]) {
for (int f = 0; f < fill_count; f++) {