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

content/browser/indexed_db/leveldb/leveldb_database.h

Index: content/browser/indexed_db/leveldb/leveldb_database.h
diff --git a/content/browser/indexed_db/leveldb/leveldb_database.h b/content/browser/indexed_db/leveldb/leveldb_database.h
index cdcabb54647c83c9d2f56690e2a84ba2fecd87c9..75780e31fdf7d711c6a72a3a02258477c76232b8 100644
--- a/content/browser/indexed_db/leveldb/leveldb_database.h
+++ b/content/browser/indexed_db/leveldb/leveldb_database.h
@@ -8,6 +8,7 @@
 #include <memory>
 #include <string>
 
+#include "base/containers/mru_cache.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/strings/string16.h"
@@ -21,6 +22,7 @@ namespace leveldb {
 class Comparator;
 class DB;
 class FilterPolicy;
+class Iterator;
 class Env;
 class Snapshot;
 }
@@ -58,6 +60,10 @@ class CONTENT_EXPORT LevelDBLock {
 class CONTENT_EXPORT LevelDBDatabase
     : public base::trace_event::MemoryDumpProvider {
  public:
+  // Necessary because every iterator hangs onto leveldb blocks which can be
+  // large. See https://crbug/696055.
+  static const size_t kDefaultMaxOpenIteratorsPerDatabase = 50;
+
   class ComparatorAdapter : public leveldb::Comparator {
    public:
     explicit ComparatorAdapter(const LevelDBComparator* comparator);
@@ -75,10 +81,13 @@ class CONTENT_EXPORT LevelDBDatabase
     const LevelDBComparator* comparator_;
   };
 
+  // |max_open_cursors| cannot be 0.
   static leveldb::Status Open(const base::FilePath& file_name,
                               const LevelDBComparator* comparator,
                               std::unique_ptr<LevelDBDatabase>* db,
+                              size_t max_open_cursors,

[pwnall, 2017/03/27 17:08:04]

Sorry I caught this so late, but I think this param should come before db,
because it's an input, not an output.

[dmurph, 2017/03/27 21:32:04]

Good catch, thanks.

                               bool* is_disk_full = 0);
+
   static std::unique_ptr<LevelDBDatabase> OpenInMemory(
       const LevelDBComparator* comparator);
   static leveldb::Status Destroy(const base::FilePath& file_name);
@@ -102,11 +111,20 @@ class CONTENT_EXPORT LevelDBDatabase
   bool OnMemoryDump(const base::trace_event::MemoryDumpArgs& args,
                     base::trace_event::ProcessMemoryDump* pmd) override;
 
+  void SetCursorPoolSizeForTesting(size_t size);
+
  protected:
-  LevelDBDatabase();
+  LevelDBDatabase(size_t max_open_iterators);
 
  private:
   friend class LevelDBSnapshot;
+  friend class LevelDBIteratorImpl;
+
+  // Methods for iterator pooling.
+  std::unique_ptr<leveldb::Iterator> CreateLevelDBIterator(
+      const leveldb::Snapshot*);
+  void NotifyIteratorUsed(LevelDBIterator*);
+  void NotifyIteratorDestroyed(LevelDBIterator*);
 
   void CloseDatabase();
 
@@ -115,6 +133,28 @@ class CONTENT_EXPORT LevelDBDatabase
   std::unique_ptr<leveldb::DB> db_;
   std::unique_ptr<const leveldb::FilterPolicy> filter_policy_;
   const LevelDBComparator* comparator_;
+
+  struct DetachIteratorOnDestruct {

[pwnall, 2017/03/27 17:08:04]

OnDelete / WhenDestroyed / OnDestruction?

Also, does Chromium style forbid any sort of implementation in header files? It
seems to me that the methods here are so simple that we'd get less code from an
inline rather than a call + function.

[dmurph, 2017/03/27 21:32:04]

Done - we now only have the destructor in the .cc file, as it needs the
leveldb_iterator include file.

+    DetachIteratorOnDestruct();
+    explicit DetachIteratorOnDestruct(LevelDBIterator*);
+    DetachIteratorOnDestruct(DetachIteratorOnDestruct&& that);
+    ~DetachIteratorOnDestruct();
+
+   private:
+    LevelDBIterator* it_ = nullptr;
+
+    DISALLOW_COPY_AND_ASSIGN(DetachIteratorOnDestruct);
+  };
+
+  // Despite the type name, this object uses LRU eviction.
+  size_t lru_max_size_ = 0;
+  base::HashingMRUCache<LevelDBIterator*, DetachIteratorOnDestruct>
+      iterator_lru_;
+
+  // Recorded for UMA reporting.
+  uint32_t num_iterators_ = 0;
+  uint32_t max_iterators_ = 0;
+
   std::string file_name_for_tracing;
 };