From 6caf73ad9dae0ee91873bcb39554537b85163770 Mon Sep 17 00:00:00 2001 From: costan Date: Mon, 4 Jun 2018 12:29:13 -0700 Subject: [PATCH] Clean up Iterator. This CL renames the private struct Iterator::Cleanup -> Iterator::CleanupNode, to better reflect that it's a linked list node, and extracts duplicated code from its user in IsEmpty() and Run() methods. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=199175058 --- include/leveldb/iterator.h | 16 +++++++++--- table/iterator.cc | 61 +++++++++++++++++++++++++--------------------- 2 files changed, 45 insertions(+), 32 deletions(-) diff --git a/include/leveldb/iterator.h b/include/leveldb/iterator.h index 436508a..6c1d91b 100644 --- a/include/leveldb/iterator.h +++ b/include/leveldb/iterator.h @@ -77,17 +77,25 @@ class LEVELDB_EXPORT Iterator { // // Note that unlike all of the preceding methods, this method is // not abstract and therefore clients should not override it. - typedef void (*CleanupFunction)(void* arg1, void* arg2); + using CleanupFunction = void (*)(void* arg1, void* arg2); void RegisterCleanup(CleanupFunction function, void* arg1, void* arg2); private: - struct Cleanup { + // Cleanup functions are stored in a single-linked list. + // The list's head node is inlined in the iterator. + struct CleanupNode { + // The head node is used if the function pointer is not null. CleanupFunction function; void* arg1; void* arg2; - Cleanup* next; + CleanupNode* next; + + // True if the node is not used. Only head nodes might be unused. + bool IsEmpty() const { return function == nullptr; } + // Invokes the cleanup function. + void Run() { assert(function != nullptr); (*function)(arg1, arg2); } }; - Cleanup cleanup_; + CleanupNode cleanup_head_; }; // Return an empty iterator (yields nothing). diff --git a/table/iterator.cc b/table/iterator.cc index aff0e59..41ec1aa 100644 --- a/table/iterator.cc +++ b/table/iterator.cc @@ -7,54 +7,59 @@ namespace leveldb { Iterator::Iterator() { - cleanup_.function = nullptr; - cleanup_.next = nullptr; + cleanup_head_.function = nullptr; + cleanup_head_.next = nullptr; } Iterator::~Iterator() { - if (cleanup_.function != nullptr) { - (*cleanup_.function)(cleanup_.arg1, cleanup_.arg2); - for (Cleanup* c = cleanup_.next; c != nullptr; ) { - (*c->function)(c->arg1, c->arg2); - Cleanup* next = c->next; - delete c; - c = next; + if (!cleanup_head_.IsEmpty()) { + cleanup_head_.Run(); + for (CleanupNode* node = cleanup_head_.next; node != nullptr; ) { + node->Run(); + CleanupNode* next_node = node->next; + delete node; + node = next_node; } } } void Iterator::RegisterCleanup(CleanupFunction func, void* arg1, void* arg2) { assert(func != nullptr); - Cleanup* c; - if (cleanup_.function == nullptr) { - c = &cleanup_; + CleanupNode* node; + if (cleanup_head_.IsEmpty()) { + node = &cleanup_head_; } else { - c = new Cleanup; - c->next = cleanup_.next; - cleanup_.next = c; + node = new CleanupNode(); + node->next = cleanup_head_.next; + cleanup_head_.next = node; } - c->function = func; - c->arg1 = arg1; - c->arg2 = arg2; + node->function = func; + node->arg1 = arg1; + node->arg2 = arg2; } namespace { + class EmptyIterator : public Iterator { public: EmptyIterator(const Status& s) : status_(s) { } - virtual bool Valid() const { return false; } - virtual void Seek(const Slice& target) { } - virtual void SeekToFirst() { } - virtual void SeekToLast() { } - virtual void Next() { assert(false); } - virtual void Prev() { assert(false); } - Slice key() const { assert(false); return Slice(); } - Slice value() const { assert(false); return Slice(); } - virtual Status status() const { return status_; } + ~EmptyIterator() override = default; + + bool Valid() const override { return false; } + void Seek(const Slice& target) override { } + void SeekToFirst() override { } + void SeekToLast() override { } + void Next() override { assert(false); } + void Prev() override { assert(false); } + Slice key() const override { assert(false); return Slice(); } + Slice value() const override { assert(false); return Slice(); } + Status status() const override { return status_; } + private: Status status_; }; -} // namespace + +} // anonymous namespace Iterator* NewEmptyIterator() { return new EmptyIterator(Status::OK());