From 6c80e8b16e66b6de395c311ca672c545fa33078c Mon Sep 17 00:00:00 2001 From: Lior Sventitzky Date: Thu, 26 Feb 2026 19:25:39 +0000 Subject: [PATCH 1/6] started quicklist tracking mem Signed-off-by: Lior Sventitzky --- src/object.c | 157 +++++++++++++++++++++++++++++++++++++++++ src/quicklist.c | 181 ++++++++++++++++++++++++++++++++---------------- src/quicklist.h | 2 + 3 files changed, 282 insertions(+), 58 deletions(-) diff --git a/src/object.c b/src/object.c index 7db5b97996f..5474001f7af 100644 --- a/src/object.c +++ b/src/object.c @@ -1356,6 +1356,163 @@ size_t objectComputeSize(robj *key, robj *o, size_t sample_size, int dbid) { return asize; } +/* Same as objectComputeSize but uses tracked_size for quicklists instead of + * iterating through nodes. Used for testing tracked_size accuracy. */ +size_t objectComputeSizeWithTrackedSize(robj *key, robj *o, size_t sample_size, int dbid) { + size_t elesize = 0, samples = 0; + size_t asize = zmalloc_size((void *)o); + + if (o->type == OBJ_STRING) { + if (o->encoding == OBJ_ENCODING_RAW) { + asize += sdsAllocSize(objectGetVal(o)); + } else if (o->encoding != OBJ_ENCODING_INT && o->encoding != OBJ_ENCODING_EMBSTR) { + serverPanic("Unknown string encoding"); + } + } else if (o->type == OBJ_LIST) { + if (o->encoding == OBJ_ENCODING_QUICKLIST) { + quicklist *ql = objectGetVal(o); + asize += ql->tracked_size; + } else if (o->encoding == OBJ_ENCODING_LISTPACK) { + asize += zmalloc_size(objectGetVal(o)); + } else { + serverPanic("Unknown list encoding"); + } + } else if (o->type == OBJ_SET) { + if (o->encoding == OBJ_ENCODING_HASHTABLE) { + hashtable *ht = objectGetVal(o); + asize += hashtableMemUsage(ht); + + hashtableIterator iter; + hashtableInitIterator(&iter, ht, 0); + void *next; + while (hashtableNext(&iter, &next) && samples < sample_size) { + sds element = next; + elesize += sdsAllocSize(element); + samples++; + } + hashtableCleanupIterator(&iter); + if (samples) asize += (double)elesize / samples * hashtableSize(ht); + } else if (o->encoding == OBJ_ENCODING_INTSET) { + asize += zmalloc_size(objectGetVal(o)); + } else if (o->encoding == OBJ_ENCODING_LISTPACK) { + asize += zmalloc_size(objectGetVal(o)); + } else { + serverPanic("Unknown set encoding"); + } + } else if (o->type == OBJ_ZSET) { + if (o->encoding == OBJ_ENCODING_LISTPACK) { + asize += zmalloc_size(objectGetVal(o)); + } else if (o->encoding == OBJ_ENCODING_SKIPLIST) { + hashtable *ht = ((zset *)objectGetVal(o))->ht; + zskiplist *zsl = ((zset *)objectGetVal(o))->zsl; + zskiplistNode *zheader = zslGetHeader(zsl); + zskiplistNode *znode = zheader->level[0].forward; + asize += sizeof(zset) + zslGetAllocSize() + hashtableMemUsage(ht); + while (znode != NULL && samples < sample_size) { + elesize += zmalloc_size(znode); + samples++; + znode = znode->level[0].forward; + } + if (samples) asize += (double)elesize / samples * hashtableSize(ht); + } else { + serverPanic("Unknown sorted set encoding"); + } + } else if (o->type == OBJ_HASH) { + if (o->encoding == OBJ_ENCODING_LISTPACK) { + asize += zmalloc_size(objectGetVal(o)); + } else if (o->encoding == OBJ_ENCODING_HASHTABLE) { + hashtable *ht = objectGetVal(o); + hashtableIterator iter; + vset *volatile_fields = hashtableMetadata(ht); + hashtableInitIterator(&iter, ht, 0); + void *next; + + asize += hashtableMemUsage(ht); + while (hashtableNext(&iter, &next) && samples < sample_size) { + elesize += entryMemUsage(next); + samples++; + } + hashtableCleanupIterator(&iter); + if (samples) asize += (double)elesize / samples * hashtableSize(ht); + if (vsetIsValid(volatile_fields)) asize += vsetMemUsage(volatile_fields); + } else { + serverPanic("Unknown hash encoding"); + } + } else if (o->type == OBJ_STREAM) { + stream *s = objectGetVal(o); + asize += sizeof(*s) + raxAllocSize(s->rax); + + /* Now we have to add the listpacks. The last listpack is often non + * complete, so we estimate the size of the first N listpacks, and + * use the average to compute the size of the first N-1 listpacks, and + * finally add the real size of the last node. */ + raxIterator ri; + raxStart(&ri, s->rax); + raxSeek(&ri, "^", NULL, 0); + size_t lpsize = 0; + samples = 0; + while (samples < sample_size && raxNext(&ri)) { + unsigned char *lp = ri.data; + /* Use the allocated size, since we overprovision the node initially. */ + lpsize += zmalloc_size(lp); + samples++; + } + if (s->rax->numele <= samples) { + asize += lpsize; + } else { + if (samples) lpsize /= samples; /* Compute the average. */ + asize += lpsize * (s->rax->numele - 1); + /* No need to seek, we are already at the last element. */ + asize += zmalloc_size(ri.data); + } + raxStop(&ri); + + /* Consumer groups also have a non-trivial memory overhead if there + * are many consumers and many groups, let's count at least the + * overhead of the pending entries in the groups and consumers + * PELs. */ + if (s->cgroups) { + raxStart(&ri, s->cgroups); + raxSeek(&ri, "^", NULL, 0); + samples = 0; + elesize = 0; + while (samples < sample_size && raxNext(&ri)) { + streamCG *cg = ri.data; + elesize += sizeof(*cg); + elesize += raxAllocSize(cg->pel); + elesize += sizeof(streamNACK) * raxSize(cg->pel); + + /* For each consumer we also need to add the basic data + * structures and the PEL memory usage. */ + raxIterator cri; + raxStart(&cri, cg->consumers); + raxSeek(&cri, "^", NULL, 0); + size_t inner_samples = 0; + size_t inner_elesize = 0; + while (inner_samples < sample_size && raxNext(&cri)) { + streamConsumer *consumer = cri.data; + inner_elesize += sizeof(*consumer); + inner_elesize += sdslen(consumer->name); + inner_elesize += raxAllocSize(consumer->pel); + /* Don't count NACKs again, they are shared with the + * consumer group PEL. */ + inner_samples++; + } + raxStop(&cri); + if (inner_samples) elesize += (double)inner_elesize / inner_samples * raxSize(cg->consumers); + samples++; + } + raxStop(&ri); + if (samples) asize += (double)elesize / samples * raxSize(s->cgroups); + } + } else if (o->type == OBJ_MODULE) { + asize += moduleGetMemUsage(key, o, sample_size, dbid); + } else { + serverPanic("Unknown object type"); + } + return asize; +} + /* Release data obtained with getMemoryOverheadData(). */ void freeMemoryOverheadData(struct serverMemOverhead *mh) { zfree(mh->db); diff --git a/src/quicklist.c b/src/quicklist.c index 67ffe4e17bf..2d8d44bc169 100644 --- a/src/quicklist.c +++ b/src/quicklist.c @@ -96,7 +96,7 @@ quicklistBookmark *_quicklistBookmarkFindByName(quicklist *ql, const char *name) quicklistBookmark *_quicklistBookmarkFindByNode(quicklist *ql, quicklistNode *node); void _quicklistBookmarkDelete(quicklist *ql, quicklistBookmark *bm); -static quicklistNode *_quicklistSplitNode(quicklistNode *node, int offset, int after); +static quicklistNode *_quicklistSplitNode(quicklist *quicklist, quicklistNode *node, int offset, int after); static quicklistNode *_quicklistMergeNodes(quicklist *quicklist, quicklistNode *center); /* Simple way to give quicklistEntry structs default values with one call. */ @@ -118,6 +118,26 @@ static quicklistNode *_quicklistMergeNodes(quicklist *quicklist, quicklistNode * (iter)->zi = NULL; \ } while (0) +/* Helper macros for tracking memory allocations. + * Use quicklistTrackEntryBefore() before modifying node->entry, + * and quicklistTrackEntryAfter() after the modification. + * These macros handle NULL quicklist for standalone node operations (e.g., unit tests). */ +#define quicklistTrackEntryBefore(ql, node) \ + size_t _old_entry_sz = (node)->entry ? zmalloc_size((node)->entry) : 0 + +#define quicklistTrackEntryAfter(ql, node) \ + do { \ + if (ql) (ql)->tracked_size += zmalloc_size((node)->entry) - _old_entry_sz; \ + } while (0) + +/* Track node struct and entry deallocation. + * Use sizeof(quicklistNode) instead of zmalloc_size(node) to match + * the original objectComputeSize calculation. */ +#define quicklistTrackNodeFree(ql, node) \ + do { \ + if (ql) (ql)->tracked_size -= zmalloc_size((node)->entry) + sizeof(quicklistNode); \ + } while (0) + /* Create a new quicklist. * Free with quicklistRelease(). */ quicklist *quicklistCreate(void) { @@ -130,6 +150,7 @@ quicklist *quicklistCreate(void) { quicklist->compress = 0; quicklist->fill = -2; quicklist->bookmark_count = 0; + quicklist->tracked_size = sizeof(*quicklist); return quicklist; } @@ -209,7 +230,7 @@ void quicklistRelease(quicklist *quicklist) { /* Compress the listpack in 'node' and update encoding details. * Returns 1 if listpack compressed successfully. * Returns 0 if compression failed or if listpack too small to compress. */ -static int __quicklistCompressNode(quicklistNode *node) { +static int __quicklistCompressNode(quicklist *quicklist, quicklistNode *node) { node->attempted_compress = 1; if (node->dont_compress) return 0; @@ -231,23 +252,25 @@ static int __quicklistCompressNode(quicklistNode *node) { return 0; } lzf = zrealloc(lzf, sizeof(*lzf) + lzf->sz); + quicklistTrackEntryBefore(quicklist, node); zfree(node->entry); node->entry = (unsigned char *)lzf; + quicklistTrackEntryAfter(quicklist, node); node->encoding = QUICKLIST_NODE_ENCODING_LZF; return 1; } /* Compress only uncompressed nodes. */ -#define quicklistCompressNode(_node) \ +#define quicklistCompressNode(_ql, _node) \ do { \ if ((_node) && (_node)->encoding == QUICKLIST_NODE_ENCODING_RAW) { \ - __quicklistCompressNode((_node)); \ + __quicklistCompressNode((_ql), (_node)); \ } \ } while (0) /* Uncompress the listpack in 'node' and update encoding details. * Returns 1 on successful decode, 0 on failure to decode. */ -static int __quicklistDecompressNode(quicklistNode *node) { +static int __quicklistDecompressNode(quicklist *quicklist, quicklistNode *node) { node->attempted_compress = 0; node->recompress = 0; @@ -258,25 +281,27 @@ static int __quicklistDecompressNode(quicklistNode *node) { zfree(decompressed); return 0; } + quicklistTrackEntryBefore(quicklist, node); zfree(lzf); node->entry = decompressed; + quicklistTrackEntryAfter(quicklist, node); node->encoding = QUICKLIST_NODE_ENCODING_RAW; return 1; } /* Decompress only compressed nodes. */ -#define quicklistDecompressNode(_node) \ +#define quicklistDecompressNode(_ql, _node) \ do { \ if ((_node) && (_node)->encoding == QUICKLIST_NODE_ENCODING_LZF) { \ - __quicklistDecompressNode((_node)); \ + __quicklistDecompressNode((_ql), (_node)); \ } \ } while (0) /* Force node to not be immediately re-compressible */ -#define quicklistDecompressNodeForUse(_node) \ +#define quicklistDecompressNodeForUse(_ql, _node) \ do { \ if ((_node) && (_node)->encoding == QUICKLIST_NODE_ENCODING_LZF) { \ - __quicklistDecompressNode((_node)); \ + __quicklistDecompressNode((_ql), (_node)); \ (_node)->recompress = 1; \ } \ } while (0) @@ -296,7 +321,7 @@ size_t quicklistGetLzf(const quicklistNode *node, void **data) { * The only way to guarantee interior nodes get compressed is to iterate * to our "interior" compress depth then compress the next node we find. * If compress depth is larger than the entire list, we return immediately. */ -static void __quicklistCompress(const quicklist *quicklist, quicklistNode *node) { +static void __quicklistCompress(quicklist *quicklist, quicklistNode *node) { if (quicklist->len == 0) return; /* The head and tail should never be compressed (we should not attempt to recompress them) */ @@ -310,26 +335,26 @@ static void __quicklistCompress(const quicklist *quicklist, quicklistNode *node) /* Optimized cases for small depth counts */ if (quicklist->compress == 1) { quicklistNode *h = quicklist->head, *t = quicklist->tail; - quicklistDecompressNode(h); - quicklistDecompressNode(t); + quicklistDecompressNode(quicklist, h); + quicklistDecompressNode(quicklist, t); if (h != node && t != node) - quicklistCompressNode(node); + quicklistCompressNode(quicklist, node); return; } else if (quicklist->compress == 2) { quicklistNode *h = quicklist->head, *hn = h->next, *hnn = hn->next; quicklistNode *t = quicklist->tail, *tp = t->prev, *tpp = tp->prev; - quicklistDecompressNode(h); - quicklistDecompressNode(hn); - quicklistDecompressNode(t); - quicklistDecompressNode(tp); + quicklistDecompressNode(quicklist, h); + quicklistDecompressNode(quicklist, hn); + quicklistDecompressNode(quicklist, t); + quicklistDecompressNode(quicklist, tp); if (h != node && hn != node && t != node && tp != node) { - quicklistCompressNode(node); + quicklistCompressNode(quicklist, node); } if (hnn != t) { - quicklistCompressNode(hnn); + quicklistCompressNode(quicklist, hnn); } if (tpp != h) { - quicklistCompressNode(tpp); + quicklistCompressNode(quicklist, tpp); } return; } @@ -343,8 +368,8 @@ static void __quicklistCompress(const quicklist *quicklist, quicklistNode *node) int depth = 0; int in_depth = 0; while (depth++ < quicklist->compress) { - quicklistDecompressNode(forward); - quicklistDecompressNode(reverse); + quicklistDecompressNode(quicklist, forward); + quicklistDecompressNode(quicklist, reverse); if (forward == node || reverse == node) in_depth = 1; @@ -356,11 +381,11 @@ static void __quicklistCompress(const quicklist *quicklist, quicklistNode *node) reverse = reverse->prev; } - if (!in_depth) quicklistCompressNode(node); + if (!in_depth) quicklistCompressNode(quicklist, node); /* At this point, forward and reverse are one node beyond depth */ - quicklistCompressNode(forward); - quicklistCompressNode(reverse); + quicklistCompressNode(quicklist, forward); + quicklistCompressNode(quicklist, reverse); } /* This macro is used to compress a node. @@ -372,18 +397,18 @@ static void __quicklistCompress(const quicklist *quicklist, quicklistNode *node) * * If the 'recompress' flag of the node is false, we check whether the node is * within the range of compress depth before compressing it. */ -#define quicklistCompress(_ql, _node) \ - do { \ - if ((_node)->recompress) \ - quicklistCompressNode((_node)); \ - else \ - __quicklistCompress((_ql), (_node)); \ +#define quicklistCompress(_ql, _node) \ + do { \ + if ((_node)->recompress) \ + quicklistCompressNode((_ql), (_node)); \ + else \ + __quicklistCompress((_ql), (_node)); \ } while (0) /* If we previously used quicklistDecompressNodeForUse(), just recompress. */ -#define quicklistRecompressOnly(_node) \ - do { \ - if ((_node)->recompress) quicklistCompressNode((_node)); \ +#define quicklistRecompressOnly(_ql, _node) \ + do { \ + if ((_node)->recompress) quicklistCompressNode((_ql), (_node)); \ } while (0) /* Insert 'new_node' after 'old_node' if 'after' is 1. @@ -413,6 +438,11 @@ static void __quicklistInsertNode(quicklist *quicklist, quicklistNode *old_node, quicklist->head = quicklist->tail = new_node; } + /* Track memory: node struct + entry. + * Use sizeof(quicklistNode) to match original objectComputeSize calculation. */ + quicklist->tracked_size += sizeof(quicklistNode); + if (new_node->entry) quicklist->tracked_size += zmalloc_size(new_node->entry); + /* Update len first, so in __quicklistCompress we know exactly len */ quicklist->len++; @@ -553,7 +583,9 @@ int quicklistPushHead(quicklist *quicklist, void *value, size_t sz) { } if (likely(_quicklistNodeAllowInsert(quicklist->head, quicklist->fill, sz))) { + quicklistTrackEntryBefore(quicklist, quicklist->head); quicklist->head->entry = lpPrepend(quicklist->head->entry, value, sz); + quicklistTrackEntryAfter(quicklist, quicklist->head); quicklistNodeUpdateSz(quicklist->head); } else { quicklistNode *node = quicklistCreateNode(); @@ -579,7 +611,9 @@ int quicklistPushTail(quicklist *quicklist, void *value, size_t sz) { } if (likely(_quicklistNodeAllowInsert(quicklist->tail, quicklist->fill, sz))) { + quicklistTrackEntryBefore(quicklist, quicklist->tail); quicklist->tail->entry = lpAppend(quicklist->tail->entry, value, sz); + quicklistTrackEntryAfter(quicklist, quicklist->tail); quicklistNodeUpdateSz(quicklist->tail); } else { quicklistNode *node = quicklistCreateNode(); @@ -659,6 +693,7 @@ static void __quicklistDelNode(quicklist *quicklist, quicklistNode *node) { * now have compressed nodes needing to be decompressed. */ __quicklistCompress(quicklist, NULL); + quicklistTrackNodeFree(quicklist, node); zfree(node->entry); zfree(node); } @@ -678,7 +713,9 @@ static int quicklistDelIndex(quicklist *quicklist, quicklistNode *node, unsigned __quicklistDelNode(quicklist, node); return 1; } + quicklistTrackEntryBefore(quicklist, node); node->entry = lpDelete(node->entry, *p, p); + quicklistTrackEntryAfter(quicklist, node); node->count--; if (node->count == 0) { gone = 1; @@ -728,17 +765,22 @@ void quicklistReplaceEntry(quicklistIter *iter, quicklistEntry *entry, void *dat quicklist *quicklist = iter->quicklist; quicklistNode *node = entry->node; unsigned char *newentry; + size_t _old_entry_sz = 0; if (likely(!QL_NODE_IS_PLAIN(entry->node) && !isLargeElement(sz, quicklist->fill) && - (newentry = lpReplace(entry->node->entry, &entry->zi, data, sz)) != NULL)) { + (_old_entry_sz = zmalloc_size(entry->node->entry), + newentry = lpReplace(entry->node->entry, &entry->zi, data, sz)) != NULL)) { entry->node->entry = newentry; + quicklist->tracked_size += zmalloc_size(entry->node->entry) - _old_entry_sz; quicklistNodeUpdateSz(entry->node); /* quicklistNext() and quicklistGetIteratorEntryAtIdx() provide an uncompressed node */ quicklistCompress(quicklist, entry->node); } else if (QL_NODE_IS_PLAIN(entry->node)) { if (isLargeElement(sz, quicklist->fill)) { + quicklist->tracked_size -= zmalloc_size(entry->node->entry); zfree(entry->node->entry); entry->node->entry = zmalloc(sz); + quicklist->tracked_size += zmalloc_size(entry->node->entry); entry->node->sz = sz; memcpy(entry->node->entry, data, sz); quicklistCompress(quicklist, entry->node); @@ -752,7 +794,7 @@ void quicklistReplaceEntry(quicklistIter *iter, quicklistEntry *entry, void *dat /* If the entry is not at the tail, split the node at the entry's offset. */ if (entry->offset != node->count - 1 && entry->offset != -1) - split_node = _quicklistSplitNode(node, entry->offset, 1); + split_node = _quicklistSplitNode(quicklist, node, entry->offset, 1); /* Create a new node and insert it after the original node. * If the original node was split, insert the split node after the new node. */ @@ -818,17 +860,25 @@ int quicklistReplaceAtIndex(quicklist *quicklist, long index, void *data, size_t static quicklistNode *_quicklistListpackMerge(quicklist *quicklist, quicklistNode *a, quicklistNode *b) { D("Requested merge (a,b) (%u, %u)", a->count, b->count); - quicklistDecompressNode(a); - quicklistDecompressNode(b); + quicklistDecompressNode(quicklist, a); + quicklistDecompressNode(quicklist, b); + size_t a_entry_sz = zmalloc_size(a->entry); + size_t b_entry_sz = zmalloc_size(b->entry); if ((lpMerge(&a->entry, &b->entry))) { /* We merged listpacks! Now remove the unused quicklistNode. */ quicklistNode *keep = NULL, *nokeep = NULL; if (!a->entry) { nokeep = a; keep = b; + /* Update tracked size: a's entry was freed, b's entry was reallocated */ + quicklist->tracked_size -= a_entry_sz; + quicklist->tracked_size += zmalloc_size(b->entry) - b_entry_sz; } else if (!b->entry) { nokeep = b; keep = a; + /* Update tracked size: b's entry was freed, a's entry was reallocated */ + quicklist->tracked_size -= b_entry_sz; + quicklist->tracked_size += zmalloc_size(a->entry) - a_entry_sz; } keep->count = lpLength(keep->entry); quicklistNodeUpdateSz(keep); @@ -917,7 +967,7 @@ static quicklistNode *_quicklistMergeNodes(quicklist *quicklist, quicklistNode * * The input node keeps all elements not taken by the returned node. * * Returns newly created node or NULL if split not possible. */ -static quicklistNode *_quicklistSplitNode(quicklistNode *node, int offset, int after) { +static quicklistNode *_quicklistSplitNode(quicklist *quicklist, quicklistNode *node, int offset, int after) { size_t zl_sz = node->sz; quicklistNode *new_node = quicklistCreateNode(); @@ -937,7 +987,10 @@ static quicklistNode *_quicklistSplitNode(quicklistNode *node, int offset, int a D("After %d (%d); ranges: [%d, %d], [%d, %d]", after, offset, orig_start, orig_extent, new_start, new_extent); + /* Track entry change for original node (new_node not in list yet) */ + quicklistTrackEntryBefore(quicklist, node); node->entry = lpDeleteRange(node->entry, orig_start, orig_extent); + quicklistTrackEntryAfter(quicklist, node); node->count = lpLength(node->entry); quicklistNodeUpdateSz(node); @@ -1003,8 +1056,8 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v if (QL_NODE_IS_PLAIN(node) || (at_tail && after) || (at_head && !after)) { __quicklistInsertPlainNode(quicklist, node, value, sz, after); } else { - quicklistDecompressNodeForUse(node); - new_node = _quicklistSplitNode(node, entry->offset, after); + quicklistDecompressNodeForUse(quicklist, node); + new_node = _quicklistSplitNode(quicklist, node, entry->offset, after); quicklistNode *entry_node = __quicklistCreateNode(QUICKLIST_NODE_CONTAINER_PLAIN, value, sz); __quicklistInsertNode(quicklist, node, entry_node, after); __quicklistInsertNode(quicklist, entry_node, new_node, after); @@ -1016,40 +1069,48 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v /* Now determine where and how to insert the new element */ if (!full && after) { D("Not full, inserting after current position."); - quicklistDecompressNodeForUse(node); + quicklistDecompressNodeForUse(quicklist, node); + quicklistTrackEntryBefore(quicklist, node); node->entry = lpInsertString(node->entry, value, sz, entry->zi, LP_AFTER, NULL); + quicklistTrackEntryAfter(quicklist, node); node->count++; quicklistNodeUpdateSz(node); - quicklistRecompressOnly(node); + quicklistRecompressOnly(quicklist, node); } else if (!full && !after) { D("Not full, inserting before current position."); - quicklistDecompressNodeForUse(node); + quicklistDecompressNodeForUse(quicklist, node); + quicklistTrackEntryBefore(quicklist, node); node->entry = lpInsertString(node->entry, value, sz, entry->zi, LP_BEFORE, NULL); + quicklistTrackEntryAfter(quicklist, node); node->count++; quicklistNodeUpdateSz(node); - quicklistRecompressOnly(node); + quicklistRecompressOnly(quicklist, node); } 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"); new_node = node->next; - quicklistDecompressNodeForUse(new_node); + quicklistDecompressNodeForUse(quicklist, new_node); + quicklistTrackEntryBefore(quicklist, new_node); new_node->entry = lpPrepend(new_node->entry, value, sz); + quicklistTrackEntryAfter(quicklist, new_node); new_node->count++; quicklistNodeUpdateSz(new_node); - quicklistRecompressOnly(new_node); - quicklistRecompressOnly(node); + quicklistRecompressOnly(quicklist, new_node); + quicklistRecompressOnly(quicklist, node); } 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"); new_node = node->prev; - quicklistDecompressNodeForUse(new_node); + quicklistDecompressNodeForUse(quicklist, new_node); + quicklistTrackEntryBefore(quicklist, new_node); new_node->entry = lpAppend(new_node->entry, value, sz); + quicklistTrackEntryAfter(quicklist, new_node); new_node->count++; quicklistNodeUpdateSz(new_node); - quicklistRecompressOnly(new_node); - quicklistRecompressOnly(node); + quicklistRecompressOnly(quicklist, new_node); + quicklistRecompressOnly(quicklist, node); } 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 */ @@ -1063,12 +1124,14 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v /* else, node is full we need to split it. */ /* covers both after and !after cases */ D("\tsplitting node..."); - quicklistDecompressNodeForUse(node); - new_node = _quicklistSplitNode(node, entry->offset, after); + quicklistDecompressNodeForUse(quicklist, node); + new_node = _quicklistSplitNode(quicklist, node, entry->offset, after); + quicklistTrackEntryBefore(quicklist, new_node); if (after) new_node->entry = lpPrepend(new_node->entry, value, sz); else new_node->entry = lpAppend(new_node->entry, value, sz); + quicklistTrackEntryAfter(quicklist, new_node); new_node->count++; quicklistNodeUpdateSz(new_node); __quicklistInsertNode(quicklist, node, new_node, after); @@ -1157,13 +1220,15 @@ int quicklistDelRange(quicklist *quicklist, const long start, const long count) if (delete_entire_node || QL_NODE_IS_PLAIN(node)) { __quicklistDelNode(quicklist, node); } else { - quicklistDecompressNodeForUse(node); + quicklistDecompressNodeForUse(quicklist, node); + quicklistTrackEntryBefore(quicklist, node); node->entry = lpDeleteRange(node->entry, offset, del); + quicklistTrackEntryAfter(quicklist, node); quicklistNodeUpdateSz(node); node->count -= del; quicklist->count -= del; quicklistDeleteIfEmpty(quicklist, node); - if (node) quicklistRecompressOnly(node); + if (node) quicklistRecompressOnly(quicklist, node); } extent -= del; @@ -1310,7 +1375,7 @@ int quicklistNext(quicklistIter *iter, quicklistEntry *entry) { int plain = QL_NODE_IS_PLAIN(iter->current); if (!iter->zi) { /* If !zi, use current index. */ - quicklistDecompressNodeForUse(iter->current); + quicklistDecompressNodeForUse(iter->quicklist, iter->current); if (unlikely(plain)) iter->zi = iter->current->entry; else @@ -1595,7 +1660,7 @@ void quicklistRepr(unsigned char *ql, int full) { node->attempted_compress); if (full) { - quicklistDecompressNode(node); + quicklistDecompressNode(quicklist, node); if (node->container == QUICKLIST_NODE_CONTAINER_PACKED) { printf("{ listpack:\n"); lpRepr(node->entry); @@ -1605,7 +1670,7 @@ void quicklistRepr(unsigned char *ql, int full) { printf("{ entry : %s }\n", node->entry); } printf("}\n"); - quicklistRecompressOnly(node); + quicklistRecompressOnly(quicklist, node); } node = node->next; } diff --git a/src/quicklist.h b/src/quicklist.h index 4411f823b0a..5933dda0760 100644 --- a/src/quicklist.h +++ b/src/quicklist.h @@ -109,6 +109,7 @@ typedef struct quicklist { quicklistNode *tail; unsigned long count; /* total count of all entries in all listpacks */ unsigned long len; /* number of quicklistNodes */ + size_t tracked_size; /* total memory allocated for this quicklist */ signed int fill : QL_FILL_BITS; /* fill factor for individual nodes */ unsigned int compress : QL_COMP_BITS; /* depth of end nodes not to compress;0=off */ unsigned int bookmark_count : QL_BM_BITS; @@ -185,6 +186,7 @@ int quicklistPopCustom(quicklist *quicklist, void *(*saver)(unsigned char *data, size_t sz)); int quicklistPop(quicklist *quicklist, int where, unsigned char **data, size_t *sz, long long *slong); unsigned long quicklistCount(const quicklist *ql); +size_t quicklistGetAllocatedSize(const quicklist *ql); int quicklistCompare(quicklistEntry *entry, unsigned char *p2, const size_t p2_len); size_t quicklistGetLzf(const quicklistNode *node, void **data); void quicklistNodeLimit(int fill, size_t *size, unsigned int *count); From 24d915c8f2f17da9ad229ca30134f30faa5ab05e Mon Sep 17 00:00:00 2001 From: Lior Sventitzky Date: Sun, 1 Mar 2026 17:29:46 +0000 Subject: [PATCH 2/6] changed to capture size not with zmalloc_size Signed-off-by: Lior Sventitzky --- src/listpack.c | 2 ++ src/listpack_malloc.h | 10 ++++-- src/quicklist.c | 78 +++++++++++++++++++++++++++---------------- src/quicklist.h | 1 + 4 files changed, 61 insertions(+), 30 deletions(-) diff --git a/src/listpack.c b/src/listpack.c index 7d2febd01d6..a1695b51e68 100644 --- a/src/listpack.c +++ b/src/listpack.c @@ -41,6 +41,8 @@ #include "listpack.h" #include "listpack_malloc.h" + +__thread size_t lp_last_alloc_size = 0; #include "serverassert.h" #include "util.h" #include "config.h" diff --git a/src/listpack_malloc.h b/src/listpack_malloc.h index a75bd318177..9e8e5d386d0 100644 --- a/src/listpack_malloc.h +++ b/src/listpack_malloc.h @@ -39,11 +39,17 @@ #ifndef LISTPACK_ALLOC_H #define LISTPACK_ALLOC_H #include "zmalloc.h" + +/* Thread-local that captures the usable allocation size from the last + * lp_malloc / lp_realloc call. zrealloc_usable already computes this + * internally; we just stop discarding it. */ +extern __thread size_t lp_last_alloc_size; + /* We use zmalloc_usable/zrealloc_usable instead of zmalloc/zrealloc * to ensure the safe invocation of 'zmalloc_usable_size(). * See comment in zmalloc_usable_size(). */ -#define lp_malloc(sz) zmalloc_usable(sz, NULL) -#define lp_realloc(ptr, sz) zrealloc_usable(ptr, sz, NULL) +#define lp_malloc(sz) zmalloc_usable(sz, &lp_last_alloc_size) +#define lp_realloc(ptr, sz) zrealloc_usable(ptr, sz, &lp_last_alloc_size) #define lp_free zfree #define lp_malloc_size zmalloc_usable_size #endif diff --git a/src/quicklist.c b/src/quicklist.c index 2d8d44bc169..dc39c3c56fc 100644 --- a/src/quicklist.c +++ b/src/quicklist.c @@ -35,6 +35,7 @@ #include "zmalloc.h" #include "config.h" #include "listpack.h" +#include "listpack_malloc.h" #include "util.h" /* for ll2string */ #include "lzf.h" #include "serverassert.h" @@ -121,21 +122,26 @@ static quicklistNode *_quicklistMergeNodes(quicklist *quicklist, quicklistNode * /* Helper macros for tracking memory allocations. * Use quicklistTrackEntryBefore() before modifying node->entry, * and quicklistTrackEntryAfter() after the modification. - * These macros handle NULL quicklist for standalone node operations (e.g., unit tests). */ + * These macros handle NULL quicklist for standalone node operations (e.g., unit tests). + * + * The "After" macro reads lp_last_alloc_size which was set by the most recent + * lp_malloc / lp_realloc inside the listpack operation that just modified + * node->entry. This avoids an expensive zmalloc_size() call. */ #define quicklistTrackEntryBefore(ql, node) \ - size_t _old_entry_sz = (node)->entry ? zmalloc_size((node)->entry) : 0 + size_t _old_entry_sz = (node)->entry_alloc_sz -#define quicklistTrackEntryAfter(ql, node) \ - do { \ - if (ql) (ql)->tracked_size += zmalloc_size((node)->entry) - _old_entry_sz; \ +#define quicklistTrackEntryAfter(ql, node) \ + do { \ + (node)->entry_alloc_sz = lp_last_alloc_size; \ + if (ql) (ql)->tracked_size += (node)->entry_alloc_sz - _old_entry_sz; \ } while (0) /* Track node struct and entry deallocation. * Use sizeof(quicklistNode) instead of zmalloc_size(node) to match * the original objectComputeSize calculation. */ -#define quicklistTrackNodeFree(ql, node) \ - do { \ - if (ql) (ql)->tracked_size -= zmalloc_size((node)->entry) + sizeof(quicklistNode); \ +#define quicklistTrackNodeFree(ql, node) \ + do { \ + if (ql) (ql)->tracked_size -= (node)->entry_alloc_sz + sizeof(quicklistNode); \ } while (0) /* Create a new quicklist. @@ -192,6 +198,7 @@ static quicklistNode *quicklistCreateNode(void) { node->entry = NULL; node->count = 0; node->sz = 0; + node->entry_alloc_sz = 0; node->next = node->prev = NULL; node->encoding = QUICKLIST_NODE_ENCODING_RAW; node->container = QUICKLIST_NODE_CONTAINER_PACKED; @@ -251,11 +258,13 @@ static int __quicklistCompressNode(quicklist *quicklist, quicklistNode *node) { zfree(lzf); return 0; } - lzf = zrealloc(lzf, sizeof(*lzf) + lzf->sz); - quicklistTrackEntryBefore(quicklist, node); + size_t new_entry_alloc_sz; + lzf = zrealloc_usable(lzf, sizeof(*lzf) + lzf->sz, &new_entry_alloc_sz); + size_t old_entry_alloc_sz = node->entry_alloc_sz; zfree(node->entry); node->entry = (unsigned char *)lzf; - quicklistTrackEntryAfter(quicklist, node); + node->entry_alloc_sz = new_entry_alloc_sz; + if (quicklist) quicklist->tracked_size += new_entry_alloc_sz - old_entry_alloc_sz; node->encoding = QUICKLIST_NODE_ENCODING_LZF; return 1; } @@ -274,17 +283,19 @@ static int __quicklistDecompressNode(quicklist *quicklist, quicklistNode *node) node->attempted_compress = 0; node->recompress = 0; - void *decompressed = zmalloc(node->sz); + size_t new_entry_alloc_sz; + void *decompressed = zmalloc_usable(node->sz, &new_entry_alloc_sz); quicklistLZF *lzf = (quicklistLZF *)node->entry; if (lzf_decompress(lzf->compressed, lzf->sz, decompressed, node->sz) == 0) { /* Someone requested decompress, but we can't decompress. Not good. */ zfree(decompressed); return 0; } - quicklistTrackEntryBefore(quicklist, node); + size_t old_entry_alloc_sz = node->entry_alloc_sz; zfree(lzf); node->entry = decompressed; - quicklistTrackEntryAfter(quicklist, node); + node->entry_alloc_sz = new_entry_alloc_sz; + if (quicklist) quicklist->tracked_size += new_entry_alloc_sz - old_entry_alloc_sz; node->encoding = QUICKLIST_NODE_ENCODING_RAW; return 1; } @@ -441,7 +452,7 @@ static void __quicklistInsertNode(quicklist *quicklist, quicklistNode *old_node, /* Track memory: node struct + entry. * Use sizeof(quicklistNode) to match original objectComputeSize calculation. */ quicklist->tracked_size += sizeof(quicklistNode); - if (new_node->entry) quicklist->tracked_size += zmalloc_size(new_node->entry); + if (new_node->entry) quicklist->tracked_size += new_node->entry_alloc_sz; /* Update len first, so in __quicklistCompress we know exactly len */ quicklist->len++; @@ -553,10 +564,11 @@ static quicklistNode *__quicklistCreateNode(int container, void *value, size_t s quicklistNode *new_node = quicklistCreateNode(); new_node->container = container; if (container == QUICKLIST_NODE_CONTAINER_PLAIN) { - new_node->entry = zmalloc(sz); + new_node->entry = zmalloc_usable(sz, &new_node->entry_alloc_sz); memcpy(new_node->entry, value, sz); } else { new_node->entry = lpPrepend(lpNew(0), value, sz); + new_node->entry_alloc_sz = lp_last_alloc_size; } new_node->sz = sz; new_node->count++; @@ -590,6 +602,7 @@ int quicklistPushHead(quicklist *quicklist, void *value, size_t sz) { } else { quicklistNode *node = quicklistCreateNode(); node->entry = lpPrepend(lpNew(0), value, sz); + node->entry_alloc_sz = lp_last_alloc_size; quicklistNodeUpdateSz(node); _quicklistInsertNodeBefore(quicklist, quicklist->head, node); @@ -618,6 +631,7 @@ int quicklistPushTail(quicklist *quicklist, void *value, size_t sz) { } else { quicklistNode *node = quicklistCreateNode(); node->entry = lpAppend(lpNew(0), value, sz); + node->entry_alloc_sz = lp_last_alloc_size; quicklistNodeUpdateSz(node); _quicklistInsertNodeAfter(quicklist, quicklist->tail, node); @@ -636,6 +650,7 @@ void quicklistAppendListpack(quicklist *quicklist, unsigned char *zl) { node->entry = zl; node->count = lpLength(node->entry); node->sz = lpBytes(zl); + node->entry_alloc_sz = zmalloc_size(zl); _quicklistInsertNodeAfter(quicklist, quicklist->tail, node); quicklist->count += node->count; @@ -651,6 +666,7 @@ void quicklistAppendPlainNode(quicklist *quicklist, unsigned char *data, size_t node->entry = data; node->count = 1; node->sz = sz; + node->entry_alloc_sz = zmalloc_size(data); node->container = QUICKLIST_NODE_CONTAINER_PLAIN; _quicklistInsertNodeAfter(quicklist, quicklist->tail, node); @@ -768,19 +784,20 @@ void quicklistReplaceEntry(quicklistIter *iter, quicklistEntry *entry, void *dat size_t _old_entry_sz = 0; if (likely(!QL_NODE_IS_PLAIN(entry->node) && !isLargeElement(sz, quicklist->fill) && - (_old_entry_sz = zmalloc_size(entry->node->entry), + (_old_entry_sz = entry->node->entry_alloc_sz, newentry = lpReplace(entry->node->entry, &entry->zi, data, sz)) != NULL)) { entry->node->entry = newentry; - quicklist->tracked_size += zmalloc_size(entry->node->entry) - _old_entry_sz; + entry->node->entry_alloc_sz = lp_last_alloc_size; + quicklist->tracked_size += entry->node->entry_alloc_sz - _old_entry_sz; quicklistNodeUpdateSz(entry->node); /* quicklistNext() and quicklistGetIteratorEntryAtIdx() provide an uncompressed node */ quicklistCompress(quicklist, entry->node); } else if (QL_NODE_IS_PLAIN(entry->node)) { if (isLargeElement(sz, quicklist->fill)) { - quicklist->tracked_size -= zmalloc_size(entry->node->entry); + quicklist->tracked_size -= entry->node->entry_alloc_sz; zfree(entry->node->entry); - entry->node->entry = zmalloc(sz); - quicklist->tracked_size += zmalloc_size(entry->node->entry); + entry->node->entry = zmalloc_usable(sz, &entry->node->entry_alloc_sz); + quicklist->tracked_size += entry->node->entry_alloc_sz; entry->node->sz = sz; memcpy(entry->node->entry, data, sz); quicklistCompress(quicklist, entry->node); @@ -862,8 +879,8 @@ static quicklistNode *_quicklistListpackMerge(quicklist *quicklist, quicklistNod quicklistDecompressNode(quicklist, a); quicklistDecompressNode(quicklist, b); - size_t a_entry_sz = zmalloc_size(a->entry); - size_t b_entry_sz = zmalloc_size(b->entry); + size_t a_entry_sz = a->entry_alloc_sz; + size_t b_entry_sz = b->entry_alloc_sz; if ((lpMerge(&a->entry, &b->entry))) { /* We merged listpacks! Now remove the unused quicklistNode. */ quicklistNode *keep = NULL, *nokeep = NULL; @@ -871,14 +888,16 @@ static quicklistNode *_quicklistListpackMerge(quicklist *quicklist, quicklistNod nokeep = a; keep = b; /* Update tracked size: a's entry was freed, b's entry was reallocated */ + keep->entry_alloc_sz = lp_last_alloc_size; quicklist->tracked_size -= a_entry_sz; - quicklist->tracked_size += zmalloc_size(b->entry) - b_entry_sz; + quicklist->tracked_size += keep->entry_alloc_sz - b_entry_sz; } else if (!b->entry) { nokeep = b; keep = a; /* Update tracked size: b's entry was freed, a's entry was reallocated */ + keep->entry_alloc_sz = lp_last_alloc_size; quicklist->tracked_size -= b_entry_sz; - quicklist->tracked_size += zmalloc_size(a->entry) - a_entry_sz; + quicklist->tracked_size += keep->entry_alloc_sz - a_entry_sz; } keep->count = lpLength(keep->entry); quicklistNodeUpdateSz(keep); @@ -971,7 +990,7 @@ static quicklistNode *_quicklistSplitNode(quicklist *quicklist, quicklistNode *n size_t zl_sz = node->sz; quicklistNode *new_node = quicklistCreateNode(); - new_node->entry = zmalloc(zl_sz); + new_node->entry = zmalloc_usable(zl_sz, &new_node->entry_alloc_sz); /* Copy original listpack so we can split it */ memcpy(new_node->entry, node->entry, zl_sz); @@ -995,6 +1014,7 @@ static quicklistNode *_quicklistSplitNode(quicklist *quicklist, quicklistNode *n quicklistNodeUpdateSz(node); new_node->entry = lpDeleteRange(new_node->entry, new_start, new_extent); + new_node->entry_alloc_sz = lp_last_alloc_size; new_node->count = lpLength(new_node->entry); quicklistNodeUpdateSz(new_node); @@ -1022,6 +1042,7 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v } new_node = quicklistCreateNode(); new_node->entry = lpPrepend(lpNew(0), value, sz); + new_node->entry_alloc_sz = lp_last_alloc_size; __quicklistInsertNode(quicklist, NULL, new_node, after); new_node->count++; quicklist->count++; @@ -1117,6 +1138,7 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v D("\tprovisioning new node..."); new_node = quicklistCreateNode(); new_node->entry = lpPrepend(lpNew(0), value, sz); + new_node->entry_alloc_sz = lp_last_alloc_size; new_node->count++; quicklistNodeUpdateSz(new_node); __quicklistInsertNode(quicklist, node, new_node, after); @@ -1451,10 +1473,10 @@ quicklist *quicklistDup(quicklist *orig) { if (current->encoding == QUICKLIST_NODE_ENCODING_LZF) { quicklistLZF *lzf = (quicklistLZF *)current->entry; size_t lzf_sz = sizeof(*lzf) + lzf->sz; - node->entry = zmalloc(lzf_sz); + node->entry = zmalloc_usable(lzf_sz, &node->entry_alloc_sz); memcpy(node->entry, current->entry, lzf_sz); } else if (current->encoding == QUICKLIST_NODE_ENCODING_RAW) { - node->entry = zmalloc(current->sz); + node->entry = zmalloc_usable(current->sz, &node->entry_alloc_sz); memcpy(node->entry, current->entry, current->sz); } diff --git a/src/quicklist.h b/src/quicklist.h index 5933dda0760..30a0c8fd96a 100644 --- a/src/quicklist.h +++ b/src/quicklist.h @@ -49,6 +49,7 @@ typedef struct quicklistNode { struct quicklistNode *next; unsigned char *entry; size_t sz; /* entry size in bytes */ + size_t entry_alloc_sz; /* usable allocation size of entry */ unsigned int count : 16; /* count of items in listpack */ unsigned int encoding : 2; /* RAW==1 or LZF==2 */ unsigned int container : 2; /* PLAIN==1 or PACKED==2 */ From 0ebae0fb2f425c489140f14cced2e220a8c8850d Mon Sep 17 00:00:00 2001 From: Lior Sventitzky Date: Sun, 1 Mar 2026 20:25:08 +0000 Subject: [PATCH 3/6] seperated to diff file, bug fixes Signed-off-by: Lior Sventitzky --- src/quicklist.c | 89 ++-- src/unit/test_quicklist_tracking.c | 773 +++++++++++++++++++++++++++++ 2 files changed, 813 insertions(+), 49 deletions(-) create mode 100644 src/unit/test_quicklist_tracking.c diff --git a/src/quicklist.c b/src/quicklist.c index dc39c3c56fc..b9230f56dd5 100644 --- a/src/quicklist.c +++ b/src/quicklist.c @@ -119,30 +119,11 @@ static quicklistNode *_quicklistMergeNodes(quicklist *quicklist, quicklistNode * (iter)->zi = NULL; \ } while (0) -/* Helper macros for tracking memory allocations. - * Use quicklistTrackEntryBefore() before modifying node->entry, - * and quicklistTrackEntryAfter() after the modification. - * These macros handle NULL quicklist for standalone node operations (e.g., unit tests). - * - * The "After" macro reads lp_last_alloc_size which was set by the most recent - * lp_malloc / lp_realloc inside the listpack operation that just modified - * node->entry. This avoids an expensive zmalloc_size() call. */ -#define quicklistTrackEntryBefore(ql, node) \ - size_t _old_entry_sz = (node)->entry_alloc_sz - -#define quicklistTrackEntryAfter(ql, node) \ - do { \ - (node)->entry_alloc_sz = lp_last_alloc_size; \ - if (ql) (ql)->tracked_size += (node)->entry_alloc_sz - _old_entry_sz; \ - } while (0) - -/* Track node struct and entry deallocation. - * Use sizeof(quicklistNode) instead of zmalloc_size(node) to match - * the original objectComputeSize calculation. */ -#define quicklistTrackNodeFree(ql, node) \ - do { \ - if (ql) (ql)->tracked_size -= (node)->entry_alloc_sz + sizeof(quicklistNode); \ - } while (0) +/* After a listpack operation that reallocates node->entry (e.g. lpPrepend, + * lpAppend, lpDelete, lpInsertString, lpReplace, lpDeleteRange), the + * lp_last_alloc_size thread-local holds the usable allocation size of the + * new entry buffer. We use it to update node->entry_alloc_sz and + * quicklist->tracked_size without calling zmalloc_size(). */ /* Create a new quicklist. * Free with quicklistRelease(). */ @@ -595,9 +576,10 @@ int quicklistPushHead(quicklist *quicklist, void *value, size_t sz) { } if (likely(_quicklistNodeAllowInsert(quicklist->head, quicklist->fill, sz))) { - quicklistTrackEntryBefore(quicklist, quicklist->head); + size_t old_sz = quicklist->head->entry_alloc_sz; quicklist->head->entry = lpPrepend(quicklist->head->entry, value, sz); - quicklistTrackEntryAfter(quicklist, quicklist->head); + quicklist->head->entry_alloc_sz = lp_last_alloc_size; + quicklist->tracked_size += quicklist->head->entry_alloc_sz - old_sz; quicklistNodeUpdateSz(quicklist->head); } else { quicklistNode *node = quicklistCreateNode(); @@ -624,9 +606,10 @@ int quicklistPushTail(quicklist *quicklist, void *value, size_t sz) { } if (likely(_quicklistNodeAllowInsert(quicklist->tail, quicklist->fill, sz))) { - quicklistTrackEntryBefore(quicklist, quicklist->tail); + size_t old_sz = quicklist->tail->entry_alloc_sz; quicklist->tail->entry = lpAppend(quicklist->tail->entry, value, sz); - quicklistTrackEntryAfter(quicklist, quicklist->tail); + quicklist->tail->entry_alloc_sz = lp_last_alloc_size; + quicklist->tracked_size += quicklist->tail->entry_alloc_sz - old_sz; quicklistNodeUpdateSz(quicklist->tail); } else { quicklistNode *node = quicklistCreateNode(); @@ -709,7 +692,7 @@ static void __quicklistDelNode(quicklist *quicklist, quicklistNode *node) { * now have compressed nodes needing to be decompressed. */ __quicklistCompress(quicklist, NULL); - quicklistTrackNodeFree(quicklist, node); + quicklist->tracked_size -= node->entry_alloc_sz + sizeof(quicklistNode); zfree(node->entry); zfree(node); } @@ -729,9 +712,10 @@ static int quicklistDelIndex(quicklist *quicklist, quicklistNode *node, unsigned __quicklistDelNode(quicklist, node); return 1; } - quicklistTrackEntryBefore(quicklist, node); + size_t old_sz = node->entry_alloc_sz; node->entry = lpDelete(node->entry, *p, p); - quicklistTrackEntryAfter(quicklist, node); + node->entry_alloc_sz = lp_last_alloc_size; + quicklist->tracked_size += node->entry_alloc_sz - old_sz; node->count--; if (node->count == 0) { gone = 1; @@ -887,18 +871,19 @@ static quicklistNode *_quicklistListpackMerge(quicklist *quicklist, quicklistNod if (!a->entry) { nokeep = a; keep = b; - /* Update tracked size: a's entry was freed, b's entry was reallocated */ + /* Update tracked size: b's entry was reallocated by lpMerge */ keep->entry_alloc_sz = lp_last_alloc_size; - quicklist->tracked_size -= a_entry_sz; quicklist->tracked_size += keep->entry_alloc_sz - b_entry_sz; } else if (!b->entry) { nokeep = b; keep = a; - /* Update tracked size: b's entry was freed, a's entry was reallocated */ + /* Update tracked size: a's entry was reallocated by lpMerge */ keep->entry_alloc_sz = lp_last_alloc_size; - quicklist->tracked_size -= b_entry_sz; quicklist->tracked_size += keep->entry_alloc_sz - a_entry_sz; } + /* nokeep->entry was freed by lpMerge (entry is NULL), but we leave + * nokeep->entry_alloc_sz unchanged so __quicklistDelNode subtracts + * the correct amount (entry_alloc_sz + sizeof(node)). */ keep->count = lpLength(keep->entry); quicklistNodeUpdateSz(keep); keep->recompress = 0; /* Prevent 'keep' from being recompressed if @@ -1007,9 +992,10 @@ static quicklistNode *_quicklistSplitNode(quicklist *quicklist, quicklistNode *n D("After %d (%d); ranges: [%d, %d], [%d, %d]", after, offset, orig_start, orig_extent, new_start, new_extent); /* Track entry change for original node (new_node not in list yet) */ - quicklistTrackEntryBefore(quicklist, node); + size_t old_sz = node->entry_alloc_sz; node->entry = lpDeleteRange(node->entry, orig_start, orig_extent); - quicklistTrackEntryAfter(quicklist, node); + node->entry_alloc_sz = lp_last_alloc_size; + if (quicklist) quicklist->tracked_size += node->entry_alloc_sz - old_sz; node->count = lpLength(node->entry); quicklistNodeUpdateSz(node); @@ -1091,18 +1077,20 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v if (!full && after) { D("Not full, inserting after current position."); quicklistDecompressNodeForUse(quicklist, node); - quicklistTrackEntryBefore(quicklist, node); + size_t old_sz = node->entry_alloc_sz; node->entry = lpInsertString(node->entry, value, sz, entry->zi, LP_AFTER, NULL); - quicklistTrackEntryAfter(quicklist, node); + node->entry_alloc_sz = lp_last_alloc_size; + quicklist->tracked_size += node->entry_alloc_sz - old_sz; node->count++; quicklistNodeUpdateSz(node); quicklistRecompressOnly(quicklist, node); } else if (!full && !after) { D("Not full, inserting before current position."); quicklistDecompressNodeForUse(quicklist, node); - quicklistTrackEntryBefore(quicklist, node); + size_t old_sz = node->entry_alloc_sz; node->entry = lpInsertString(node->entry, value, sz, entry->zi, LP_BEFORE, NULL); - quicklistTrackEntryAfter(quicklist, node); + node->entry_alloc_sz = lp_last_alloc_size; + quicklist->tracked_size += node->entry_alloc_sz - old_sz; node->count++; quicklistNodeUpdateSz(node); quicklistRecompressOnly(quicklist, node); @@ -1112,9 +1100,10 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v D("Full and tail, but next isn't full; inserting next node head"); new_node = node->next; quicklistDecompressNodeForUse(quicklist, new_node); - quicklistTrackEntryBefore(quicklist, new_node); + size_t old_sz = new_node->entry_alloc_sz; new_node->entry = lpPrepend(new_node->entry, value, sz); - quicklistTrackEntryAfter(quicklist, new_node); + new_node->entry_alloc_sz = lp_last_alloc_size; + quicklist->tracked_size += new_node->entry_alloc_sz - old_sz; new_node->count++; quicklistNodeUpdateSz(new_node); quicklistRecompressOnly(quicklist, new_node); @@ -1125,9 +1114,10 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v D("Full and head, but prev isn't full, inserting prev node tail"); new_node = node->prev; quicklistDecompressNodeForUse(quicklist, new_node); - quicklistTrackEntryBefore(quicklist, new_node); + size_t old_sz = new_node->entry_alloc_sz; new_node->entry = lpAppend(new_node->entry, value, sz); - quicklistTrackEntryAfter(quicklist, new_node); + new_node->entry_alloc_sz = lp_last_alloc_size; + quicklist->tracked_size += new_node->entry_alloc_sz - old_sz; new_node->count++; quicklistNodeUpdateSz(new_node); quicklistRecompressOnly(quicklist, new_node); @@ -1148,12 +1138,12 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v D("\tsplitting node..."); quicklistDecompressNodeForUse(quicklist, node); new_node = _quicklistSplitNode(quicklist, node, entry->offset, after); - quicklistTrackEntryBefore(quicklist, new_node); if (after) new_node->entry = lpPrepend(new_node->entry, value, sz); else new_node->entry = lpAppend(new_node->entry, value, sz); - quicklistTrackEntryAfter(quicklist, new_node); + new_node->entry_alloc_sz = lp_last_alloc_size; + /* Don't track delta here; __quicklistInsertNode below adds the full entry_alloc_sz */ new_node->count++; quicklistNodeUpdateSz(new_node); __quicklistInsertNode(quicklist, node, new_node, after); @@ -1243,9 +1233,10 @@ int quicklistDelRange(quicklist *quicklist, const long start, const long count) __quicklistDelNode(quicklist, node); } else { quicklistDecompressNodeForUse(quicklist, node); - quicklistTrackEntryBefore(quicklist, node); + size_t old_sz = node->entry_alloc_sz; node->entry = lpDeleteRange(node->entry, offset, del); - quicklistTrackEntryAfter(quicklist, node); + node->entry_alloc_sz = lp_last_alloc_size; + quicklist->tracked_size += node->entry_alloc_sz - old_sz; quicklistNodeUpdateSz(node); node->count -= del; quicklist->count -= del; diff --git a/src/unit/test_quicklist_tracking.c b/src/unit/test_quicklist_tracking.c new file mode 100644 index 00000000000..8ad4216a1fb --- /dev/null +++ b/src/unit/test_quicklist_tracking.c @@ -0,0 +1,773 @@ +#include +#include +#include + +#include "../server.h" +#include "../quicklist.h" +#include "test_help.h" + +/* These are defined in object.c but not declared in any header. */ +extern size_t objectComputeSize(robj *key, robj *o, size_t sample_size, int dbid); +extern size_t objectComputeSizeWithTrackedSize(robj *key, robj *o, size_t sample_size, int dbid); + +/* Helper: assert tracked_size matches ground-truth objectComputeSize. + * objectComputeSize crashes on empty quicklists (do-while on NULL head), + * so we guard against that by only comparing when the list is non-empty. + * For empty lists, we just verify tracked_size equals sizeof(quicklist). */ +#define ASSERT_TRACKED_SIZE_CORRECT(list) do { \ + quicklist *_ql = objectGetVal(list); \ + if (_ql->len > 0) { \ + size_t _computed = objectComputeSize(NULL, (list), LLONG_MAX, 0); \ + size_t _tracked = objectComputeSizeWithTrackedSize(NULL, (list), LLONG_MAX, 0); \ + TEST_ASSERT(_computed == _tracked); \ + } else { \ + TEST_ASSERT(_ql->tracked_size == sizeof(quicklist)); \ + } \ +} while (0) + +int test_quicklist_tracked_size(int argc, char **argv, int flags) { + UNUSED(argc); + UNUSED(argv); + UNUSED(flags); + + /* Note: quicklistSetPackedThreshold() is the public API for packed_threshold. */ + + /* ======================================================== + * 1. Basic pushHead / pushTail into existing nodes + * Exercises: lpPrepend/lpAppend on existing node + lp_last_alloc_size capture + * ======================================================== */ + { + robj *list = createQuicklistObject(3, 0); /* fill=3 (count-based), no compression */ + quicklist *ql = objectGetVal(list); + + for (int i = 0; i < 50; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "element_%d", i); + if (i % 2 == 0) + quicklistPushTail(ql, buf, strlen(buf)); + else + quicklistPushHead(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + decrRefCount(list); + } + + /* ======================================================== + * 2. pushHead / pushTail that create new nodes + * Exercises: new node creation with lpPrepend(lpNew(0), ...) + * fill=1 forces a new node per element + * ======================================================== */ + { + robj *list = createQuicklistObject(1, 0); + quicklist *ql = objectGetVal(list); + + for (int i = 0; i < 20; i++) { + char buf[64]; + snprintf(buf, sizeof(buf), "single_node_element_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + TEST_ASSERT(ql->len == 20); /* one element per node */ + ASSERT_TRACKED_SIZE_CORRECT(list); + decrRefCount(list); + } + + /* ======================================================== + * 3. Pop from head and tail + * Exercises: quicklistDelIndex -> lpDelete + tracked_size delta + * Also exercises node deletion when node becomes empty + * ======================================================== */ + { + robj *list = createQuicklistObject(3, 0); + quicklist *ql = objectGetVal(list); + + for (int i = 0; i < 30; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "pop_test_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Pop from head */ + for (int i = 0; i < 8; i++) { + unsigned char *data; + size_t sz; + long long val; + quicklistPop(ql, QUICKLIST_HEAD, &data, &sz, &val); + if (data) zfree(data); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Pop from tail */ + for (int i = 0; i < 8; i++) { + unsigned char *data; + size_t sz; + long long val; + quicklistPop(ql, QUICKLIST_TAIL, &data, &sz, &val); + if (data) zfree(data); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); + } + + /* ======================================================== + * 4. Delete entire node (pop all elements from a node) + * Exercises: __quicklistDelNode -> tracked_size -= entry_alloc_sz + sizeof(quicklistNode) + * ======================================================== */ + { + robj *list = createQuicklistObject(2, 0); /* 2 elements per node */ + quicklist *ql = objectGetVal(list); + + for (int i = 0; i < 10; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "del_node_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + TEST_ASSERT(ql->len == 5); /* 10 elements / 2 per node */ + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Pop all elements, forcing node deletions */ + while (quicklistCount(ql) > 0) { + unsigned char *data; + size_t sz; + long long val; + quicklistPop(ql, QUICKLIST_HEAD, &data, &sz, &val); + if (data) zfree(data); + ASSERT_TRACKED_SIZE_CORRECT(list); + } + decrRefCount(list); + } + + /* ======================================================== + * 5. Compression and decompression + * Exercises: __quicklistCompressNode (zrealloc_usable for LZF) + * __quicklistDecompressNode (zmalloc_usable for decompressed buf) + * ======================================================== */ + { + robj *list = createQuicklistObject(-2, 1); /* size-based fill, compress depth=1 */ + quicklist *ql = objectGetVal(list); + + /* Add enough elements to create multiple nodes so interior ones get compressed */ + for (int i = 0; i < 500; i++) { + char buf[64]; + snprintf(buf, sizeof(buf), "compress_test_value_%d_padding_data", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + TEST_ASSERT(ql->len >= 3); /* need at least 3 nodes for compression to kick in */ + /* Verify at least one interior node is actually compressed */ + int has_compressed = 0; + for (quicklistNode *n = ql->head; n; n = n->next) { + if (quicklistNodeIsCompressed(n)) { has_compressed = 1; break; } + } + TEST_ASSERT(has_compressed); + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Pop from head — forces decompression of the new head's neighbor */ + for (int i = 0; i < 50; i++) { + unsigned char *data; + size_t sz; + long long val; + quicklistPop(ql, QUICKLIST_HEAD, &data, &sz, &val); + if (data) zfree(data); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Pop from tail — forces decompression of the new tail's neighbor */ + for (int i = 0; i < 50; i++) { + unsigned char *data; + size_t sz; + long long val; + quicklistPop(ql, QUICKLIST_TAIL, &data, &sz, &val); + if (data) zfree(data); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Push to head — may decompress/recompress nodes */ + for (int i = 0; i < 50; i++) { + char buf[64]; + snprintf(buf, sizeof(buf), "after_compress_push_%d", i); + quicklistPushHead(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); + } + + /* ======================================================== + * 6. InsertBefore / InsertAfter into non-full node + * Exercises: lpInsertString + lp_last_alloc_size in _quicklistInsert + * ======================================================== */ + { + robj *list = createQuicklistObject(-2, 0); + quicklist *ql = objectGetVal(list); + + for (int i = 0; i < 20; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "insert_test_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Insert after the 5th element */ + quicklistEntry entry; + quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 5, &entry); + TEST_ASSERT(iter != NULL); + quicklistInsertAfter(iter, &entry, "INSERTED_AFTER", 14); + quicklistReleaseIterator(iter); + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Insert before the 10th element */ + iter = quicklistGetIteratorEntryAtIdx(ql, 10, &entry); + TEST_ASSERT(iter != NULL); + quicklistInsertBefore(iter, &entry, "INSERTED_BEFORE", 15); + quicklistReleaseIterator(iter); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); + } + + /* ======================================================== + * 7. Insert that triggers node split + * Exercises: _quicklistSplitNode (zmalloc_usable + lpDeleteRange) + * + insert into split node + * ======================================================== */ + { + robj *list = createQuicklistObject(3, 0); /* fill=3, nodes fill up fast */ + quicklist *ql = objectGetVal(list); + + for (int i = 0; i < 12; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "split_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + TEST_ASSERT(ql->len == 4); /* 12 / 3 = 4 nodes */ + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Insert in the middle of a full node — forces split */ + quicklistEntry entry; + quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 4, &entry); + TEST_ASSERT(iter != NULL); + quicklistInsertAfter(iter, &entry, "SPLIT_INSERT", 12); + quicklistReleaseIterator(iter); + TEST_ASSERT(ql->len > 4); /* split must have created a new node */ + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); + } + + /* ======================================================== + * 8a. Insert into next neighbor (full node, at_tail, avail_next) + * Exercises: lpPrepend into next node in _quicklistInsert + * ======================================================== */ + { + robj *list = createQuicklistObject(4, 0); /* fill=4 */ + quicklist *ql = objectGetVal(list); + + /* Create two nodes: first full (4 items), second with 2 items */ + for (int i = 0; i < 6; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "neighbor_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + TEST_ASSERT(ql->len == 2); + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Insert at end of first node (at_tail && after && avail_next) + * First node is full (4), second has room (2 < 4). */ + quicklistEntry entry; + quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 3, &entry); + TEST_ASSERT(iter != NULL); + quicklistInsertAfter(iter, &entry, "NEIGHBOR_NEXT", 13); + quicklistReleaseIterator(iter); + TEST_ASSERT(ql->len == 2); /* should NOT have created a new node */ + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); + } + + /* ======================================================== + * 8b. Insert into prev neighbor (full node, at_head, avail_prev) + * Exercises: lpAppend into prev node in _quicklistInsert + * ======================================================== */ + { + robj *list = createQuicklistObject(4, 0); /* fill=4 */ + quicklist *ql = objectGetVal(list); + + /* Create: node0(4 full) + node1(4 full), then pop one from head + * so node0 has room. */ + for (int i = 0; i < 8; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "avprev_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + TEST_ASSERT(ql->len == 2); + /* Pop one from head to make room in node0 (3 items) while node1 stays full (4) */ + unsigned char *data; size_t sz; long long val; + quicklistPop(ql, QUICKLIST_HEAD, &data, &sz, &val); + if (data) zfree(data); + TEST_ASSERT(ql->head->count == 3); + TEST_ASSERT(ql->tail->count == 4); + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Insert before head of node1 (at_head && !after && avail_prev) + * node1 is full (4), its prev (node0) has room (3 < 4). */ + quicklistEntry entry; + quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 3, &entry); + TEST_ASSERT(iter != NULL); + quicklistInsertBefore(iter, &entry, "NEIGHBOR_PREV", 13); + quicklistReleaseIterator(iter); + TEST_ASSERT(ql->len == 2); /* should NOT have created a new node */ + TEST_ASSERT(ql->head->count == 4); /* prev absorbed the insert */ + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); + } + + /* ======================================================== + * 9. ReplaceEntry — listpack path (lpReplace) + * Exercises: entry_alloc_sz read + lp_last_alloc_size after lpReplace + * ======================================================== */ + { + robj *list = createQuicklistObject(-2, 0); + quicklist *ql = objectGetVal(list); + + for (int i = 0; i < 20; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "replace_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Replace with a smaller value */ + quicklistEntry entry; + quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 5, &entry); + TEST_ASSERT(iter != NULL); + quicklistReplaceEntry(iter, &entry, "S", 1); + quicklistReleaseIterator(iter); + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Replace with a larger value */ + iter = quicklistGetIteratorEntryAtIdx(ql, 10, &entry); + TEST_ASSERT(iter != NULL); + char big_replace[200]; + memset(big_replace, 'X', sizeof(big_replace)); + quicklistReplaceEntry(iter, &entry, big_replace, sizeof(big_replace)); + quicklistReleaseIterator(iter); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); + } + + /* ======================================================== + * 10. ReplaceEntry — plain node path (zmalloc_usable for new entry) + * Exercises: plain node replace with large element + * ======================================================== */ + { + quicklistSetPackedThreshold(64); + + robj *list = createQuicklistObject(-2, 0); + quicklist *ql = objectGetVal(list); + + /* Create a plain node with a large element */ + char large[128]; + memset(large, 'A', sizeof(large)); + quicklistPushTail(ql, large, sizeof(large)); + /* Add some normal elements too */ + for (int i = 0; i < 10; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "mixed_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Replace the plain node with another large element */ + quicklistEntry entry; + quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 0, &entry); + TEST_ASSERT(iter != NULL); + TEST_ASSERT(QL_NODE_IS_PLAIN(entry.node)); + char new_large[256]; + memset(new_large, 'B', sizeof(new_large)); + quicklistReplaceEntry(iter, &entry, new_large, sizeof(new_large)); + quicklistReleaseIterator(iter); + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistSetPackedThreshold(0); + decrRefCount(list); + } + + /* ======================================================== + * 11. DelRange — partial and full node deletion + * Exercises: lpDeleteRange + tracked_size delta, and __quicklistDelNode + * ======================================================== */ + { + robj *list = createQuicklistObject(5, 0); + quicklist *ql = objectGetVal(list); + + for (int i = 0; i < 30; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "delrange_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Delete a partial range from the middle (doesn't delete whole nodes) */ + quicklistDelRange(ql, 3, 2); + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Delete a range spanning multiple nodes */ + quicklistDelRange(ql, 2, 15); + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Delete from head */ + quicklistDelRange(ql, 0, 3); + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Delete from tail (negative index) */ + quicklistDelRange(ql, -3, 3); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); + } + + /* ======================================================== + * 12. Rotate (move tail to head) + * Exercises: quicklistPushHead + quicklistDelIndex on tail + * ======================================================== */ + { + robj *list = createQuicklistObject(3, 0); + quicklist *ql = objectGetVal(list); + + for (int i = 0; i < 15; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "rotate_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + for (int i = 0; i < 10; i++) { + quicklistRotate(ql); + ASSERT_TRACKED_SIZE_CORRECT(list); + } + + decrRefCount(list); + } + + /* ======================================================== + * 13. Dup (deep copy) + * Exercises: quicklistDup -> zmalloc_usable for each node entry + * ======================================================== */ + { + robj *list = createQuicklistObject(-2, 0); + quicklist *ql = objectGetVal(list); + + for (int i = 0; i < 100; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "dup_test_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklist *copy = quicklistDup(ql); + robj *copy_list = createObject(OBJ_LIST, copy); + copy_list->encoding = OBJ_ENCODING_QUICKLIST; + ASSERT_TRACKED_SIZE_CORRECT(copy_list); + + /* Mutate the copy and verify tracking stays correct */ + for (int i = 0; i < 20; i++) { + unsigned char *data; + size_t sz; + long long val; + quicklistPop(copy, QUICKLIST_TAIL, &data, &sz, &val); + if (data) zfree(data); + } + ASSERT_TRACKED_SIZE_CORRECT(copy_list); + + decrRefCount(copy_list); + decrRefCount(list); + } + + /* ======================================================== + * 14. Dup with compression + * Exercises: quicklistDup with LZF-compressed nodes + * ======================================================== */ + { + robj *list = createQuicklistObject(-2, 1); /* compress depth=1 */ + quicklist *ql = objectGetVal(list); + + for (int i = 0; i < 500; i++) { + char buf[64]; + snprintf(buf, sizeof(buf), "dup_compress_%d_padding_data_here", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklist *copy = quicklistDup(ql); + robj *copy_list = createObject(OBJ_LIST, copy); + copy_list->encoding = OBJ_ENCODING_QUICKLIST; + ASSERT_TRACKED_SIZE_CORRECT(copy_list); + + decrRefCount(copy_list); + decrRefCount(list); + } + + /* ======================================================== + * 15. Node merge via _quicklistListpackMerge + * Exercises: lpMerge + lp_last_alloc_size for merged node + * _quicklistMergeNodes is called after insert-triggered splits. + * Use fill=4: create 3 full nodes (12 elements), then delete + * elements from 2 adjacent nodes so they become small enough to + * merge when a split triggers _quicklistMergeNodes. + * ======================================================== */ + { + robj *list = createQuicklistObject(4, 0); /* fill=4, count-based */ + quicklist *ql = objectGetVal(list); + + /* Create 3 full nodes of 4 elements each = 12 elements */ + for (int i = 0; i < 12; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "merge_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + TEST_ASSERT(ql->len == 3); + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Delete 3 elements from the second node to make it small (count=1) */ + quicklistDelRange(ql, 4, 3); + /* Now: node0 has 4, node1 has 1, node2 has 4. Total 3 nodes, 9 elements. */ + TEST_ASSERT(ql->len == 3); + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Delete 3 elements from the third node to make it small (count=1) */ + quicklistDelRange(ql, 5, 3); + /* Now: node0 has 4, node1 has 1, node2 has 1. Total 3 nodes, 6 elements. */ + TEST_ASSERT(ql->len == 3); + ASSERT_TRACKED_SIZE_CORRECT(list); + + unsigned long nodes_before = ql->len; + + /* Insert into the middle of the full first node to trigger a split. + * After split, _quicklistMergeNodes will try to merge neighbors. + * node1 (count=1) and node2 (count=1) can merge since 1+1 <= fill=4. */ + quicklistEntry entry; + quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 2, &entry); + TEST_ASSERT(iter != NULL); + quicklistInsertAfter(iter, &entry, "MERGE_TRIGGER", 13); + quicklistReleaseIterator(iter); + + /* The split created a new node, but merges should have reduced the count */ + TEST_ASSERT(ql->len <= nodes_before + 1); /* split adds 1, merges may reduce */ + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); + } + + /* ======================================================== + * 16. Plain node insert + * Exercises: __quicklistCreateNode with PLAIN container, zmalloc_usable + * ======================================================== */ + { + quicklistSetPackedThreshold(64); + + robj *list = createQuicklistObject(-2, 0); + quicklist *ql = objectGetVal(list); + + /* Push small elements first */ + for (int i = 0; i < 5; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "small_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Push large elements that become plain nodes */ + for (int i = 0; i < 5; i++) { + char large[128]; + memset(large, 'P' + i, sizeof(large)); + quicklistPushTail(ql, large, sizeof(large)); + ASSERT_TRACKED_SIZE_CORRECT(list); + } + + /* Pop plain nodes */ + for (int i = 0; i < 3; i++) { + unsigned char *data; + size_t sz; + long long val; + quicklistPop(ql, QUICKLIST_TAIL, &data, &sz, &val); + if (data) zfree(data); + ASSERT_TRACKED_SIZE_CORRECT(list); + } + + quicklistSetPackedThreshold(0); + decrRefCount(list); + } + + /* ======================================================== + * 17. Insert into empty list (no reference node) + * Exercises: _quicklistInsert with node==NULL path + * ======================================================== */ + { + robj *list = createQuicklistObject(-2, 0); + quicklist *ql = objectGetVal(list); + + quicklistEntry entry; + memset(&entry, 0, sizeof(entry)); + entry.quicklist = ql; + quicklistIter *iter = quicklistGetIterator(ql, AL_START_HEAD); + quicklistInsertAfter(iter, &entry, "first_ever", 10); + quicklistReleaseIterator(iter); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); + } + + /* ======================================================== + * 18. Compression with operations interleaved + * Exercises: repeated compress/decompress cycles via mixed ops + * ======================================================== */ + { + robj *list = createQuicklistObject(-2, 2); /* compress depth=2 */ + quicklist *ql = objectGetVal(list); + + /* Build a large list so inner nodes get compressed */ + for (int i = 0; i < 1000; i++) { + char buf[64]; + snprintf(buf, sizeof(buf), "interleaved_%d_extra_padding_here", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Interleave pushes and pops from both ends */ + for (int i = 0; i < 100; i++) { + char buf[64]; + snprintf(buf, sizeof(buf), "interleaved_new_%d", i); + if (i % 2 == 0) + quicklistPushHead(ql, buf, strlen(buf)); + else + quicklistPushTail(ql, buf, strlen(buf)); + + unsigned char *data; + size_t sz; + long long val; + if (i % 3 == 0) + quicklistPop(ql, QUICKLIST_HEAD, &data, &sz, &val); + else + quicklistPop(ql, QUICKLIST_TAIL, &data, &sz, &val); + if (data) zfree(data); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Replace elements in the middle (forces decompress + recompress) */ + for (int i = 0; i < 10; i++) { + quicklistEntry entry; + quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, (long)quicklistCount(ql) / 2, &entry); + if (iter) { + char buf[100]; + memset(buf, 'R', sizeof(buf)); + quicklistReplaceEntry(iter, &entry, buf, sizeof(buf)); + quicklistReleaseIterator(iter); + } + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); + } + + /* ======================================================== + * 19. AppendListpack / AppendPlainNode (RDB load paths) + * Exercises: zmalloc_size at load time for entry_alloc_sz + * ======================================================== */ + { + robj *list = createQuicklistObject(-2, 0); + quicklist *ql = objectGetVal(list); + + /* Simulate RDB load: create a listpack externally and append */ + unsigned char *lp = lpNew(0); + for (int i = 0; i < 10; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "rdb_lp_%d", i); + lp = lpAppend(lp, (unsigned char *)buf, strlen(buf)); + } + quicklistAppendListpack(ql, lp); + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Append another listpack */ + unsigned char *lp2 = lpNew(0); + for (int i = 0; i < 5; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "rdb_lp2_%d", i); + lp2 = lpAppend(lp2, (unsigned char *)buf, strlen(buf)); + } + quicklistAppendListpack(ql, lp2); + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Append a plain node */ + size_t plain_sz = 256; + unsigned char *plain_data = zmalloc(plain_sz); + memset(plain_data, 'Z', plain_sz); + quicklistAppendPlainNode(ql, plain_data, plain_sz); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); + } + + /* ======================================================== + * 20. DelEntry via iterator + * Exercises: quicklistDelEntry -> quicklistDelIndex for each entry + * ======================================================== */ + { + robj *list = createQuicklistObject(5, 0); + quicklist *ql = objectGetVal(list); + + for (int i = 0; i < 25; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "delentry_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Delete every other element via iterator */ + quicklistIter *iter = quicklistGetIterator(ql, AL_START_HEAD); + quicklistEntry entry; + int count = 0; + while (quicklistNext(iter, &entry)) { + if (count % 2 == 0) { + quicklistDelEntry(iter, &entry); + } + count++; + } + quicklistReleaseIterator(iter); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); + } + + /* ======================================================== + * 21. ReplaceAtIndex + * Exercises: quicklistReplaceAtIndex -> quicklistReplaceEntry + * ======================================================== */ + { + robj *list = createQuicklistObject(-2, 0); + quicklist *ql = objectGetVal(list); + + for (int i = 0; i < 30; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "replaceatidx_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Replace at various indices */ + quicklistReplaceAtIndex(ql, 0, "HEAD_REPLACED", 13); + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistReplaceAtIndex(ql, 15, "MID_REPLACED", 12); + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistReplaceAtIndex(ql, 29, "TAIL_REPLACED", 13); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); + } + + return 0; +} From 2c2c669ea5f928700eb5d228fae7a03e806d43d0 Mon Sep 17 00:00:00 2001 From: Lior Sventitzky Date: Sun, 1 Mar 2026 21:43:23 +0000 Subject: [PATCH 4/6] fixed to gtest update Signed-off-by: Lior Sventitzky --- src/quicklist.c | 20 +- src/unit/test_quicklist_tracking.c | 773 --------------------------- src/unit/test_quicklist_tracking.cpp | 660 +++++++++++++++++++++++ 3 files changed, 670 insertions(+), 783 deletions(-) delete mode 100644 src/unit/test_quicklist_tracking.c create mode 100644 src/unit/test_quicklist_tracking.cpp diff --git a/src/quicklist.c b/src/quicklist.c index b9230f56dd5..a89545379b6 100644 --- a/src/quicklist.c +++ b/src/quicklist.c @@ -389,17 +389,17 @@ static void __quicklistCompress(quicklist *quicklist, quicklistNode *node) { * * If the 'recompress' flag of the node is false, we check whether the node is * within the range of compress depth before compressing it. */ -#define quicklistCompress(_ql, _node) \ - do { \ - if ((_node)->recompress) \ - quicklistCompressNode((_ql), (_node)); \ - else \ - __quicklistCompress((_ql), (_node)); \ +#define quicklistCompress(_ql, _node) \ + do { \ + if ((_node)->recompress) \ + quicklistCompressNode((_ql), (_node)); \ + else \ + __quicklistCompress((_ql), (_node)); \ } while (0) /* If we previously used quicklistDecompressNodeForUse(), just recompress. */ -#define quicklistRecompressOnly(_ql, _node) \ - do { \ +#define quicklistRecompressOnly(_ql, _node) \ + do { \ if ((_node)->recompress) quicklistCompressNode((_ql), (_node)); \ } while (0) @@ -1782,9 +1782,9 @@ quicklistNode *testOnlyQuicklistCreateNodeWithValue(int container, void *value, } int testOnlyQuicklistCompressNode(quicklistNode *node) { - return __quicklistCompressNode(node); + return __quicklistCompressNode(NULL, node); } int testOnlyQuicklistDecompressNode(quicklistNode *node) { - return __quicklistDecompressNode(node); + return __quicklistDecompressNode(NULL, node); } diff --git a/src/unit/test_quicklist_tracking.c b/src/unit/test_quicklist_tracking.c deleted file mode 100644 index 8ad4216a1fb..00000000000 --- a/src/unit/test_quicklist_tracking.c +++ /dev/null @@ -1,773 +0,0 @@ -#include -#include -#include - -#include "../server.h" -#include "../quicklist.h" -#include "test_help.h" - -/* These are defined in object.c but not declared in any header. */ -extern size_t objectComputeSize(robj *key, robj *o, size_t sample_size, int dbid); -extern size_t objectComputeSizeWithTrackedSize(robj *key, robj *o, size_t sample_size, int dbid); - -/* Helper: assert tracked_size matches ground-truth objectComputeSize. - * objectComputeSize crashes on empty quicklists (do-while on NULL head), - * so we guard against that by only comparing when the list is non-empty. - * For empty lists, we just verify tracked_size equals sizeof(quicklist). */ -#define ASSERT_TRACKED_SIZE_CORRECT(list) do { \ - quicklist *_ql = objectGetVal(list); \ - if (_ql->len > 0) { \ - size_t _computed = objectComputeSize(NULL, (list), LLONG_MAX, 0); \ - size_t _tracked = objectComputeSizeWithTrackedSize(NULL, (list), LLONG_MAX, 0); \ - TEST_ASSERT(_computed == _tracked); \ - } else { \ - TEST_ASSERT(_ql->tracked_size == sizeof(quicklist)); \ - } \ -} while (0) - -int test_quicklist_tracked_size(int argc, char **argv, int flags) { - UNUSED(argc); - UNUSED(argv); - UNUSED(flags); - - /* Note: quicklistSetPackedThreshold() is the public API for packed_threshold. */ - - /* ======================================================== - * 1. Basic pushHead / pushTail into existing nodes - * Exercises: lpPrepend/lpAppend on existing node + lp_last_alloc_size capture - * ======================================================== */ - { - robj *list = createQuicklistObject(3, 0); /* fill=3 (count-based), no compression */ - quicklist *ql = objectGetVal(list); - - for (int i = 0; i < 50; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "element_%d", i); - if (i % 2 == 0) - quicklistPushTail(ql, buf, strlen(buf)); - else - quicklistPushHead(ql, buf, strlen(buf)); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - decrRefCount(list); - } - - /* ======================================================== - * 2. pushHead / pushTail that create new nodes - * Exercises: new node creation with lpPrepend(lpNew(0), ...) - * fill=1 forces a new node per element - * ======================================================== */ - { - robj *list = createQuicklistObject(1, 0); - quicklist *ql = objectGetVal(list); - - for (int i = 0; i < 20; i++) { - char buf[64]; - snprintf(buf, sizeof(buf), "single_node_element_%d", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - TEST_ASSERT(ql->len == 20); /* one element per node */ - ASSERT_TRACKED_SIZE_CORRECT(list); - decrRefCount(list); - } - - /* ======================================================== - * 3. Pop from head and tail - * Exercises: quicklistDelIndex -> lpDelete + tracked_size delta - * Also exercises node deletion when node becomes empty - * ======================================================== */ - { - robj *list = createQuicklistObject(3, 0); - quicklist *ql = objectGetVal(list); - - for (int i = 0; i < 30; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "pop_test_%d", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Pop from head */ - for (int i = 0; i < 8; i++) { - unsigned char *data; - size_t sz; - long long val; - quicklistPop(ql, QUICKLIST_HEAD, &data, &sz, &val); - if (data) zfree(data); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Pop from tail */ - for (int i = 0; i < 8; i++) { - unsigned char *data; - size_t sz; - long long val; - quicklistPop(ql, QUICKLIST_TAIL, &data, &sz, &val); - if (data) zfree(data); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - decrRefCount(list); - } - - /* ======================================================== - * 4. Delete entire node (pop all elements from a node) - * Exercises: __quicklistDelNode -> tracked_size -= entry_alloc_sz + sizeof(quicklistNode) - * ======================================================== */ - { - robj *list = createQuicklistObject(2, 0); /* 2 elements per node */ - quicklist *ql = objectGetVal(list); - - for (int i = 0; i < 10; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "del_node_%d", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - TEST_ASSERT(ql->len == 5); /* 10 elements / 2 per node */ - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Pop all elements, forcing node deletions */ - while (quicklistCount(ql) > 0) { - unsigned char *data; - size_t sz; - long long val; - quicklistPop(ql, QUICKLIST_HEAD, &data, &sz, &val); - if (data) zfree(data); - ASSERT_TRACKED_SIZE_CORRECT(list); - } - decrRefCount(list); - } - - /* ======================================================== - * 5. Compression and decompression - * Exercises: __quicklistCompressNode (zrealloc_usable for LZF) - * __quicklistDecompressNode (zmalloc_usable for decompressed buf) - * ======================================================== */ - { - robj *list = createQuicklistObject(-2, 1); /* size-based fill, compress depth=1 */ - quicklist *ql = objectGetVal(list); - - /* Add enough elements to create multiple nodes so interior ones get compressed */ - for (int i = 0; i < 500; i++) { - char buf[64]; - snprintf(buf, sizeof(buf), "compress_test_value_%d_padding_data", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - TEST_ASSERT(ql->len >= 3); /* need at least 3 nodes for compression to kick in */ - /* Verify at least one interior node is actually compressed */ - int has_compressed = 0; - for (quicklistNode *n = ql->head; n; n = n->next) { - if (quicklistNodeIsCompressed(n)) { has_compressed = 1; break; } - } - TEST_ASSERT(has_compressed); - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Pop from head — forces decompression of the new head's neighbor */ - for (int i = 0; i < 50; i++) { - unsigned char *data; - size_t sz; - long long val; - quicklistPop(ql, QUICKLIST_HEAD, &data, &sz, &val); - if (data) zfree(data); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Pop from tail — forces decompression of the new tail's neighbor */ - for (int i = 0; i < 50; i++) { - unsigned char *data; - size_t sz; - long long val; - quicklistPop(ql, QUICKLIST_TAIL, &data, &sz, &val); - if (data) zfree(data); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Push to head — may decompress/recompress nodes */ - for (int i = 0; i < 50; i++) { - char buf[64]; - snprintf(buf, sizeof(buf), "after_compress_push_%d", i); - quicklistPushHead(ql, buf, strlen(buf)); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - decrRefCount(list); - } - - /* ======================================================== - * 6. InsertBefore / InsertAfter into non-full node - * Exercises: lpInsertString + lp_last_alloc_size in _quicklistInsert - * ======================================================== */ - { - robj *list = createQuicklistObject(-2, 0); - quicklist *ql = objectGetVal(list); - - for (int i = 0; i < 20; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "insert_test_%d", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Insert after the 5th element */ - quicklistEntry entry; - quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 5, &entry); - TEST_ASSERT(iter != NULL); - quicklistInsertAfter(iter, &entry, "INSERTED_AFTER", 14); - quicklistReleaseIterator(iter); - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Insert before the 10th element */ - iter = quicklistGetIteratorEntryAtIdx(ql, 10, &entry); - TEST_ASSERT(iter != NULL); - quicklistInsertBefore(iter, &entry, "INSERTED_BEFORE", 15); - quicklistReleaseIterator(iter); - ASSERT_TRACKED_SIZE_CORRECT(list); - - decrRefCount(list); - } - - /* ======================================================== - * 7. Insert that triggers node split - * Exercises: _quicklistSplitNode (zmalloc_usable + lpDeleteRange) - * + insert into split node - * ======================================================== */ - { - robj *list = createQuicklistObject(3, 0); /* fill=3, nodes fill up fast */ - quicklist *ql = objectGetVal(list); - - for (int i = 0; i < 12; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "split_%d", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - TEST_ASSERT(ql->len == 4); /* 12 / 3 = 4 nodes */ - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Insert in the middle of a full node — forces split */ - quicklistEntry entry; - quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 4, &entry); - TEST_ASSERT(iter != NULL); - quicklistInsertAfter(iter, &entry, "SPLIT_INSERT", 12); - quicklistReleaseIterator(iter); - TEST_ASSERT(ql->len > 4); /* split must have created a new node */ - ASSERT_TRACKED_SIZE_CORRECT(list); - - decrRefCount(list); - } - - /* ======================================================== - * 8a. Insert into next neighbor (full node, at_tail, avail_next) - * Exercises: lpPrepend into next node in _quicklistInsert - * ======================================================== */ - { - robj *list = createQuicklistObject(4, 0); /* fill=4 */ - quicklist *ql = objectGetVal(list); - - /* Create two nodes: first full (4 items), second with 2 items */ - for (int i = 0; i < 6; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "neighbor_%d", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - TEST_ASSERT(ql->len == 2); - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Insert at end of first node (at_tail && after && avail_next) - * First node is full (4), second has room (2 < 4). */ - quicklistEntry entry; - quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 3, &entry); - TEST_ASSERT(iter != NULL); - quicklistInsertAfter(iter, &entry, "NEIGHBOR_NEXT", 13); - quicklistReleaseIterator(iter); - TEST_ASSERT(ql->len == 2); /* should NOT have created a new node */ - ASSERT_TRACKED_SIZE_CORRECT(list); - - decrRefCount(list); - } - - /* ======================================================== - * 8b. Insert into prev neighbor (full node, at_head, avail_prev) - * Exercises: lpAppend into prev node in _quicklistInsert - * ======================================================== */ - { - robj *list = createQuicklistObject(4, 0); /* fill=4 */ - quicklist *ql = objectGetVal(list); - - /* Create: node0(4 full) + node1(4 full), then pop one from head - * so node0 has room. */ - for (int i = 0; i < 8; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "avprev_%d", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - TEST_ASSERT(ql->len == 2); - /* Pop one from head to make room in node0 (3 items) while node1 stays full (4) */ - unsigned char *data; size_t sz; long long val; - quicklistPop(ql, QUICKLIST_HEAD, &data, &sz, &val); - if (data) zfree(data); - TEST_ASSERT(ql->head->count == 3); - TEST_ASSERT(ql->tail->count == 4); - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Insert before head of node1 (at_head && !after && avail_prev) - * node1 is full (4), its prev (node0) has room (3 < 4). */ - quicklistEntry entry; - quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 3, &entry); - TEST_ASSERT(iter != NULL); - quicklistInsertBefore(iter, &entry, "NEIGHBOR_PREV", 13); - quicklistReleaseIterator(iter); - TEST_ASSERT(ql->len == 2); /* should NOT have created a new node */ - TEST_ASSERT(ql->head->count == 4); /* prev absorbed the insert */ - ASSERT_TRACKED_SIZE_CORRECT(list); - - decrRefCount(list); - } - - /* ======================================================== - * 9. ReplaceEntry — listpack path (lpReplace) - * Exercises: entry_alloc_sz read + lp_last_alloc_size after lpReplace - * ======================================================== */ - { - robj *list = createQuicklistObject(-2, 0); - quicklist *ql = objectGetVal(list); - - for (int i = 0; i < 20; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "replace_%d", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Replace with a smaller value */ - quicklistEntry entry; - quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 5, &entry); - TEST_ASSERT(iter != NULL); - quicklistReplaceEntry(iter, &entry, "S", 1); - quicklistReleaseIterator(iter); - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Replace with a larger value */ - iter = quicklistGetIteratorEntryAtIdx(ql, 10, &entry); - TEST_ASSERT(iter != NULL); - char big_replace[200]; - memset(big_replace, 'X', sizeof(big_replace)); - quicklistReplaceEntry(iter, &entry, big_replace, sizeof(big_replace)); - quicklistReleaseIterator(iter); - ASSERT_TRACKED_SIZE_CORRECT(list); - - decrRefCount(list); - } - - /* ======================================================== - * 10. ReplaceEntry — plain node path (zmalloc_usable for new entry) - * Exercises: plain node replace with large element - * ======================================================== */ - { - quicklistSetPackedThreshold(64); - - robj *list = createQuicklistObject(-2, 0); - quicklist *ql = objectGetVal(list); - - /* Create a plain node with a large element */ - char large[128]; - memset(large, 'A', sizeof(large)); - quicklistPushTail(ql, large, sizeof(large)); - /* Add some normal elements too */ - for (int i = 0; i < 10; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "mixed_%d", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Replace the plain node with another large element */ - quicklistEntry entry; - quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 0, &entry); - TEST_ASSERT(iter != NULL); - TEST_ASSERT(QL_NODE_IS_PLAIN(entry.node)); - char new_large[256]; - memset(new_large, 'B', sizeof(new_large)); - quicklistReplaceEntry(iter, &entry, new_large, sizeof(new_large)); - quicklistReleaseIterator(iter); - ASSERT_TRACKED_SIZE_CORRECT(list); - - quicklistSetPackedThreshold(0); - decrRefCount(list); - } - - /* ======================================================== - * 11. DelRange — partial and full node deletion - * Exercises: lpDeleteRange + tracked_size delta, and __quicklistDelNode - * ======================================================== */ - { - robj *list = createQuicklistObject(5, 0); - quicklist *ql = objectGetVal(list); - - for (int i = 0; i < 30; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "delrange_%d", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Delete a partial range from the middle (doesn't delete whole nodes) */ - quicklistDelRange(ql, 3, 2); - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Delete a range spanning multiple nodes */ - quicklistDelRange(ql, 2, 15); - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Delete from head */ - quicklistDelRange(ql, 0, 3); - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Delete from tail (negative index) */ - quicklistDelRange(ql, -3, 3); - ASSERT_TRACKED_SIZE_CORRECT(list); - - decrRefCount(list); - } - - /* ======================================================== - * 12. Rotate (move tail to head) - * Exercises: quicklistPushHead + quicklistDelIndex on tail - * ======================================================== */ - { - robj *list = createQuicklistObject(3, 0); - quicklist *ql = objectGetVal(list); - - for (int i = 0; i < 15; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "rotate_%d", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - for (int i = 0; i < 10; i++) { - quicklistRotate(ql); - ASSERT_TRACKED_SIZE_CORRECT(list); - } - - decrRefCount(list); - } - - /* ======================================================== - * 13. Dup (deep copy) - * Exercises: quicklistDup -> zmalloc_usable for each node entry - * ======================================================== */ - { - robj *list = createQuicklistObject(-2, 0); - quicklist *ql = objectGetVal(list); - - for (int i = 0; i < 100; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "dup_test_%d", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - quicklist *copy = quicklistDup(ql); - robj *copy_list = createObject(OBJ_LIST, copy); - copy_list->encoding = OBJ_ENCODING_QUICKLIST; - ASSERT_TRACKED_SIZE_CORRECT(copy_list); - - /* Mutate the copy and verify tracking stays correct */ - for (int i = 0; i < 20; i++) { - unsigned char *data; - size_t sz; - long long val; - quicklistPop(copy, QUICKLIST_TAIL, &data, &sz, &val); - if (data) zfree(data); - } - ASSERT_TRACKED_SIZE_CORRECT(copy_list); - - decrRefCount(copy_list); - decrRefCount(list); - } - - /* ======================================================== - * 14. Dup with compression - * Exercises: quicklistDup with LZF-compressed nodes - * ======================================================== */ - { - robj *list = createQuicklistObject(-2, 1); /* compress depth=1 */ - quicklist *ql = objectGetVal(list); - - for (int i = 0; i < 500; i++) { - char buf[64]; - snprintf(buf, sizeof(buf), "dup_compress_%d_padding_data_here", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - quicklist *copy = quicklistDup(ql); - robj *copy_list = createObject(OBJ_LIST, copy); - copy_list->encoding = OBJ_ENCODING_QUICKLIST; - ASSERT_TRACKED_SIZE_CORRECT(copy_list); - - decrRefCount(copy_list); - decrRefCount(list); - } - - /* ======================================================== - * 15. Node merge via _quicklistListpackMerge - * Exercises: lpMerge + lp_last_alloc_size for merged node - * _quicklistMergeNodes is called after insert-triggered splits. - * Use fill=4: create 3 full nodes (12 elements), then delete - * elements from 2 adjacent nodes so they become small enough to - * merge when a split triggers _quicklistMergeNodes. - * ======================================================== */ - { - robj *list = createQuicklistObject(4, 0); /* fill=4, count-based */ - quicklist *ql = objectGetVal(list); - - /* Create 3 full nodes of 4 elements each = 12 elements */ - for (int i = 0; i < 12; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "merge_%d", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - TEST_ASSERT(ql->len == 3); - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Delete 3 elements from the second node to make it small (count=1) */ - quicklistDelRange(ql, 4, 3); - /* Now: node0 has 4, node1 has 1, node2 has 4. Total 3 nodes, 9 elements. */ - TEST_ASSERT(ql->len == 3); - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Delete 3 elements from the third node to make it small (count=1) */ - quicklistDelRange(ql, 5, 3); - /* Now: node0 has 4, node1 has 1, node2 has 1. Total 3 nodes, 6 elements. */ - TEST_ASSERT(ql->len == 3); - ASSERT_TRACKED_SIZE_CORRECT(list); - - unsigned long nodes_before = ql->len; - - /* Insert into the middle of the full first node to trigger a split. - * After split, _quicklistMergeNodes will try to merge neighbors. - * node1 (count=1) and node2 (count=1) can merge since 1+1 <= fill=4. */ - quicklistEntry entry; - quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 2, &entry); - TEST_ASSERT(iter != NULL); - quicklistInsertAfter(iter, &entry, "MERGE_TRIGGER", 13); - quicklistReleaseIterator(iter); - - /* The split created a new node, but merges should have reduced the count */ - TEST_ASSERT(ql->len <= nodes_before + 1); /* split adds 1, merges may reduce */ - ASSERT_TRACKED_SIZE_CORRECT(list); - - decrRefCount(list); - } - - /* ======================================================== - * 16. Plain node insert - * Exercises: __quicklistCreateNode with PLAIN container, zmalloc_usable - * ======================================================== */ - { - quicklistSetPackedThreshold(64); - - robj *list = createQuicklistObject(-2, 0); - quicklist *ql = objectGetVal(list); - - /* Push small elements first */ - for (int i = 0; i < 5; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "small_%d", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Push large elements that become plain nodes */ - for (int i = 0; i < 5; i++) { - char large[128]; - memset(large, 'P' + i, sizeof(large)); - quicklistPushTail(ql, large, sizeof(large)); - ASSERT_TRACKED_SIZE_CORRECT(list); - } - - /* Pop plain nodes */ - for (int i = 0; i < 3; i++) { - unsigned char *data; - size_t sz; - long long val; - quicklistPop(ql, QUICKLIST_TAIL, &data, &sz, &val); - if (data) zfree(data); - ASSERT_TRACKED_SIZE_CORRECT(list); - } - - quicklistSetPackedThreshold(0); - decrRefCount(list); - } - - /* ======================================================== - * 17. Insert into empty list (no reference node) - * Exercises: _quicklistInsert with node==NULL path - * ======================================================== */ - { - robj *list = createQuicklistObject(-2, 0); - quicklist *ql = objectGetVal(list); - - quicklistEntry entry; - memset(&entry, 0, sizeof(entry)); - entry.quicklist = ql; - quicklistIter *iter = quicklistGetIterator(ql, AL_START_HEAD); - quicklistInsertAfter(iter, &entry, "first_ever", 10); - quicklistReleaseIterator(iter); - ASSERT_TRACKED_SIZE_CORRECT(list); - - decrRefCount(list); - } - - /* ======================================================== - * 18. Compression with operations interleaved - * Exercises: repeated compress/decompress cycles via mixed ops - * ======================================================== */ - { - robj *list = createQuicklistObject(-2, 2); /* compress depth=2 */ - quicklist *ql = objectGetVal(list); - - /* Build a large list so inner nodes get compressed */ - for (int i = 0; i < 1000; i++) { - char buf[64]; - snprintf(buf, sizeof(buf), "interleaved_%d_extra_padding_here", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Interleave pushes and pops from both ends */ - for (int i = 0; i < 100; i++) { - char buf[64]; - snprintf(buf, sizeof(buf), "interleaved_new_%d", i); - if (i % 2 == 0) - quicklistPushHead(ql, buf, strlen(buf)); - else - quicklistPushTail(ql, buf, strlen(buf)); - - unsigned char *data; - size_t sz; - long long val; - if (i % 3 == 0) - quicklistPop(ql, QUICKLIST_HEAD, &data, &sz, &val); - else - quicklistPop(ql, QUICKLIST_TAIL, &data, &sz, &val); - if (data) zfree(data); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Replace elements in the middle (forces decompress + recompress) */ - for (int i = 0; i < 10; i++) { - quicklistEntry entry; - quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, (long)quicklistCount(ql) / 2, &entry); - if (iter) { - char buf[100]; - memset(buf, 'R', sizeof(buf)); - quicklistReplaceEntry(iter, &entry, buf, sizeof(buf)); - quicklistReleaseIterator(iter); - } - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - decrRefCount(list); - } - - /* ======================================================== - * 19. AppendListpack / AppendPlainNode (RDB load paths) - * Exercises: zmalloc_size at load time for entry_alloc_sz - * ======================================================== */ - { - robj *list = createQuicklistObject(-2, 0); - quicklist *ql = objectGetVal(list); - - /* Simulate RDB load: create a listpack externally and append */ - unsigned char *lp = lpNew(0); - for (int i = 0; i < 10; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "rdb_lp_%d", i); - lp = lpAppend(lp, (unsigned char *)buf, strlen(buf)); - } - quicklistAppendListpack(ql, lp); - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Append another listpack */ - unsigned char *lp2 = lpNew(0); - for (int i = 0; i < 5; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "rdb_lp2_%d", i); - lp2 = lpAppend(lp2, (unsigned char *)buf, strlen(buf)); - } - quicklistAppendListpack(ql, lp2); - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Append a plain node */ - size_t plain_sz = 256; - unsigned char *plain_data = zmalloc(plain_sz); - memset(plain_data, 'Z', plain_sz); - quicklistAppendPlainNode(ql, plain_data, plain_sz); - ASSERT_TRACKED_SIZE_CORRECT(list); - - decrRefCount(list); - } - - /* ======================================================== - * 20. DelEntry via iterator - * Exercises: quicklistDelEntry -> quicklistDelIndex for each entry - * ======================================================== */ - { - robj *list = createQuicklistObject(5, 0); - quicklist *ql = objectGetVal(list); - - for (int i = 0; i < 25; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "delentry_%d", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Delete every other element via iterator */ - quicklistIter *iter = quicklistGetIterator(ql, AL_START_HEAD); - quicklistEntry entry; - int count = 0; - while (quicklistNext(iter, &entry)) { - if (count % 2 == 0) { - quicklistDelEntry(iter, &entry); - } - count++; - } - quicklistReleaseIterator(iter); - ASSERT_TRACKED_SIZE_CORRECT(list); - - decrRefCount(list); - } - - /* ======================================================== - * 21. ReplaceAtIndex - * Exercises: quicklistReplaceAtIndex -> quicklistReplaceEntry - * ======================================================== */ - { - robj *list = createQuicklistObject(-2, 0); - quicklist *ql = objectGetVal(list); - - for (int i = 0; i < 30; i++) { - char buf[32]; - snprintf(buf, sizeof(buf), "replaceatidx_%d", i); - quicklistPushTail(ql, buf, strlen(buf)); - } - ASSERT_TRACKED_SIZE_CORRECT(list); - - /* Replace at various indices */ - quicklistReplaceAtIndex(ql, 0, "HEAD_REPLACED", 13); - ASSERT_TRACKED_SIZE_CORRECT(list); - - quicklistReplaceAtIndex(ql, 15, "MID_REPLACED", 12); - ASSERT_TRACKED_SIZE_CORRECT(list); - - quicklistReplaceAtIndex(ql, 29, "TAIL_REPLACED", 13); - ASSERT_TRACKED_SIZE_CORRECT(list); - - decrRefCount(list); - } - - return 0; -} diff --git a/src/unit/test_quicklist_tracking.cpp b/src/unit/test_quicklist_tracking.cpp new file mode 100644 index 00000000000..935df780dc7 --- /dev/null +++ b/src/unit/test_quicklist_tracking.cpp @@ -0,0 +1,660 @@ +/* + * Copyright (c) Valkey Contributors + * All rights reserved. + * SPDX-License-Identifier: BSD-3-Clause + */ + +#include "generated_wrappers.hpp" + +#include +#include +#include +#include + +extern "C" { +#include "listpack.h" +#include "quicklist.h" +#include "server.h" +#include "zmalloc.h" + +size_t objectComputeSize(robj *key, robj *o, size_t sample_size, int dbid); +size_t objectComputeSizeWithTrackedSize(robj *key, robj *o, size_t sample_size, int dbid); +} + +class QuicklistTrackingTest : public ::testing::Test { +}; + +/* Helper: assert tracked_size matches ground-truth objectComputeSize. + * objectComputeSize crashes on empty quicklists (do-while on NULL head), + * so we guard against that by only comparing when the list is non-empty. + * For empty lists, we just verify tracked_size equals sizeof(quicklist). */ +#define ASSERT_TRACKED_SIZE_CORRECT(list) \ + do { \ + quicklist *_ql = (quicklist *)objectGetVal(list); \ + if (_ql->len > 0) { \ + size_t _computed = objectComputeSize(nullptr, (list), SIZE_MAX, 0); \ + size_t _tracked = objectComputeSizeWithTrackedSize(nullptr, (list), SIZE_MAX, 0); \ + ASSERT_EQ(_computed, _tracked); \ + } else { \ + ASSERT_EQ(_ql->tracked_size, sizeof(quicklist)); \ + } \ + } while (0) + +/* 1. Basic pushHead / pushTail into existing nodes */ +TEST_F(QuicklistTrackingTest, BasicPushHeadTail) { + robj *list = createQuicklistObject(3, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 50; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "element_%d", i); + if (i % 2 == 0) + quicklistPushTail(ql, buf, strlen(buf)); + else + quicklistPushHead(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + decrRefCount(list); +} + +/* 2. pushHead / pushTail that create new nodes (fill=1 forces new node per element) */ +TEST_F(QuicklistTrackingTest, PushCreatesNewNodes) { + robj *list = createQuicklistObject(1, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 20; i++) { + char buf[64]; + snprintf(buf, sizeof(buf), "single_node_element_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_EQ(ql->len, 20ul); + ASSERT_TRACKED_SIZE_CORRECT(list); + decrRefCount(list); +} + +/* 3. Pop from head and tail */ +TEST_F(QuicklistTrackingTest, PopHeadAndTail) { + robj *list = createQuicklistObject(3, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 30; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "pop_test_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + for (int i = 0; i < 8; i++) { + unsigned char *data; + size_t sz; + long long val; + quicklistPop(ql, QUICKLIST_HEAD, &data, &sz, &val); + if (data) zfree(data); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + for (int i = 0; i < 8; i++) { + unsigned char *data; + size_t sz; + long long val; + quicklistPop(ql, QUICKLIST_TAIL, &data, &sz, &val); + if (data) zfree(data); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); +} + +/* 4. Delete entire node (pop all elements) */ +TEST_F(QuicklistTrackingTest, DeleteEntireNode) { + robj *list = createQuicklistObject(2, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 10; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "del_node_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_EQ(ql->len, 5ul); + ASSERT_TRACKED_SIZE_CORRECT(list); + + while (quicklistCount(ql) > 0) { + unsigned char *data; + size_t sz; + long long val; + quicklistPop(ql, QUICKLIST_HEAD, &data, &sz, &val); + if (data) zfree(data); + ASSERT_TRACKED_SIZE_CORRECT(list); + } + decrRefCount(list); +} + +/* 5. Compression and decompression */ +TEST_F(QuicklistTrackingTest, CompressDecompress) { + robj *list = createQuicklistObject(-2, 1); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 500; i++) { + char buf[64]; + snprintf(buf, sizeof(buf), "compress_test_value_%d_padding_data", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_GE(ql->len, 3ul); + bool has_compressed = false; + for (quicklistNode *n = ql->head; n; n = n->next) { + if (quicklistNodeIsCompressed(n)) { + has_compressed = true; + break; + } + } + ASSERT_TRUE(has_compressed); + ASSERT_TRACKED_SIZE_CORRECT(list); + + for (int i = 0; i < 50; i++) { + unsigned char *data; + size_t sz; + long long val; + quicklistPop(ql, QUICKLIST_HEAD, &data, &sz, &val); + if (data) zfree(data); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + for (int i = 0; i < 50; i++) { + unsigned char *data; + size_t sz; + long long val; + quicklistPop(ql, QUICKLIST_TAIL, &data, &sz, &val); + if (data) zfree(data); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + for (int i = 0; i < 50; i++) { + char buf[64]; + snprintf(buf, sizeof(buf), "after_compress_push_%d", i); + quicklistPushHead(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); +} + +/* 6. InsertBefore / InsertAfter into non-full node */ +TEST_F(QuicklistTrackingTest, InsertNonFullNode) { + robj *list = createQuicklistObject(-2, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 20; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "insert_test_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistEntry entry; + quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 5, &entry); + ASSERT_NE(iter, nullptr); + quicklistInsertAfter(iter, &entry, (void *)"INSERTED_AFTER", 14); + quicklistReleaseIterator(iter); + ASSERT_TRACKED_SIZE_CORRECT(list); + + iter = quicklistGetIteratorEntryAtIdx(ql, 10, &entry); + ASSERT_NE(iter, nullptr); + quicklistInsertBefore(iter, &entry, (void *)"INSERTED_BEFORE", 15); + quicklistReleaseIterator(iter); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); +} + +/* 7. Insert that triggers node split */ +TEST_F(QuicklistTrackingTest, InsertTriggersSplit) { + robj *list = createQuicklistObject(3, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 12; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "split_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_EQ(ql->len, 4ul); + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistEntry entry; + quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 4, &entry); + ASSERT_NE(iter, nullptr); + quicklistInsertAfter(iter, &entry, (void *)"SPLIT_INSERT", 12); + quicklistReleaseIterator(iter); + ASSERT_GT(ql->len, 4ul); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); +} + +/* 8a. Insert into next neighbor (full node, at_tail, avail_next) */ +TEST_F(QuicklistTrackingTest, InsertNextNeighbor) { + robj *list = createQuicklistObject(4, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 6; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "neighbor_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_EQ(ql->len, 2ul); + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistEntry entry; + quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 3, &entry); + ASSERT_NE(iter, nullptr); + quicklistInsertAfter(iter, &entry, (void *)"NEIGHBOR_NEXT", 13); + quicklistReleaseIterator(iter); + ASSERT_EQ(ql->len, 2ul); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); +} + +/* 8b. Insert into prev neighbor (full node, at_head, avail_prev) */ +TEST_F(QuicklistTrackingTest, InsertPrevNeighbor) { + robj *list = createQuicklistObject(4, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 8; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "avprev_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_EQ(ql->len, 2ul); + unsigned char *data; + size_t sz; + long long val; + quicklistPop(ql, QUICKLIST_HEAD, &data, &sz, &val); + if (data) zfree(data); + ASSERT_EQ(ql->head->count, 3u); + ASSERT_EQ(ql->tail->count, 4u); + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistEntry entry; + quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 3, &entry); + ASSERT_NE(iter, nullptr); + quicklistInsertBefore(iter, &entry, (void *)"NEIGHBOR_PREV", 13); + quicklistReleaseIterator(iter); + ASSERT_EQ(ql->len, 2ul); + ASSERT_EQ(ql->head->count, 4u); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); +} + +/* 9. ReplaceEntry - listpack path (lpReplace) */ +TEST_F(QuicklistTrackingTest, ReplaceEntryListpack) { + robj *list = createQuicklistObject(-2, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 20; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "replace_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistEntry entry; + quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 5, &entry); + ASSERT_NE(iter, nullptr); + quicklistReplaceEntry(iter, &entry, (void *)"S", 1); + quicklistReleaseIterator(iter); + ASSERT_TRACKED_SIZE_CORRECT(list); + + iter = quicklistGetIteratorEntryAtIdx(ql, 10, &entry); + ASSERT_NE(iter, nullptr); + char big_replace[200]; + memset(big_replace, 'X', sizeof(big_replace)); + quicklistReplaceEntry(iter, &entry, big_replace, sizeof(big_replace)); + quicklistReleaseIterator(iter); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); +} + +/* 10. ReplaceEntry - plain node path */ +TEST_F(QuicklistTrackingTest, ReplaceEntryPlainNode) { + quicklistSetPackedThreshold(64); + + robj *list = createQuicklistObject(-2, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + char large[128]; + memset(large, 'A', sizeof(large)); + quicklistPushTail(ql, large, sizeof(large)); + for (int i = 0; i < 10; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "mixed_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistEntry entry; + quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 0, &entry); + ASSERT_NE(iter, nullptr); + ASSERT_TRUE(QL_NODE_IS_PLAIN(entry.node)); + char new_large[256]; + memset(new_large, 'B', sizeof(new_large)); + quicklistReplaceEntry(iter, &entry, new_large, sizeof(new_large)); + quicklistReleaseIterator(iter); + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistSetPackedThreshold(0); + decrRefCount(list); +} + +/* 11. DelRange - partial and full node deletion */ +TEST_F(QuicklistTrackingTest, DelRange) { + robj *list = createQuicklistObject(5, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 30; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "delrange_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistDelRange(ql, 3, 2); + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistDelRange(ql, 2, 15); + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistDelRange(ql, 0, 3); + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistDelRange(ql, -3, 3); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); +} + +/* 12. Rotate (move tail to head) */ +TEST_F(QuicklistTrackingTest, Rotate) { + robj *list = createQuicklistObject(3, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 15; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "rotate_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + for (int i = 0; i < 10; i++) { + quicklistRotate(ql); + ASSERT_TRACKED_SIZE_CORRECT(list); + } + + decrRefCount(list); +} + +/* 13. Dup (deep copy) */ +TEST_F(QuicklistTrackingTest, Dup) { + robj *list = createQuicklistObject(-2, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 100; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "dup_test_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklist *copy = quicklistDup(ql); + robj *copy_list = createObject(OBJ_LIST, copy); + copy_list->encoding = OBJ_ENCODING_QUICKLIST; + ASSERT_TRACKED_SIZE_CORRECT(copy_list); + + for (int i = 0; i < 20; i++) { + unsigned char *data; + size_t sz; + long long val; + quicklistPop(copy, QUICKLIST_TAIL, &data, &sz, &val); + if (data) zfree(data); + } + ASSERT_TRACKED_SIZE_CORRECT(copy_list); + + decrRefCount(copy_list); + decrRefCount(list); +} + +/* 14. Dup with compression */ +TEST_F(QuicklistTrackingTest, DupWithCompression) { + robj *list = createQuicklistObject(-2, 1); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 500; i++) { + char buf[64]; + snprintf(buf, sizeof(buf), "dup_compress_%d_padding_data_here", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklist *copy = quicklistDup(ql); + robj *copy_list = createObject(OBJ_LIST, copy); + copy_list->encoding = OBJ_ENCODING_QUICKLIST; + ASSERT_TRACKED_SIZE_CORRECT(copy_list); + + decrRefCount(copy_list); + decrRefCount(list); +} + +/* 15. Node merge via _quicklistListpackMerge */ +TEST_F(QuicklistTrackingTest, NodeMerge) { + robj *list = createQuicklistObject(4, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 12; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "merge_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_EQ(ql->len, 3ul); + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistDelRange(ql, 4, 3); + ASSERT_EQ(ql->len, 3ul); + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistDelRange(ql, 5, 3); + ASSERT_EQ(ql->len, 3ul); + ASSERT_TRACKED_SIZE_CORRECT(list); + + unsigned long nodes_before = ql->len; + + quicklistEntry entry; + quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, 2, &entry); + ASSERT_NE(iter, nullptr); + quicklistInsertAfter(iter, &entry, (void *)"MERGE_TRIGGER", 13); + quicklistReleaseIterator(iter); + + ASSERT_LE(ql->len, nodes_before + 1); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); +} + +/* 16. Plain node insert */ +TEST_F(QuicklistTrackingTest, PlainNodeInsert) { + quicklistSetPackedThreshold(64); + + robj *list = createQuicklistObject(-2, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 5; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "small_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + for (int i = 0; i < 5; i++) { + char large[128]; + memset(large, 'P' + i, sizeof(large)); + quicklistPushTail(ql, large, sizeof(large)); + ASSERT_TRACKED_SIZE_CORRECT(list); + } + + for (int i = 0; i < 3; i++) { + unsigned char *data; + size_t sz; + long long val; + quicklistPop(ql, QUICKLIST_TAIL, &data, &sz, &val); + if (data) zfree(data); + ASSERT_TRACKED_SIZE_CORRECT(list); + } + + quicklistSetPackedThreshold(0); + decrRefCount(list); +} + +/* 17. Insert into empty list (no reference node) */ +TEST_F(QuicklistTrackingTest, InsertEmptyList) { + robj *list = createQuicklistObject(-2, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + quicklistEntry entry; + memset(&entry, 0, sizeof(entry)); + entry.quicklist = ql; + quicklistIter *iter = quicklistGetIterator(ql, AL_START_HEAD); + quicklistInsertAfter(iter, &entry, (void *)"first_ever", 10); + quicklistReleaseIterator(iter); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); +} + +/* 18. Compression with operations interleaved */ +TEST_F(QuicklistTrackingTest, InterleavedCompressOps) { + robj *list = createQuicklistObject(-2, 2); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 1000; i++) { + char buf[64]; + snprintf(buf, sizeof(buf), "interleaved_%d_extra_padding_here", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + for (int i = 0; i < 100; i++) { + char buf[64]; + snprintf(buf, sizeof(buf), "interleaved_new_%d", i); + if (i % 2 == 0) + quicklistPushHead(ql, buf, strlen(buf)); + else + quicklistPushTail(ql, buf, strlen(buf)); + + unsigned char *data; + size_t sz; + long long val; + if (i % 3 == 0) + quicklistPop(ql, QUICKLIST_HEAD, &data, &sz, &val); + else + quicklistPop(ql, QUICKLIST_TAIL, &data, &sz, &val); + if (data) zfree(data); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + for (int i = 0; i < 10; i++) { + quicklistEntry entry; + quicklistIter *iter = quicklistGetIteratorEntryAtIdx(ql, (long)quicklistCount(ql) / 2, &entry); + if (iter) { + char buf[100]; + memset(buf, 'R', sizeof(buf)); + quicklistReplaceEntry(iter, &entry, buf, sizeof(buf)); + quicklistReleaseIterator(iter); + } + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); +} + +/* 19. AppendListpack / AppendPlainNode (RDB load paths) */ +TEST_F(QuicklistTrackingTest, AppendListpackAndPlainNode) { + robj *list = createQuicklistObject(-2, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + unsigned char *lp = lpNew(0); + for (int i = 0; i < 10; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "rdb_lp_%d", i); + lp = lpAppend(lp, (unsigned char *)buf, strlen(buf)); + } + quicklistAppendListpack(ql, lp); + ASSERT_TRACKED_SIZE_CORRECT(list); + + unsigned char *lp2 = lpNew(0); + for (int i = 0; i < 5; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "rdb_lp2_%d", i); + lp2 = lpAppend(lp2, (unsigned char *)buf, strlen(buf)); + } + quicklistAppendListpack(ql, lp2); + ASSERT_TRACKED_SIZE_CORRECT(list); + + size_t plain_sz = 256; + unsigned char *plain_data = (unsigned char *)zmalloc(plain_sz); + memset(plain_data, 'Z', plain_sz); + quicklistAppendPlainNode(ql, plain_data, plain_sz); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); +} + +/* 20. DelEntry via iterator */ +TEST_F(QuicklistTrackingTest, DelEntryViaIterator) { + robj *list = createQuicklistObject(5, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 25; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "delentry_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistIter *iter = quicklistGetIterator(ql, AL_START_HEAD); + quicklistEntry entry; + int count = 0; + while (quicklistNext(iter, &entry)) { + if (count % 2 == 0) { + quicklistDelEntry(iter, &entry); + } + count++; + } + quicklistReleaseIterator(iter); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); +} + +/* 21. ReplaceAtIndex */ +TEST_F(QuicklistTrackingTest, ReplaceAtIndex) { + robj *list = createQuicklistObject(-2, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + for (int i = 0; i < 30; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), "replaceatidx_%d", i); + quicklistPushTail(ql, buf, strlen(buf)); + } + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistReplaceAtIndex(ql, 0, (void *)"HEAD_REPLACED", 13); + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistReplaceAtIndex(ql, 15, (void *)"MID_REPLACED", 12); + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistReplaceAtIndex(ql, 29, (void *)"TAIL_REPLACED", 13); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); +} From 898201ea6a67b33d697a6a84a8eec93eef64780f Mon Sep 17 00:00:00 2001 From: Lior Sventitzky Date: Mon, 2 Mar 2026 13:55:39 +0000 Subject: [PATCH 5/6] changed global to static, fixed same-size listpack changes Signed-off-by: Lior Sventitzky --- src/listpack.c | 46 +++++++++++++++++++++++----- src/listpack.h | 1 + src/listpack_malloc.h | 9 +++--- src/quicklist.c | 42 ++++++++++++------------- src/unit/test_quicklist_tracking.cpp | 41 +++++++++++++++++++++++++ 5 files changed, 106 insertions(+), 33 deletions(-) diff --git a/src/listpack.c b/src/listpack.c index a1695b51e68..7cfbcf199bf 100644 --- a/src/listpack.c +++ b/src/listpack.c @@ -42,7 +42,13 @@ #include "listpack.h" #include "listpack_malloc.h" -__thread size_t lp_last_alloc_size = 0; +/* lp_last_alloc_size is defined as a file-local static in listpack_malloc.h + * (included above). This getter exposes it to other modules (e.g. quicklist) + * without leaking the variable as an extern global. */ +size_t lpLastAllocSize(void) { + return lp_last_alloc_size; +} + #include "serverassert.h" #include "util.h" #include "config.h" @@ -177,9 +183,13 @@ void lpFreeVoid(void *lp) { /* Shrink the memory to fit. */ unsigned char *lpShrinkToFit(unsigned char *lp) { size_t size = lpGetTotalBytes(lp); - if (size < lp_malloc_size(lp)) { + size_t alloc_size = lp_malloc_size(lp); + if (size < alloc_size) { return lp_realloc(lp, size); } else { + /* Already at minimum allocation — no realloc needed. + * Update lp_last_alloc_size so callers don't see a stale value. */ + lp_last_alloc_size = alloc_size; return lp; } } @@ -779,9 +789,20 @@ unsigned char *lpInsert(unsigned char *lp, unsigned char *dst = lp + poff; /* May be updated after reallocation. */ /* Realloc before: we need more room. */ - if (new_listpack_bytes > old_listpack_bytes && new_listpack_bytes > lp_malloc_size(lp)) { - if ((lp = lp_realloc(lp, new_listpack_bytes)) == NULL) return NULL; - dst = lp + poff; + if (new_listpack_bytes > old_listpack_bytes) { + size_t alloc_size = lp_malloc_size(lp); + if (new_listpack_bytes > alloc_size) { + if ((lp = lp_realloc(lp, new_listpack_bytes)) == NULL) return NULL; + dst = lp + poff; + } else { + /* Growth fits within jemalloc slack — no realloc needed. + * Update lp_last_alloc_size so callers see the current value. */ + lp_last_alloc_size = alloc_size; + } + } else if (new_listpack_bytes == old_listpack_bytes) { + /* Same size (e.g. replace with same-length element) — no realloc. + * Update lp_last_alloc_size so callers don't see a stale value. */ + lp_last_alloc_size = lp_malloc_size(lp); } /* Setup the listpack relocating the elements to make the exact room @@ -929,7 +950,10 @@ unsigned char *lpDeleteRangeWithEntry(unsigned char *lp, unsigned char **p, unsi unsigned char *first, *tail; first = tail = *p; - if (num == 0) return lp; /* Nothing to delete, return ASAP. */ + if (num == 0) { /* Nothing to delete, return ASAP. */ + lp_last_alloc_size = lp_malloc_size(lp); + return lp; + } /* Find the next entry to the last entry that needs to be deleted. * lpLength may be unreliable due to corrupt data, so we cannot @@ -968,8 +992,14 @@ unsigned char *lpDeleteRange(unsigned char *lp, long index, unsigned long num) { unsigned char *p; uint32_t numele = lpGetNumElements(lp); - if (num == 0) return lp; /* Nothing to delete, return ASAP. */ - if ((p = lpSeek(lp, index)) == NULL) return lp; + if (num == 0) { /* Nothing to delete, return ASAP. */ + lp_last_alloc_size = lp_malloc_size(lp); + return lp; + } + if ((p = lpSeek(lp, index)) == NULL) { + lp_last_alloc_size = lp_malloc_size(lp); + return lp; + } /* If we know we're gonna delete beyond the end of the listpack, we can just move * the EOF marker, and there's no need to iterate through the entries, diff --git a/src/listpack.h b/src/listpack.h index b1437972610..5886b6ea6d9 100644 --- a/src/listpack.h +++ b/src/listpack.h @@ -97,5 +97,6 @@ unsigned char * lpNextRandom(unsigned char *lp, unsigned char *p, unsigned int *index, unsigned int remaining, int even_only); int lpSafeToAdd(unsigned char *lp, size_t add); void lpRepr(unsigned char *lp); +size_t lpLastAllocSize(void); #endif diff --git a/src/listpack_malloc.h b/src/listpack_malloc.h index 9e8e5d386d0..352c6948cf9 100644 --- a/src/listpack_malloc.h +++ b/src/listpack_malloc.h @@ -40,10 +40,11 @@ #define LISTPACK_ALLOC_H #include "zmalloc.h" -/* Thread-local that captures the usable allocation size from the last - * lp_malloc / lp_realloc call. zrealloc_usable already computes this - * internally; we just stop discarding it. */ -extern __thread size_t lp_last_alloc_size; +/* File-local variable defined in listpack.c that captures the usable + * allocation size from the last lp_malloc / lp_realloc call. zmalloc_usable + * and zrealloc_usable already compute this internally; we just stop + * discarding it. Exposed to callers via lpLastAllocSize(). */ +static size_t lp_last_alloc_size = 0; /* We use zmalloc_usable/zrealloc_usable instead of zmalloc/zrealloc * to ensure the safe invocation of 'zmalloc_usable_size(). diff --git a/src/quicklist.c b/src/quicklist.c index a89545379b6..327e38be064 100644 --- a/src/quicklist.c +++ b/src/quicklist.c @@ -121,8 +121,8 @@ static quicklistNode *_quicklistMergeNodes(quicklist *quicklist, quicklistNode * /* After a listpack operation that reallocates node->entry (e.g. lpPrepend, * lpAppend, lpDelete, lpInsertString, lpReplace, lpDeleteRange), the - * lp_last_alloc_size thread-local holds the usable allocation size of the - * new entry buffer. We use it to update node->entry_alloc_sz and + * lpLastAllocSize() returns the usable allocation size of the new entry + * buffer. We use it to update node->entry_alloc_sz and * quicklist->tracked_size without calling zmalloc_size(). */ /* Create a new quicklist. @@ -549,7 +549,7 @@ static quicklistNode *__quicklistCreateNode(int container, void *value, size_t s memcpy(new_node->entry, value, sz); } else { new_node->entry = lpPrepend(lpNew(0), value, sz); - new_node->entry_alloc_sz = lp_last_alloc_size; + new_node->entry_alloc_sz = lpLastAllocSize(); } new_node->sz = sz; new_node->count++; @@ -578,13 +578,13 @@ int quicklistPushHead(quicklist *quicklist, void *value, size_t sz) { if (likely(_quicklistNodeAllowInsert(quicklist->head, quicklist->fill, sz))) { size_t old_sz = quicklist->head->entry_alloc_sz; quicklist->head->entry = lpPrepend(quicklist->head->entry, value, sz); - quicklist->head->entry_alloc_sz = lp_last_alloc_size; + quicklist->head->entry_alloc_sz = lpLastAllocSize(); quicklist->tracked_size += quicklist->head->entry_alloc_sz - old_sz; quicklistNodeUpdateSz(quicklist->head); } else { quicklistNode *node = quicklistCreateNode(); node->entry = lpPrepend(lpNew(0), value, sz); - node->entry_alloc_sz = lp_last_alloc_size; + node->entry_alloc_sz = lpLastAllocSize(); quicklistNodeUpdateSz(node); _quicklistInsertNodeBefore(quicklist, quicklist->head, node); @@ -608,13 +608,13 @@ int quicklistPushTail(quicklist *quicklist, void *value, size_t sz) { if (likely(_quicklistNodeAllowInsert(quicklist->tail, quicklist->fill, sz))) { size_t old_sz = quicklist->tail->entry_alloc_sz; quicklist->tail->entry = lpAppend(quicklist->tail->entry, value, sz); - quicklist->tail->entry_alloc_sz = lp_last_alloc_size; + quicklist->tail->entry_alloc_sz = lpLastAllocSize(); quicklist->tracked_size += quicklist->tail->entry_alloc_sz - old_sz; quicklistNodeUpdateSz(quicklist->tail); } else { quicklistNode *node = quicklistCreateNode(); node->entry = lpAppend(lpNew(0), value, sz); - node->entry_alloc_sz = lp_last_alloc_size; + node->entry_alloc_sz = lpLastAllocSize(); quicklistNodeUpdateSz(node); _quicklistInsertNodeAfter(quicklist, quicklist->tail, node); @@ -714,7 +714,7 @@ static int quicklistDelIndex(quicklist *quicklist, quicklistNode *node, unsigned } size_t old_sz = node->entry_alloc_sz; node->entry = lpDelete(node->entry, *p, p); - node->entry_alloc_sz = lp_last_alloc_size; + node->entry_alloc_sz = lpLastAllocSize(); quicklist->tracked_size += node->entry_alloc_sz - old_sz; node->count--; if (node->count == 0) { @@ -771,7 +771,7 @@ void quicklistReplaceEntry(quicklistIter *iter, quicklistEntry *entry, void *dat (_old_entry_sz = entry->node->entry_alloc_sz, newentry = lpReplace(entry->node->entry, &entry->zi, data, sz)) != NULL)) { entry->node->entry = newentry; - entry->node->entry_alloc_sz = lp_last_alloc_size; + entry->node->entry_alloc_sz = lpLastAllocSize(); quicklist->tracked_size += entry->node->entry_alloc_sz - _old_entry_sz; quicklistNodeUpdateSz(entry->node); /* quicklistNext() and quicklistGetIteratorEntryAtIdx() provide an uncompressed node */ @@ -872,13 +872,13 @@ static quicklistNode *_quicklistListpackMerge(quicklist *quicklist, quicklistNod nokeep = a; keep = b; /* Update tracked size: b's entry was reallocated by lpMerge */ - keep->entry_alloc_sz = lp_last_alloc_size; + keep->entry_alloc_sz = lpLastAllocSize(); quicklist->tracked_size += keep->entry_alloc_sz - b_entry_sz; } else if (!b->entry) { nokeep = b; keep = a; /* Update tracked size: a's entry was reallocated by lpMerge */ - keep->entry_alloc_sz = lp_last_alloc_size; + keep->entry_alloc_sz = lpLastAllocSize(); quicklist->tracked_size += keep->entry_alloc_sz - a_entry_sz; } /* nokeep->entry was freed by lpMerge (entry is NULL), but we leave @@ -994,13 +994,13 @@ static quicklistNode *_quicklistSplitNode(quicklist *quicklist, quicklistNode *n /* Track entry change for original node (new_node not in list yet) */ size_t old_sz = node->entry_alloc_sz; node->entry = lpDeleteRange(node->entry, orig_start, orig_extent); - node->entry_alloc_sz = lp_last_alloc_size; + node->entry_alloc_sz = lpLastAllocSize(); if (quicklist) quicklist->tracked_size += node->entry_alloc_sz - old_sz; node->count = lpLength(node->entry); quicklistNodeUpdateSz(node); new_node->entry = lpDeleteRange(new_node->entry, new_start, new_extent); - new_node->entry_alloc_sz = lp_last_alloc_size; + new_node->entry_alloc_sz = lpLastAllocSize(); new_node->count = lpLength(new_node->entry); quicklistNodeUpdateSz(new_node); @@ -1028,7 +1028,7 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v } new_node = quicklistCreateNode(); new_node->entry = lpPrepend(lpNew(0), value, sz); - new_node->entry_alloc_sz = lp_last_alloc_size; + new_node->entry_alloc_sz = lpLastAllocSize(); __quicklistInsertNode(quicklist, NULL, new_node, after); new_node->count++; quicklist->count++; @@ -1079,7 +1079,7 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v quicklistDecompressNodeForUse(quicklist, node); size_t old_sz = node->entry_alloc_sz; node->entry = lpInsertString(node->entry, value, sz, entry->zi, LP_AFTER, NULL); - node->entry_alloc_sz = lp_last_alloc_size; + node->entry_alloc_sz = lpLastAllocSize(); quicklist->tracked_size += node->entry_alloc_sz - old_sz; node->count++; quicklistNodeUpdateSz(node); @@ -1089,7 +1089,7 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v quicklistDecompressNodeForUse(quicklist, node); size_t old_sz = node->entry_alloc_sz; node->entry = lpInsertString(node->entry, value, sz, entry->zi, LP_BEFORE, NULL); - node->entry_alloc_sz = lp_last_alloc_size; + node->entry_alloc_sz = lpLastAllocSize(); quicklist->tracked_size += node->entry_alloc_sz - old_sz; node->count++; quicklistNodeUpdateSz(node); @@ -1102,7 +1102,7 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v quicklistDecompressNodeForUse(quicklist, new_node); size_t old_sz = new_node->entry_alloc_sz; new_node->entry = lpPrepend(new_node->entry, value, sz); - new_node->entry_alloc_sz = lp_last_alloc_size; + new_node->entry_alloc_sz = lpLastAllocSize(); quicklist->tracked_size += new_node->entry_alloc_sz - old_sz; new_node->count++; quicklistNodeUpdateSz(new_node); @@ -1116,7 +1116,7 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v quicklistDecompressNodeForUse(quicklist, new_node); size_t old_sz = new_node->entry_alloc_sz; new_node->entry = lpAppend(new_node->entry, value, sz); - new_node->entry_alloc_sz = lp_last_alloc_size; + new_node->entry_alloc_sz = lpLastAllocSize(); quicklist->tracked_size += new_node->entry_alloc_sz - old_sz; new_node->count++; quicklistNodeUpdateSz(new_node); @@ -1128,7 +1128,7 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v D("\tprovisioning new node..."); new_node = quicklistCreateNode(); new_node->entry = lpPrepend(lpNew(0), value, sz); - new_node->entry_alloc_sz = lp_last_alloc_size; + new_node->entry_alloc_sz = lpLastAllocSize(); new_node->count++; quicklistNodeUpdateSz(new_node); __quicklistInsertNode(quicklist, node, new_node, after); @@ -1142,7 +1142,7 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v new_node->entry = lpPrepend(new_node->entry, value, sz); else new_node->entry = lpAppend(new_node->entry, value, sz); - new_node->entry_alloc_sz = lp_last_alloc_size; + new_node->entry_alloc_sz = lpLastAllocSize(); /* Don't track delta here; __quicklistInsertNode below adds the full entry_alloc_sz */ new_node->count++; quicklistNodeUpdateSz(new_node); @@ -1235,7 +1235,7 @@ int quicklistDelRange(quicklist *quicklist, const long start, const long count) quicklistDecompressNodeForUse(quicklist, node); size_t old_sz = node->entry_alloc_sz; node->entry = lpDeleteRange(node->entry, offset, del); - node->entry_alloc_sz = lp_last_alloc_size; + node->entry_alloc_sz = lpLastAllocSize(); quicklist->tracked_size += node->entry_alloc_sz - old_sz; quicklistNodeUpdateSz(node); node->count -= del; diff --git a/src/unit/test_quicklist_tracking.cpp b/src/unit/test_quicklist_tracking.cpp index 935df780dc7..830cd8eb82d 100644 --- a/src/unit/test_quicklist_tracking.cpp +++ b/src/unit/test_quicklist_tracking.cpp @@ -658,3 +658,44 @@ TEST_F(QuicklistTrackingTest, ReplaceAtIndex) { decrRefCount(list); } + +/* 23. Same-size replacement: ensures lpLastAllocSize is not stale. + * When lpReplace replaces an element with one of the exact same encoded + * size, lpInsert may skip lp_realloc entirely, which used to leave + * lp_last_alloc_size holding a value from a previous (different node) + * operation. This test creates two nodes, mutates node A (changing + * lp_last_alloc_size), then does a same-size replace on node B to + * verify that tracked_size remains accurate. */ +TEST_F(QuicklistTrackingTest, SameSizeReplaceNoStale) { + /* fill=4 to create multiple nodes quickly. */ + robj *list = createQuicklistObject(4, 0); + quicklist *ql = (quicklist *)objectGetVal(list); + + /* Fill two nodes: 4 entries of "aaaa" each. */ + for (int i = 0; i < 8; i++) { + quicklistPushTail(ql, (void *)"aaaa", 4); + } + ASSERT_EQ(ql->len, 2ul); + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Mutate node A (head) with a differently-sized value to change + * lpLastAllocSize to something different from node B's alloc size. */ + quicklistReplaceAtIndex(ql, 0, (void *)"bbbbbbbbbbbbbbbb", 16); + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Now do a same-size replace on node B (tail): replace "aaaa" with + * "cccc" (same 4 bytes). lpInsert will compute new == old bytes + * and may skip lp_realloc. If lp_last_alloc_size is stale from + * the head node's operation, tracked_size will become wrong. */ + quicklistReplaceAtIndex(ql, 7, (void *)"cccc", 4); + ASSERT_TRACKED_SIZE_CORRECT(list); + + /* Also test multiple same-size replacements in a row. */ + quicklistReplaceAtIndex(ql, 5, (void *)"dddd", 4); + ASSERT_TRACKED_SIZE_CORRECT(list); + + quicklistReplaceAtIndex(ql, 6, (void *)"eeee", 4); + ASSERT_TRACKED_SIZE_CORRECT(list); + + decrRefCount(list); +} From afc726592852896cea964bcbbd5fca8677f8ac50 Mon Sep 17 00:00:00 2001 From: Lior Sventitzky Date: Mon, 2 Mar 2026 14:20:26 +0000 Subject: [PATCH 6/6] refactored to use macro Signed-off-by: Lior Sventitzky --- src/quicklist.c | 50 ++++++++++++++++++++----------------------------- src/quicklist.h | 1 - 2 files changed, 20 insertions(+), 31 deletions(-) diff --git a/src/quicklist.c b/src/quicklist.c index 327e38be064..3d31f106e52 100644 --- a/src/quicklist.c +++ b/src/quicklist.c @@ -35,7 +35,6 @@ #include "zmalloc.h" #include "config.h" #include "listpack.h" -#include "listpack_malloc.h" #include "util.h" /* for ll2string */ #include "lzf.h" #include "serverassert.h" @@ -119,11 +118,14 @@ static quicklistNode *_quicklistMergeNodes(quicklist *quicklist, quicklistNode * (iter)->zi = NULL; \ } while (0) -/* After a listpack operation that reallocates node->entry (e.g. lpPrepend, - * lpAppend, lpDelete, lpInsertString, lpReplace, lpDeleteRange), the - * lpLastAllocSize() returns the usable allocation size of the new entry - * buffer. We use it to update node->entry_alloc_sz and - * quicklist->tracked_size without calling zmalloc_size(). */ +/* Update node->entry_alloc_sz from lpLastAllocSize() and adjust + * quicklist->tracked_size by the delta. Use after any listpack operation + * that may reallocate node->entry, for nodes already linked in the list. */ +#define quicklistTrackEntryResize(ql, node, old_alloc_sz) \ + do { \ + (node)->entry_alloc_sz = lpLastAllocSize(); \ + if (ql) (ql)->tracked_size += (node)->entry_alloc_sz - (old_alloc_sz); \ + } while (0) /* Create a new quicklist. * Free with quicklistRelease(). */ @@ -578,8 +580,7 @@ int quicklistPushHead(quicklist *quicklist, void *value, size_t sz) { if (likely(_quicklistNodeAllowInsert(quicklist->head, quicklist->fill, sz))) { size_t old_sz = quicklist->head->entry_alloc_sz; quicklist->head->entry = lpPrepend(quicklist->head->entry, value, sz); - quicklist->head->entry_alloc_sz = lpLastAllocSize(); - quicklist->tracked_size += quicklist->head->entry_alloc_sz - old_sz; + quicklistTrackEntryResize(quicklist, quicklist->head, old_sz); quicklistNodeUpdateSz(quicklist->head); } else { quicklistNode *node = quicklistCreateNode(); @@ -608,8 +609,7 @@ int quicklistPushTail(quicklist *quicklist, void *value, size_t sz) { if (likely(_quicklistNodeAllowInsert(quicklist->tail, quicklist->fill, sz))) { size_t old_sz = quicklist->tail->entry_alloc_sz; quicklist->tail->entry = lpAppend(quicklist->tail->entry, value, sz); - quicklist->tail->entry_alloc_sz = lpLastAllocSize(); - quicklist->tracked_size += quicklist->tail->entry_alloc_sz - old_sz; + quicklistTrackEntryResize(quicklist, quicklist->tail, old_sz); quicklistNodeUpdateSz(quicklist->tail); } else { quicklistNode *node = quicklistCreateNode(); @@ -714,8 +714,7 @@ static int quicklistDelIndex(quicklist *quicklist, quicklistNode *node, unsigned } size_t old_sz = node->entry_alloc_sz; node->entry = lpDelete(node->entry, *p, p); - node->entry_alloc_sz = lpLastAllocSize(); - quicklist->tracked_size += node->entry_alloc_sz - old_sz; + quicklistTrackEntryResize(quicklist, node, old_sz); node->count--; if (node->count == 0) { gone = 1; @@ -771,8 +770,7 @@ void quicklistReplaceEntry(quicklistIter *iter, quicklistEntry *entry, void *dat (_old_entry_sz = entry->node->entry_alloc_sz, newentry = lpReplace(entry->node->entry, &entry->zi, data, sz)) != NULL)) { entry->node->entry = newentry; - entry->node->entry_alloc_sz = lpLastAllocSize(); - quicklist->tracked_size += entry->node->entry_alloc_sz - _old_entry_sz; + quicklistTrackEntryResize(quicklist, entry->node, _old_entry_sz); quicklistNodeUpdateSz(entry->node); /* quicklistNext() and quicklistGetIteratorEntryAtIdx() provide an uncompressed node */ quicklistCompress(quicklist, entry->node); @@ -872,14 +870,12 @@ static quicklistNode *_quicklistListpackMerge(quicklist *quicklist, quicklistNod nokeep = a; keep = b; /* Update tracked size: b's entry was reallocated by lpMerge */ - keep->entry_alloc_sz = lpLastAllocSize(); - quicklist->tracked_size += keep->entry_alloc_sz - b_entry_sz; + quicklistTrackEntryResize(quicklist, keep, b_entry_sz); } else if (!b->entry) { nokeep = b; keep = a; /* Update tracked size: a's entry was reallocated by lpMerge */ - keep->entry_alloc_sz = lpLastAllocSize(); - quicklist->tracked_size += keep->entry_alloc_sz - a_entry_sz; + quicklistTrackEntryResize(quicklist, keep, a_entry_sz); } /* nokeep->entry was freed by lpMerge (entry is NULL), but we leave * nokeep->entry_alloc_sz unchanged so __quicklistDelNode subtracts @@ -994,8 +990,7 @@ static quicklistNode *_quicklistSplitNode(quicklist *quicklist, quicklistNode *n /* Track entry change for original node (new_node not in list yet) */ size_t old_sz = node->entry_alloc_sz; node->entry = lpDeleteRange(node->entry, orig_start, orig_extent); - node->entry_alloc_sz = lpLastAllocSize(); - if (quicklist) quicklist->tracked_size += node->entry_alloc_sz - old_sz; + quicklistTrackEntryResize(quicklist, node, old_sz); node->count = lpLength(node->entry); quicklistNodeUpdateSz(node); @@ -1079,8 +1074,7 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v quicklistDecompressNodeForUse(quicklist, node); size_t old_sz = node->entry_alloc_sz; node->entry = lpInsertString(node->entry, value, sz, entry->zi, LP_AFTER, NULL); - node->entry_alloc_sz = lpLastAllocSize(); - quicklist->tracked_size += node->entry_alloc_sz - old_sz; + quicklistTrackEntryResize(quicklist, node, old_sz); node->count++; quicklistNodeUpdateSz(node); quicklistRecompressOnly(quicklist, node); @@ -1089,8 +1083,7 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v quicklistDecompressNodeForUse(quicklist, node); size_t old_sz = node->entry_alloc_sz; node->entry = lpInsertString(node->entry, value, sz, entry->zi, LP_BEFORE, NULL); - node->entry_alloc_sz = lpLastAllocSize(); - quicklist->tracked_size += node->entry_alloc_sz - old_sz; + quicklistTrackEntryResize(quicklist, node, old_sz); node->count++; quicklistNodeUpdateSz(node); quicklistRecompressOnly(quicklist, node); @@ -1102,8 +1095,7 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v quicklistDecompressNodeForUse(quicklist, new_node); size_t old_sz = new_node->entry_alloc_sz; new_node->entry = lpPrepend(new_node->entry, value, sz); - new_node->entry_alloc_sz = lpLastAllocSize(); - quicklist->tracked_size += new_node->entry_alloc_sz - old_sz; + quicklistTrackEntryResize(quicklist, new_node, old_sz); new_node->count++; quicklistNodeUpdateSz(new_node); quicklistRecompressOnly(quicklist, new_node); @@ -1116,8 +1108,7 @@ static void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry, void *v quicklistDecompressNodeForUse(quicklist, new_node); size_t old_sz = new_node->entry_alloc_sz; new_node->entry = lpAppend(new_node->entry, value, sz); - new_node->entry_alloc_sz = lpLastAllocSize(); - quicklist->tracked_size += new_node->entry_alloc_sz - old_sz; + quicklistTrackEntryResize(quicklist, new_node, old_sz); new_node->count++; quicklistNodeUpdateSz(new_node); quicklistRecompressOnly(quicklist, new_node); @@ -1235,8 +1226,7 @@ int quicklistDelRange(quicklist *quicklist, const long start, const long count) quicklistDecompressNodeForUse(quicklist, node); size_t old_sz = node->entry_alloc_sz; node->entry = lpDeleteRange(node->entry, offset, del); - node->entry_alloc_sz = lpLastAllocSize(); - quicklist->tracked_size += node->entry_alloc_sz - old_sz; + quicklistTrackEntryResize(quicklist, node, old_sz); quicklistNodeUpdateSz(node); node->count -= del; quicklist->count -= del; diff --git a/src/quicklist.h b/src/quicklist.h index 30a0c8fd96a..403aeac96cb 100644 --- a/src/quicklist.h +++ b/src/quicklist.h @@ -187,7 +187,6 @@ int quicklistPopCustom(quicklist *quicklist, void *(*saver)(unsigned char *data, size_t sz)); int quicklistPop(quicklist *quicklist, int where, unsigned char **data, size_t *sz, long long *slong); unsigned long quicklistCount(const quicklist *ql); -size_t quicklistGetAllocatedSize(const quicklist *ql); int quicklistCompare(quicklistEntry *entry, unsigned char *p2, const size_t p2_len); size_t quicklistGetLzf(const quicklistNode *node, void **data); void quicklistNodeLimit(int fill, size_t *size, unsigned int *count);