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

content/browser/indexed_db/leveldb/leveldb_transaction.cc

Index: content/browser/indexed_db/leveldb/leveldb_transaction.cc
diff --git a/content/browser/indexed_db/leveldb/leveldb_transaction.cc b/content/browser/indexed_db/leveldb/leveldb_transaction.cc
index 1722549384aeec3bb98b4a0d8afc4ac75507d778..4fba3224bd0bf24b7484160397f609f20e8a5bb9 100644
--- a/content/browser/indexed_db/leveldb/leveldb_transaction.cc
+++ b/content/browser/indexed_db/leveldb/leveldb_transaction.cc
@@ -41,7 +41,11 @@ LevelDBTransaction::LevelDBTransaction(LevelDBDatabase* db)
 LevelDBTransaction::Record::Record() {}
 LevelDBTransaction::Record::~Record() {}
 
-LevelDBTransaction::~LevelDBTransaction() {}
+LevelDBTransaction::~LevelDBTransaction() {
+  for (TransactionIterator* iter : iterators_) {

[jsbell, 2017/03/22 23:55:32]

I left a comment before... can this be a DCHECK since iterators should not
outlive transactions? Or was I wrong?

[dmurph, 2017/03/23 20:56:48]

removed.

+    db_->NotifyIteratorDestroyed(iter);
+  }
+}
 
 void LevelDBTransaction::Set(const StringPiece& key,
                              std::string* value,
@@ -244,10 +248,11 @@ LevelDBTransaction::TransactionIterator::~TransactionIterator() {
 }
 
 bool LevelDBTransaction::TransactionIterator::IsValid() const {
-  return !!current_;
+  return !!current_ || db_iterator_state_ == DBIteratorState::EVICTED_AND_VALID;
 }
 
 leveldb::Status LevelDBTransaction::TransactionIterator::SeekToLast() {
+  LoadDBIteratorIfEvicted();
   leveldb::Status s = data_iterator_->SeekToLast();
   DCHECK(s.ok());
   s = db_iterator_->SeekToLast();
@@ -262,6 +267,7 @@ leveldb::Status LevelDBTransaction::TransactionIterator::SeekToLast() {
 
 leveldb::Status LevelDBTransaction::TransactionIterator::Seek(
     const StringPiece& target) {
+  LoadDBIteratorIfEvicted();
   leveldb::Status s = data_iterator_->Seek(target);
   DCHECK(s.ok());
   s = db_iterator_->Seek(target);
@@ -275,6 +281,7 @@ leveldb::Status LevelDBTransaction::TransactionIterator::Seek(
 }
 
 leveldb::Status LevelDBTransaction::TransactionIterator::Next() {
+  LoadDBIteratorIfEvicted();
   DCHECK(IsValid());
   if (data_changed_)
     RefreshDataIterator();
@@ -311,6 +318,7 @@ leveldb::Status LevelDBTransaction::TransactionIterator::Next() {
 }
 
 leveldb::Status LevelDBTransaction::TransactionIterator::Prev() {
+  LoadDBIteratorIfEvicted();
   DCHECK(IsValid());
   leveldb::Status s;
   if (data_changed_)
@@ -354,16 +362,41 @@ StringPiece LevelDBTransaction::TransactionIterator::Key() const {
   DCHECK(IsValid());
   if (data_changed_)
     RefreshDataIterator();
+  if (current_ == nullptr && db_iterator_state_ != DBIteratorState::ACTIVE) {

[jsbell, 2017/03/22 23:55:32]

Nit: no need for {} on single line.

or do we want DCHECK_EQ(db_iterator_state_, DBIteratorState::EVICTED_AND_VALID)
?

[dmurph, 2017/03/23 20:56:48]

Changed due to moving to impl.

+    return db_key_before_eviction_;
+  }
   return current_->Key();
 }
 
 StringPiece LevelDBTransaction::TransactionIterator::Value() const {
+  if (db_iterator_state_ != DBIteratorState::ACTIVE && current_ == nullptr) {

[jsbell, 2017/03/22 23:55:32]

This logic is subtle and probably deserves a comment.

[dmurph, 2017/03/23 20:56:48]

Made better by moving to impl.

+    const_cast<LevelDBTransaction::TransactionIterator*>(this)

[jsbell, 2017/03/22 23:55:32]

Comment about the const_cast

Explain (for future readers) why making members `mutable` is not enough.

[dmurph, 2017/03/23 20:56:48]

done.

+        ->LoadDBIteratorIfEvicted();
+  }
+  DCHECK(current_);
   DCHECK(IsValid());
   if (data_changed_)
     RefreshDataIterator();
   return current_->Value();
 }
 
+void LevelDBTransaction::TransactionIterator::PurgeMemory() {
+  DCHECK_EQ(db_iterator_state_, DBIteratorState::ACTIVE);
+  if (db_iterator_->IsValid()) {
+    db_iterator_state_ = DBIteratorState::EVICTED_AND_VALID;
+    db_key_before_eviction_ = db_iterator_->Key().as_string();
+  } else {
+    db_iterator_state_ = DBIteratorState::EVICTED_AND_INVALID;
+  }
+  if (current_ == db_iterator_.get())
+    current_ = nullptr;
+  db_iterator_.reset();
+}
+
+bool LevelDBTransaction::TransactionIterator::IsMemoryPurged() const {
+  return db_iterator_state_ != DBIteratorState::ACTIVE;
+};
+
 void LevelDBTransaction::TransactionIterator::DataChanged() {
   data_changed_ = true;
 }
@@ -411,14 +444,33 @@ void LevelDBTransaction::TransactionIterator::RefreshDataIterator() const {
 }
 
 bool LevelDBTransaction::TransactionIterator::DataIteratorIsLower() const {
+  DCHECK_EQ(db_iterator_state_, DBIteratorState::ACTIVE);
   return comparator_->Compare(data_iterator_->Key(), db_iterator_->Key()) < 0;
 }
 
 bool LevelDBTransaction::TransactionIterator::DataIteratorIsHigher() const {
+  DCHECK_EQ(db_iterator_state_, DBIteratorState::ACTIVE);
   return comparator_->Compare(data_iterator_->Key(), db_iterator_->Key()) > 0;
 }
 
+void LevelDBTransaction::TransactionIterator::LoadDBIteratorIfEvicted() {
+  transaction_->db_->NotifyIteratorUsed(this);
+
+  if (db_iterator_state_ == DBIteratorState::ACTIVE)
+    return;
+
+  db_iterator_ = transaction_->db_->CreateIterator(&transaction_->snapshot_);
+  if (db_iterator_state_ == DBIteratorState::EVICTED_AND_VALID) {
+    db_iterator_->Seek(db_key_before_eviction_);
+    db_key_before_eviction_.clear();
+  }
+  if (current_ == nullptr)
+    current_ = db_iterator_.get();
+  db_iterator_state_ = DBIteratorState::ACTIVE;
+}
+
 void LevelDBTransaction::TransactionIterator::HandleConflictsAndDeletes() {
+  LoadDBIteratorIfEvicted();
   bool loop = true;
 
   while (loop) {
@@ -452,6 +504,7 @@ void LevelDBTransaction::TransactionIterator::HandleConflictsAndDeletes() {
 
 void
 LevelDBTransaction::TransactionIterator::SetCurrentIteratorToSmallestKey() {
+  DCHECK_EQ(db_iterator_state_, DBIteratorState::ACTIVE);
   LevelDBIterator* smallest = nullptr;
 
   if (data_iterator_->IsValid())
@@ -484,11 +537,13 @@ void LevelDBTransaction::TransactionIterator::SetCurrentIteratorToLargestKey() {
 void LevelDBTransaction::RegisterIterator(TransactionIterator* iterator) {
   DCHECK(iterators_.find(iterator) == iterators_.end());
   iterators_.insert(iterator);
+  db_->NotifyIteratorCreated(iterator);
 }
 
 void LevelDBTransaction::UnregisterIterator(TransactionIterator* iterator) {
   DCHECK(iterators_.find(iterator) != iterators_.end());
   iterators_.erase(iterator);
+  db_->NotifyIteratorDestroyed(iterator);
 }
 
 void LevelDBTransaction::NotifyIterators() {