Skip to content

Commit 185ec3b

Browse files
committed
fix: avoid rescanning compressed list nodes on load
Post-load zstd compression now scans backward from the tail and stops at the first already-compressed node, avoiding repeated scans of previously processed chunks.
1 parent 373b339 commit 185ec3b

2 files changed

Lines changed: 57 additions & 10 deletions

File tree

src/core/qlist.cc

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,7 @@ QList& QList::operator=(QList&& other) noexcept {
496496
void QList::MoveFrom(QList&& other) {
497497
head_ = other.head_;
498498
len_ = other.len_;
499+
malloc_size_ = other.malloc_size_;
499500
count_ = other.count_;
500501
fill_ = other.fill_;
501502
dict_learning_failed_ = other.dict_learning_failed_;
@@ -504,9 +505,12 @@ void QList::MoveFrom(QList&& other) {
504505
tiering_enabled_ = other.tiering_enabled_;
505506
compress_ = other.compress_;
506507
bookmark_count_ = other.bookmark_count_;
508+
db_id_ = other.db_id_;
509+
zstd_threshold_ = other.zstd_threshold_;
507510
tiering_params_ = std::move(other.tiering_params_);
508511

509512
other.head_ = nullptr;
513+
other.malloc_size_ = 0;
510514
other.len_ = other.count_ = 0;
511515
}
512516

@@ -967,7 +971,6 @@ void QList::CoolOff(Node* node, uint32_t node_id) {
967971
} else if (tl_zstd_dict) {
968972
// Dict exists (trained by this or another instance), bulk-compress all interior nodes.
969973
BackfillCompressWithZstdDict();
970-
dict_bulk_finished_ = 1;
971974
} else if (!dict_learning_failed_ && malloc_size_ >= zstd_threshold_ && len_ >= 2) {
972975
// No dict yet, try to train one.
973976
TrainZstdDict();
@@ -1604,13 +1607,17 @@ bool QList::TrainZstdDict() {
16041607
void QList::BackfillCompressWithZstdDict() {
16051608
DCHECK(tl_zstd_dict);
16061609

1607-
// Bulk-compress all interior nodes, tracking memory delta.
1610+
if (len_ < 3)
1611+
return;
1612+
16081613
bool any_compressed = false;
16091614
bool any_attempted = false;
1610-
for (Node* node = head_; node; node = node->next) {
1611-
if (node == head_ || node->next == nullptr)
1612-
continue;
1613-
if (node->encoding == QUICKLIST_NODE_ENCODING_RAW && node->sz >= MIN_COMPRESS_BYTES)
1615+
// Scan from tail backwards. On chunked loads, the first compressed node marks the already
1616+
// processed prefix.
1617+
for (Node* node = head_->prev->prev; node && node != head_; node = node->prev) {
1618+
if (node->encoding != QUICKLIST_NODE_ENCODING_RAW)
1619+
break;
1620+
if (node->sz >= MIN_COMPRESS_BYTES)
16141621
any_attempted = true;
16151622
size_t prev_size = zmalloc_usable_size(node->entry);
16161623
if (CompressNodeWithDict(node)) {
@@ -1620,16 +1627,19 @@ void QList::BackfillCompressWithZstdDict() {
16201627
}
16211628

16221629
// Only mark failure if we actually tried to compress nodes and all failed.
1623-
if (any_attempted && !any_compressed) {
1624-
dict_bulk_failed_ = 1;
1630+
if (any_attempted) {
1631+
if (any_compressed)
1632+
dict_bulk_finished_ = 1;
1633+
else
1634+
dict_bulk_failed_ = 1;
16251635
}
16261636
}
16271637

16281638
void QList::CompressAfterLoad() {
16291639
// Mirrors the ZSTD branch of CoolOff(), but runs in one shot after a bulk load
16301640
// instead of being driven incrementally by per-element pushes (which never
16311641
// happen during AppendListpack/AppendPlain).
1632-
if (!IsZstdDictMode())
1642+
if (!IsZstdDictMode() || dict_bulk_failed_)
16331643
return;
16341644

16351645
if (!tl_zstd_dict) {
@@ -1643,7 +1653,6 @@ void QList::CompressAfterLoad() {
16431653

16441654
// A dictionary exists (trained here or by another list on this thread).
16451655
BackfillCompressWithZstdDict();
1646-
dict_bulk_finished_ = 1;
16471656
}
16481657

16491658
bool QList::CompressNodeWithDict(Node* node) {

src/core/qlist_test.cc

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,6 +1217,44 @@ TEST_F(QListZstdTest, CompressAfterLoad) {
12171217
EXPECT_EQ(count, kNodes * kEntriesPerNode);
12181218
}
12191219

1220+
TEST_F(QListZstdTest, CompressAfterLoadChunkedAppend) {
1221+
QList ql(-1, 0);
1222+
ql.set_compr_threshold(1);
1223+
1224+
constexpr unsigned kEntriesPerNode = 30;
1225+
for (unsigned n = 0; n < 4; ++n) {
1226+
ql.AppendListpack(BuildCeleryListpack(kEntriesPerNode, 100000 + n * kEntriesPerNode));
1227+
}
1228+
ql.CompressAfterLoad();
1229+
1230+
const QList::Node* old_tail = ql.Tail();
1231+
ASSERT_EQ(old_tail->encoding, QUICKLIST_NODE_ENCODING_RAW);
1232+
1233+
QList moved(std::move(ql));
1234+
old_tail = moved.Tail();
1235+
1236+
for (unsigned n = 4; n < 7; ++n) {
1237+
moved.AppendListpack(BuildCeleryListpack(kEntriesPerNode, 100000 + n * kEntriesPerNode));
1238+
}
1239+
moved.CompressAfterLoad();
1240+
1241+
// The previous tail became interior after appending the next chunk, so it should be compressed
1242+
// without rescanning the already-finished prefix.
1243+
EXPECT_EQ(old_tail->encoding, QLIST_NODE_ENCODING_ZSTD);
1244+
EXPECT_EQ(moved.Head()->encoding, QUICKLIST_NODE_ENCODING_RAW);
1245+
EXPECT_EQ(moved.Tail()->encoding, QUICKLIST_NODE_ENCODING_RAW);
1246+
1247+
unsigned count = 0;
1248+
moved.Iterate(
1249+
[&](const QList::Entry& e) {
1250+
EXPECT_GT(e.view().size(), 50u);
1251+
++count;
1252+
return true;
1253+
},
1254+
0, -1);
1255+
EXPECT_EQ(count, 7u * kEntriesPerNode);
1256+
}
1257+
12201258
TEST_F(QListZstdTest, CompressAfterLoadPlainNode) {
12211259
// Plain (single large element) interior nodes are also covered: they are
12221260
// included in dictionary training and compressed, and decompressed on read.

0 commit comments

Comments
 (0)