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

content/browser/indexed_db/leveldb/leveldb_database.cc

 // 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.
 
 #include "content/browser/indexed_db/leveldb/leveldb_database.h"
 
 #include <inttypes.h>
 #include <stdint.h>
 
 #include <cerrno>
@@ skipping 23 matching lines @@
 #include "third_party/leveldatabase/env_chromium.h"
 #include "third_party/leveldatabase/src/helpers/memenv/memenv.h"
 #include "third_party/leveldatabase/src/include/leveldb/db.h"
 #include "third_party/leveldatabase/src/include/leveldb/env.h"
 #include "third_party/leveldatabase/src/include/leveldb/filter_policy.h"
 #include "third_party/leveldatabase/src/include/leveldb/slice.h"
 
 using base::StringPiece;
 
 namespace content {
+static const size_t kDefaultMaxOpenIteratorsPerDatabase = 50;

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

We should note that this applies only to iterators managed by
LevelDBTransaction.

Iterators opened directly against the database are not counted, but there should
be a bounded number of those, not under the control of script. (Although
enumerating those seems wise - I haven't checked myself.)

(That said... maybe that's a todo for the future.)

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

Done.

 
 // Forcing flushes to disk at the end of a transaction guarantees that the
 // data hit disk, but drastically impacts throughput when the filesystem is
 // busy with background compactions. Not syncing trades off reliability for
 // performance. Note that background compactions which move data from the
 // log to SSTs are always done with reliable writes.
 //
 // Sync writes are necessary on Windows for quota calculations; POSIX
 // calculates file sizes correctly even when not synced to disk.
 #if defined(OS_WIN)
@@ skipping 31 matching lines @@
     const leveldb::Slice& limit) const {}
 
 void LevelDBDatabase::ComparatorAdapter::FindShortSuccessor(
     std::string* key) const {}
 
 LevelDBSnapshot::LevelDBSnapshot(LevelDBDatabase* db)
     : db_(db->db_.get()), snapshot_(db_->GetSnapshot()) {}
 
 LevelDBSnapshot::~LevelDBSnapshot() { db_->ReleaseSnapshot(snapshot_); }
 
-LevelDBDatabase::LevelDBDatabase() {}
+LevelDBDatabase::LevelDBDatabase()
+    : iterator_lru_(kDefaultMaxOpenIteratorsPerDatabase) {}
+
+LevelDBDatabase::LevelDBDatabase(size_t max_open_iterators)
+    : iterator_lru_(max_open_iterators) {}
 
 LevelDBDatabase::~LevelDBDatabase() {
+  LOCAL_HISTOGRAM_COUNTS_10000("Storage.IndexedDB.LevelDB.MaxIterators",
+                               max_iterators_);
   base::trace_event::MemoryDumpManager::GetInstance()->UnregisterDumpProvider(
       this);
   // db_'s destructor uses comparator_adapter_; order of deletion is important.
   CloseDatabase();
   comparator_adapter_.reset();
   env_.reset();
 }
 
 void LevelDBDatabase::CloseDatabase() {
   if (db_) {
@@ skipping 170 matching lines @@
                               LEVEL_DB_MAX_ERROR,
                               LEVEL_DB_MAX_ERROR + 1,
                               base::HistogramBase::kUmaTargetedHistogramFlag)
       ->Add(leveldb_error);
   if (s.IsIOError())
     ParseAndHistogramIOErrorDetails(histogram_name, s);
   else
     ParseAndHistogramCorruptionDetails(histogram_name, s);
 }
 
+leveldb::Status LevelDBDatabase::OpenForTesting(

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

Too much duplicated code. :( Can we refactor?

I don't object to changing the signature of LevelDBDatabase::Open() - we could
pass in an options object and/or move the default # of cursors into
IndexedDBBackingStore.

Or just pull out the guts of Open() into OpenInternal()

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

Done.

+    const base::FilePath& file_name,
+    const LevelDBComparator* comparator,
+    std::unique_ptr<LevelDBDatabase>* result,
+    size_t max_open_cursors) {
+  IDB_TRACE("LevelDBDatabase::Open");
+  base::TimeTicks begin_time = base::TimeTicks::Now();
+
+  std::unique_ptr<ComparatorAdapter> comparator_adapter(
+      base::MakeUnique<ComparatorAdapter>(comparator));
+
+  std::unique_ptr<leveldb::DB> db;
+  std::unique_ptr<const leveldb::FilterPolicy> filter_policy;
+  const leveldb::Status s = OpenDB(comparator_adapter.get(), LevelDBEnv::Get(),
+                                   file_name, &db, &filter_policy);
+
+  if (!s.ok()) {
+    HistogramLevelDBError("WebCore.IndexedDB.LevelDBOpenErrors", s);
+    LOG(ERROR) << "Failed to open LevelDB database from "
+               << file_name.AsUTF8Unsafe() << "," << s.ToString();
+    return s;
+  }
+
+  UMA_HISTOGRAM_MEDIUM_TIMES("WebCore.IndexedDB.LevelDB.OpenTime",
+                             base::TimeTicks::Now() - begin_time);
+
+  CheckFreeSpace("Success", file_name);
+
+  (*result) = base::WrapUnique(new LevelDBDatabase(max_open_cursors));
+  (*result)->db_ = std::move(db);
+  (*result)->comparator_adapter_ = std::move(comparator_adapter);
+  (*result)->comparator_ = comparator;
+  (*result)->filter_policy_ = std::move(filter_policy);
+  (*result)->file_name_for_tracing = file_name.BaseName().AsUTF8Unsafe();
+  return s;
+}
+
 leveldb::Status LevelDBDatabase::Open(const base::FilePath& file_name,
                                       const LevelDBComparator* comparator,
                                       std::unique_ptr<LevelDBDatabase>* result,
                                       bool* is_disk_full) {
   IDB_TRACE("LevelDBDatabase::Open");
   base::TimeTicks begin_time = base::TimeTicks::Now();
 
   std::unique_ptr<ComparatorAdapter> comparator_adapter(
       base::MakeUnique<ComparatorAdapter>(comparator));
 
@@ skipping 174 matching lines @@
                   base::trace_event::MemoryAllocatorDump::kUnitsBytes, size);
   dump->AddString("file_name", "", file_name_for_tracing);
 
   // Memory is allocated from system allocator (malloc).
   pmd->AddSuballocation(dump->guid(),
                         base::trace_event::MemoryDumpManager::GetInstance()
                             ->system_allocator_pool_name());
   return true;
 }
 
+void LevelDBDatabase::NotifyIteratorCreated(LevelDBIterator* iter) {
+  num_iterators_++;
+  max_iterators_ = std::max(max_iterators_, num_iterators_);
+}
+
+void LevelDBDatabase::NotifyIteratorUsed(LevelDBIterator* iter) {
+  // This line updates the LRU if the item exists.
+  if (iterator_lru_.Get(iter) != iterator_lru_.end())
+    return;
+  if (iterator_lru_.size() == iterator_lru_.max_size()) {
+    auto to_evict_iter = iterator_lru_.rbegin();
+    LevelDBIterator* to_evict = to_evict_iter->first;
+    to_evict->PurgeMemory();
+    iterator_lru_.Erase(to_evict_iter);
+  }
+  iterator_lru_.Put(iter, iter);
+}
+
+void LevelDBDatabase::NotifyIteratorDestroyed(LevelDBIterator* iter) {
+  DCHECK_GT(num_iterators_, 0u);
+  --num_iterators_;
+  auto it = iterator_lru_.Get(iter);

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

Peek() instead of Get(), since we don't need to mess with the ordering?

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

Done.

+  if (it == iterator_lru_.end())
+    return;
+  iterator_lru_.Erase(it);
+}
 }  // namespace content