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

content/browser/indexed_db/leveldb/leveldb_database.h

 // Copyright (c) 2013 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.
 
 #ifndef CONTENT_BROWSER_INDEXED_DB_LEVELDB_LEVELDB_DATABASE_H_
 #define CONTENT_BROWSER_INDEXED_DB_LEVELDB_LEVELDB_DATABASE_H_
 
 #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"
 #include "base/strings/string_piece.h"
 #include "base/trace_event/memory_dump_provider.h"
 #include "content/common/content_export.h"
 #include "third_party/leveldatabase/src/include/leveldb/comparator.h"
 #include "third_party/leveldatabase/src/include/leveldb/status.h"
 
 namespace leveldb {
 class Comparator;
 class DB;
 class FilterPolicy;
+class Iterator;
 class Env;
 class Snapshot;
 }
 
 namespace content {
 
 class LevelDBComparator;
 class LevelDBDatabase;
 class LevelDBIterator;
 class LevelDBWriteBatch;
@@ skipping 17 matching lines @@
   LevelDBLock() {}
   virtual ~LevelDBLock() {}
 
  private:
   DISALLOW_COPY_AND_ASSIGN(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);
 
     int Compare(const leveldb::Slice& a,
                 const leveldb::Slice& b) const override;
 
     const char* Name() const override;
 
     void FindShortestSeparator(std::string* start,
                                const leveldb::Slice& limit) const override;
     void FindShortSuccessor(std::string* key) const override;
 
    private:
     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);
   static std::unique_ptr<LevelDBLock> LockForTesting(
       const base::FilePath& file_name);
   ~LevelDBDatabase() override;
 
   leveldb::Status Put(const base::StringPiece& key, std::string* value);
   leveldb::Status Remove(const base::StringPiece& key);
   virtual leveldb::Status Get(const base::StringPiece& key,
                               std::string* value,
                               bool* found,
                               const LevelDBSnapshot* = 0);
   leveldb::Status Write(const LevelDBWriteBatch& write_batch);
   std::unique_ptr<LevelDBIterator> CreateIterator(const LevelDBSnapshot* = 0);
   const LevelDBComparator* Comparator() const;
   void Compact(const base::StringPiece& start, const base::StringPiece& stop);
   void CompactAll();
 
   // base::trace_event::MemoryDumpProvider implementation.
   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();
 
   std::unique_ptr<leveldb::Env> env_;
   std::unique_ptr<leveldb::Comparator> comparator_adapter_;
   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;
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_INDEXED_DB_LEVELDB_LEVELDB_DATABASE_H_