chore: remove tail field from qlist (#4220)

Also update license's change date.

Signed-off-by: Roman Gershman <roman@dragonflydb.io>
This commit is contained in:
Roman Gershman 2024-11-28 20:27:48 +02:00 committed by GitHub
parent 4aece00aac
commit 183bfaeb67
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 44 additions and 38 deletions

View file

@ -6,7 +6,7 @@
<u>Licensed Work</u>: Dragonfly including the software components, or any portion of them, and any modification. <u>Licensed Work</u>: Dragonfly including the software components, or any portion of them, and any modification.
<u>Change Date</u>: September 1, 2028 <u>Change Date</u>: March 1, 2029
<u>Change License</u>: [Apache License, Version <u>Change License</u>: [Apache License, Version
2.0](https://www.apache.org/licenses/LICENSE-2.0), as published by the 2.0](https://www.apache.org/licenses/LICENSE-2.0), as published by the

View file

@ -60,7 +60,7 @@ namespace dfly {
namespace { namespace {
static_assert(sizeof(QList) == 32); static_assert(sizeof(QList) == 24);
enum IterDir : uint8_t { FWD = 1, REV = 0 }; enum IterDir : uint8_t { FWD = 1, REV = 0 };
@ -305,13 +305,12 @@ QList::QList(int fill, int compress) : fill_(fill), compress_(compress), bookmar
QList::QList(QList&& other) QList::QList(QList&& other)
: head_(other.head_), : head_(other.head_),
tail_(other.tail_),
count_(other.count_), count_(other.count_),
len_(other.len_), len_(other.len_),
fill_(other.fill_), fill_(other.fill_),
compress_(other.compress_), compress_(other.compress_),
bookmark_count_(other.bookmark_count_) { bookmark_count_(other.bookmark_count_) {
other.head_ = other.tail_ = nullptr; other.head_ = nullptr;
other.len_ = other.count_ = 0; other.len_ = other.count_ = 0;
} }
@ -323,14 +322,13 @@ QList& QList::operator=(QList&& other) {
if (this != &other) { if (this != &other) {
Clear(); Clear();
head_ = other.head_; head_ = other.head_;
tail_ = other.tail_;
len_ = other.len_; len_ = other.len_;
count_ = other.count_; count_ = other.count_;
fill_ = other.fill_; fill_ = other.fill_;
compress_ = other.compress_; compress_ = other.compress_;
bookmark_count_ = other.bookmark_count_; bookmark_count_ = other.bookmark_count_;
other.head_ = other.tail_ = nullptr; other.head_ = nullptr;
other.len_ = other.count_ = 0; other.len_ = other.count_ = 0;
} }
return *this; return *this;
@ -348,28 +346,24 @@ void QList::Clear() {
len_--; len_--;
current = next; current = next;
} }
head_ = tail_ = nullptr; head_ = nullptr;
count_ = 0; count_ = 0;
} }
void QList::Push(string_view value, Where where) { void QList::Push(string_view value, Where where) {
/* The head and tail should never be compressed (we don't attempt to decompress them) */ /* The head and tail should never be compressed (we don't attempt to decompress them) */
if (head_) if (head_) {
DCHECK(head_->encoding != QUICKLIST_NODE_ENCODING_LZF); DCHECK(head_->encoding != QUICKLIST_NODE_ENCODING_LZF);
if (tail_) DCHECK(head_->prev->encoding != QUICKLIST_NODE_ENCODING_LZF);
DCHECK(tail_->encoding != QUICKLIST_NODE_ENCODING_LZF); }
PushSentinel(value, where); PushSentinel(value, where);
} }
string QList::Pop(Where where) { string QList::Pop(Where where) {
DCHECK_GT(count_, 0u); DCHECK_GT(count_, 0u);
quicklistNode* node; quicklistNode* node = head_;
if (where == HEAD) { if (where == TAIL) {
node = head_; node = head_->prev;
} else {
DCHECK_EQ(TAIL, where);
node = tail_;
} }
/* The head and tail should never be compressed */ /* The head and tail should never be compressed */
@ -402,7 +396,7 @@ void QList::AppendListpack(unsigned char* zl) {
node->count = lpLength(node->entry); node->count = lpLength(node->entry);
node->sz = lpBytes(zl); node->sz = lpBytes(zl);
InsertNode(tail_, node, AFTER); InsertNode(_Tail(), node, AFTER);
count_ += node->count; count_ += node->count;
} }
@ -414,7 +408,7 @@ void QList::AppendPlain(unsigned char* data, size_t sz) {
node->sz = sz; node->sz = sz;
node->container = QUICKLIST_NODE_CONTAINER_PLAIN; node->container = QUICKLIST_NODE_CONTAINER_PLAIN;
InsertNode(tail_, node, AFTER); InsertNode(_Tail(), node, AFTER);
++count_; ++count_;
} }
@ -469,7 +463,11 @@ void QList::Iterate(IterateFunc cb, long start, long end) const {
} }
bool QList::PushSentinel(string_view value, Where where) { bool QList::PushSentinel(string_view value, Where where) {
quicklistNode* orig = where == HEAD ? head_ : tail_; quicklistNode* orig = head_;
if (where == TAIL && orig) {
orig = orig->prev;
}
InsertOpt opt = where == HEAD ? BEFORE : AFTER; InsertOpt opt = where == HEAD ? BEFORE : AFTER;
size_t sz = value.size(); size_t sz = value.size();
@ -507,23 +505,27 @@ void QList::InsertNode(quicklistNode* old_node, quicklistNode* new_node, InsertO
if (old_node->next) if (old_node->next)
old_node->next->prev = new_node; old_node->next->prev = new_node;
old_node->next = new_node; old_node->next = new_node;
if (head_->prev == old_node) // if old_node is tail, update the tail to the new node.
head_->prev = new_node;
} }
if (tail_ == old_node)
tail_ = new_node;
} else { } else {
new_node->next = old_node; new_node->next = old_node;
if (old_node) { if (old_node) {
new_node->prev = old_node->prev; new_node->prev = old_node->prev;
if (old_node->prev) // if old_node is not head, link its prev to the new node.
// head->prev is tail, so we don't need to update it.
if (old_node != head_)
old_node->prev->next = new_node; old_node->prev->next = new_node;
old_node->prev = new_node; old_node->prev = new_node;
} }
if (head_ == old_node) if (head_ == old_node)
head_ = new_node; head_ = new_node;
} }
/* If this insert creates the only element so far, initialize head/tail. */ /* If this insert creates the only element so far, initialize head/tail. */
if (len_ == 0) { if (len_ == 0) {
head_ = tail_ = new_node; head_ = new_node;
head_->prev = new_node;
} }
/* Update len first, so in Compress we know exactly len */ /* Update len first, so in Compress we know exactly len */
@ -698,7 +700,7 @@ void QList::Compress(quicklistNode* node) {
return; return;
/* The head and tail should never be compressed (we should not attempt to recompress them) */ /* The head and tail should never be compressed (we should not attempt to recompress them) */
assert(head_->recompress == 0 && tail_->recompress == 0); DCHECK(head_->recompress == 0 && head_->prev->recompress == 0);
/* If length is less than our compress depth (from both sides), /* If length is less than our compress depth (from both sides),
* we can't compress anything. */ * we can't compress anything. */
@ -709,7 +711,7 @@ void QList::Compress(quicklistNode* node) {
* Note: because we do length checks at the *top* of this function, * Note: because we do length checks at the *top* of this function,
* we can skip explicit null checks below. Everything exists. */ * we can skip explicit null checks below. Everything exists. */
quicklistNode* forward = head_; quicklistNode* forward = head_;
quicklistNode* reverse = tail_; quicklistNode* reverse = head_->prev;
int depth = 0; int depth = 0;
int in_depth = 0; int in_depth = 0;
while (depth++ < compress_) { while (depth++ < compress_) {
@ -837,12 +839,10 @@ void QList::DelNode(quicklistNode* node) {
if (node->prev) if (node->prev)
node->prev->next = node->next; node->prev->next = node->next;
if (node == tail_) {
tail_ = node->prev;
}
if (node == head_) { if (node == head_) {
head_ = node->next; head_ = node->next;
} else if (node == head_->prev) { // tail
head_->prev = node->prev;
} }
/* Update len first, so in Compress we know exactly len */ /* Update len first, so in Compress we know exactly len */
@ -892,7 +892,7 @@ auto QList::GetIterator(Where where) const -> Iterator {
it.offset_ = 0; it.offset_ = 0;
it.direction_ = FWD; it.direction_ = FWD;
} else { } else {
it.current_ = tail_; it.current_ = _Tail();
it.offset_ = -1; it.offset_ = -1;
it.direction_ = REV; it.direction_ = REV;
} }
@ -918,7 +918,7 @@ auto QList::GetIterator(long idx) const -> Iterator {
seek_index = count_ - 1 - index; seek_index = count_ - 1 - index;
} }
n = seek_forward ? head_ : tail_; n = seek_forward ? head_ : head_->prev;
while (ABSL_PREDICT_TRUE(n)) { while (ABSL_PREDICT_TRUE(n)) {
if ((accum + n->count) > seek_index) { if ((accum + n->count) > seek_index) {
break; break;
@ -1119,9 +1119,8 @@ bool QList::Iterator::Next() {
} else { } else {
/* Reverse traversal, Jumping to end of previous node */ /* Reverse traversal, Jumping to end of previous node */
DCHECK_EQ(REV, direction_); DCHECK_EQ(REV, direction_);
current_ = current_->prev;
offset_ = -1; offset_ = -1;
current_ = (current_ == owner_->head_) ? nullptr : current_->prev;
} }
return current_ ? Next() : false; return current_ ? Next() : false;

View file

@ -144,7 +144,7 @@ class QList {
} }
const quicklistNode* Tail() const { const quicklistNode* Tail() const {
return tail_; return _Tail();
} }
void set_fill(int fill) { void set_fill(int fill) {
@ -158,6 +158,10 @@ class QList {
return compress_ != 0; return compress_ != 0;
} }
quicklistNode* _Tail() const {
return head_ ? head_->prev : nullptr;
}
// Returns false if used existing sentinel, true if a new sentinel was created. // Returns false if used existing sentinel, true if a new sentinel was created.
bool PushSentinel(std::string_view value, Where where); bool PushSentinel(std::string_view value, Where where);
@ -176,7 +180,7 @@ class QList {
bool DelPackedIndex(quicklistNode* node, uint8_t** p); bool DelPackedIndex(quicklistNode* node, uint8_t** p);
quicklistNode* head_ = nullptr; quicklistNode* head_ = nullptr;
quicklistNode* tail_ = nullptr;
uint32_t count_ = 0; /* total count of all entries in all listpacks */ uint32_t count_ = 0; /* total count of all entries in all listpacks */
uint32_t len_ = 0; /* number of quicklistNodes */ uint32_t len_ = 0; /* number of quicklistNodes */
signed int fill_ : QL_FILL_BITS; /* fill factor for individual nodes */ signed int fill_ : QL_FILL_BITS; /* fill factor for individual nodes */

View file

@ -90,7 +90,7 @@ static int ql_verify(const QList& ql, uint32_t nc, uint32_t count, uint32_t head
node_size = 0; node_size = 0;
while (node) { while (node) {
node_size += node->count; node_size += node->count;
node = node->prev; node = (node == ql.Head()) ? nullptr : node->prev;
} }
if (node_size != ql.Size()) { if (node_size != ql.Size()) {
LOG(ERROR) << "has different forward count than reverse count! " LOG(ERROR) << "has different forward count than reverse count! "
@ -165,7 +165,6 @@ TEST_F(QListTest, Basic) {
EXPECT_EQ(0, ql_.Size()); EXPECT_EQ(0, ql_.Size());
ql_.Push("abc", QList::HEAD); ql_.Push("abc", QList::HEAD);
EXPECT_EQ(1, ql_.Size()); EXPECT_EQ(1, ql_.Size());
EXPECT_TRUE(ql_.Head()->prev == nullptr);
EXPECT_TRUE(ql_.Tail() == ql_.Head()); EXPECT_TRUE(ql_.Tail() == ql_.Head());
auto it = ql_.GetIterator(QList::HEAD); auto it = ql_.GetIterator(QList::HEAD);
@ -517,6 +516,10 @@ TEST_P(OptionsTest, DelNeg100From500) {
for (int i = 0; i < 500; i++) for (int i = 0; i < 500; i++)
ql_.Push(StrCat("hello", i + 1), QList::TAIL); ql_.Push(StrCat("hello", i + 1), QList::TAIL);
ql_.Erase(-100, 100); ql_.Erase(-100, 100);
QList::Iterator it = ql_.GetIterator(QList::TAIL);
ASSERT_TRUE(it.Next());
ASSERT_EQ("hello400", it.Get());
ASSERT_EQ(0, ql_verify(ql_, 13, 400, 32, 16)); ASSERT_EQ(0, ql_verify(ql_, 13, 400, 32, 16));
} }