[IndexedDB] Pool and evict leveldb iterators, to save memory

content/browser/indexed_db/leveldb/leveldb_iterator_impl.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/indexed_db/leveldb/leveldb_iterator_impl.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/logging.h"
+#include "content/browser/indexed_db/leveldb/leveldb_iterator_pool_controller.h"
 
 static leveldb::Slice MakeSlice(const base::StringPiece& s) {
   return leveldb::Slice(s.begin(), s.size());
 }
 
 static base::StringPiece MakeStringPiece(const leveldb::Slice& s) {
   return base::StringPiece(s.data(), s.size());
 }
 
 namespace content {
 
 LevelDBIteratorImpl::~LevelDBIteratorImpl() {
 }
 
-LevelDBIteratorImpl::LevelDBIteratorImpl(std::unique_ptr<leveldb::Iterator> it)
-    : iterator_(std::move(it)) {}
+LevelDBIteratorImpl::LevelDBIteratorImpl(
+    std::unique_ptr<leveldb::Iterator> it,
+    LevelDBIteratorPoolController* pool_controller,
+    const leveldb::Snapshot* snapshot)
+    : iterator_(std::move(it)),
+      pool_controller_(pool_controller),
+      snapshot_(snapshot) {}
 
 void LevelDBIteratorImpl::CheckStatus() {
+  DCHECK_EQ(iterator_state_, IteratorState::ACTIVE);
   const leveldb::Status& s = iterator_->status();
   if (!s.ok())
     LOG(ERROR) << "LevelDB iterator error: " << s.ToString();
 }
 
 bool LevelDBIteratorImpl::IsValid() const {
-  return iterator_->Valid();
+  return iterator_state_ == IteratorState::EVICTED_AND_VALID ||
+         iterator_->Valid();
 }
 
 leveldb::Status LevelDBIteratorImpl::SeekToLast() {
+  RecordUsage();
+  RestoreDBIteratorIfEvicted();
   iterator_->SeekToLast();
   CheckStatus();
   return iterator_->status();
 }
 
 leveldb::Status LevelDBIteratorImpl::Seek(const base::StringPiece& target) {
+  RecordUsage();
+  RestoreDBIteratorIfEvicted();
   iterator_->Seek(MakeSlice(target));
   CheckStatus();
   return iterator_->status();
 }
 
 leveldb::Status LevelDBIteratorImpl::Next() {
   DCHECK(IsValid());
+  RecordUsage();
+  RestoreDBIteratorIfEvicted();
   iterator_->Next();
   CheckStatus();
   return iterator_->status();
 }
 
 leveldb::Status LevelDBIteratorImpl::Prev() {
   DCHECK(IsValid());
+  RecordUsage();
+  RestoreDBIteratorIfEvicted();
   iterator_->Prev();
   CheckStatus();
   return iterator_->status();
 }
 
 base::StringPiece LevelDBIteratorImpl::Key() const {
   DCHECK(IsValid());
   return MakeStringPiece(iterator_->key());

[jsbell, 2017/03/23 21:48:21]

Doesn't this need to use key_before_eviction_ if evicted?

[dmurph, 2017/03/23 22:48:46]

Ah! Thanks for catching this. Didn't transfer in refactor. Test added.

 }
 
 base::StringPiece LevelDBIteratorImpl::Value() const {
   DCHECK(IsValid());
+  // Always need to update the LRU, so we always call this. Const-cast needed,
+  // as we're dealing with a caching layer.

[jsbell, 2017/03/23 21:48:21]

Can you see if making iterator_state_, key_before_eviction_, and iterator_
mutable is enough to avoid the const_cast ?

(They achieve the same thing, so whichever ends up being the most readable)

[dmurph, 2017/03/23 22:48:47]

Since this would require making all member variables (except snapshot - which is
4/5) mutable, I'm going to stick with const_cast.

+  LevelDBIteratorImpl* non_const = const_cast<LevelDBIteratorImpl*>(this);
+  non_const->RecordUsage();
+  non_const->RestoreDBIteratorIfEvicted();
   return MakeStringPiece(iterator_->value());
 }
 
+void LevelDBIteratorImpl::PurgeMemory() {
+  DCHECK_EQ(iterator_state_, IteratorState::ACTIVE);
+  if (iterator_->Valid()) {
+    iterator_state_ = IteratorState::EVICTED_AND_VALID;
+    key_before_eviction_ = iterator_->key().ToString();
+  } else {
+    iterator_state_ = IteratorState::EVICTED_AND_INVALID;
+  }
+  iterator_.reset();
+}
+
+bool LevelDBIteratorImpl::IsEvicted() const {
+  return iterator_state_ != IteratorState::ACTIVE;
+}
+
+void LevelDBIteratorImpl::RecordUsage() {
+  pool_controller_->NotifyIteratorUsed(this);
+}
+
+void LevelDBIteratorImpl::RestoreDBIteratorIfEvicted() {
+  if (iterator_state_ == IteratorState::ACTIVE)
+    return;
+
+  iterator_ = pool_controller_->CreateLevelDBIterator(snapshot_);
+  if (iterator_state_ == IteratorState::EVICTED_AND_VALID) {
+    iterator_->Seek(key_before_eviction_);
+    key_before_eviction_.clear();
+    DCHECK(IsValid());
+  }

[jsbell, 2017/03/23 21:48:21]

else clause that DCHECK(!IsValid()) ?

[dmurph, 2017/03/23 22:48:47]

tautology - we have to be in state EVICTED_AND_INVALID in the else clause - so
it would always return true.

[jsbell, 2017/03/24 00:30:21]

Oops - I meant DCHECK(!iterator_->Valid())

+  iterator_state_ = IteratorState::ACTIVE;
+}
+
 }  // namespace content