Attachment 162826 Details for Bug 96036 – Patch

[patch] Patch

bug-96036-20120907111415.patch (text/plain), 6.27 KB, created by Joshua Bell on 2012-09-07 11:14:36 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Joshua Bell

Created: 2012-09-07 11:14:36 PDT

Size: 6.27 KB

patch

obsolete

>Subversion Revision: 127533
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c7761d0f90e3d4c1bc2f847416fd5e87cbb99b8f..30d2b7c45ebad119a74d06e3bd2c0b86199e2bbf 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,36 @@
>+2012-09-07  Joshua Bell  <jsbell@chromium.org>
>+
>+        IndexedDB: IDBFactory.deleteDatabase() is slow
>+        https://bugs.webkit.org/show_bug.cgi?id=96036
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The deleteDatabase() operation is implemented by creating a LevelDBTansaction to accumulate
>+        the records to delete, then committing it. The transaction is a tree of key/operation pairs.
>+        As each (key, delete) entry is added to the tree compares need to be made, potentially
>+        requiring full key decodes. Since this temporary transaction is never read from, this is
>+        overkill.
>+
>+        Add a new "write only" transaction type that simply wraps a write batch, which provides
>+        transaction integrity but avoids the insertion overhead.
>+
>+        No new tests - no functional changes, only perf improvement.
>+
>+        * Modules/indexeddb/IDBLevelDBBackingStore.cpp:
>+        (WebCore::IDBLevelDBBackingStore::deleteDatabase): Use new write-only transaction type.
>+        * platform/leveldb/LevelDBTransaction.cpp:
>+        (WebCore::LevelDBWriteOnlyTransaction::create):
>+        (WebCore):
>+        (WebCore::LevelDBWriteOnlyTransaction::LevelDBWriteOnlyTransaction):
>+        (WebCore::LevelDBWriteOnlyTransaction::~LevelDBWriteOnlyTransaction):
>+        (WebCore::LevelDBWriteOnlyTransaction::put):
>+        (WebCore::LevelDBWriteOnlyTransaction::remove):
>+        (WebCore::LevelDBWriteOnlyTransaction::commit):
>+        (WebCore::LevelDBWriteOnlyTransaction::rollback):
>+        * platform/leveldb/LevelDBTransaction.h:
>+        (WebCore):
>+        (LevelDBWriteOnlyTransaction):
>+
> 2012-09-03  Sam Weinig  <sam@webkit.org>
> 
>         Part 1 of removing PlatformString.h, move remaining functions to new homes
>diff --git a/Source/WebCore/Modules/indexeddb/IDBLevelDBBackingStore.cpp b/Source/WebCore/Modules/indexeddb/IDBLevelDBBackingStore.cpp
>index 04a4fbba250307aec45bfa7939452a6eec5da3a0..d5621b5a7909dfec5899c90af36f4ecac10687e5 100644
>--- a/Source/WebCore/Modules/indexeddb/IDBLevelDBBackingStore.cpp
>+++ b/Source/WebCore/Modules/indexeddb/IDBLevelDBBackingStore.cpp
>@@ -360,7 +360,7 @@ static bool deleteRange(LevelDBTransaction* transaction, const Vector<char>& beg
> bool IDBLevelDBBackingStore::deleteDatabase(const String& name)
> {
>     IDB_TRACE("IDBLevelDBBackingStore::deleteDatabase");
>-    RefPtr<LevelDBTransaction> transaction = LevelDBTransaction::create(m_db.get());
>+    RefPtr<LevelDBWriteOnlyTransaction> transaction = LevelDBWriteOnlyTransaction::create(m_db.get());
> 
>     int64_t databaseId;
>     String version;
>@@ -372,9 +372,12 @@ bool IDBLevelDBBackingStore::deleteDatabase(const String& name)
> 
>     const Vector<char> startKey = DatabaseMetaDataKey::encode(databaseId, DatabaseMetaDataKey::OriginName);
>     const Vector<char> stopKey = DatabaseMetaDataKey::encode(databaseId + 1, DatabaseMetaDataKey::OriginName);
>-    if (!deleteRange(transaction.get(), startKey, stopKey)) {
>-        transaction->rollback();
>-        return false;
>+    OwnPtr<LevelDBIterator> it = m_db->createIterator();
>+    for (it->seek(startKey); it->isValid() && compareKeys(it->key(), stopKey) < 0; it->next()) {
>+        if (!transaction->remove(it->key())) {
>+            transaction->rollback();
>+            return false;
>+        }
>     }
> 
>     const Vector<char> key = DatabaseNameKey::encode(m_identifier, name);
>diff --git a/Source/WebCore/platform/leveldb/LevelDBTransaction.cpp b/Source/WebCore/platform/leveldb/LevelDBTransaction.cpp
>index 862a5fbfbbd5aed9a8e875fdcaf3f31812df0af6..88d2db50d0a2b1814633ce6cb5a35932e17022b8 100644
>--- a/Source/WebCore/platform/leveldb/LevelDBTransaction.cpp
>+++ b/Source/WebCore/platform/leveldb/LevelDBTransaction.cpp
>@@ -485,6 +485,56 @@ void LevelDBTransaction::notifyIteratorsOfTreeChange()
>     }
> }
> 
>+PassRefPtr<LevelDBWriteOnlyTransaction> LevelDBWriteOnlyTransaction::create(LevelDBDatabase* db)
>+{
>+    return adoptRef(new LevelDBWriteOnlyTransaction(db));
>+}
>+
>+LevelDBWriteOnlyTransaction::LevelDBWriteOnlyTransaction(LevelDBDatabase* db)
>+    : m_db(db)
>+    , m_writeBatch(LevelDBWriteBatch::create())
>+    , m_finished(false)
>+{
>+}
>+
>+LevelDBWriteOnlyTransaction::~LevelDBWriteOnlyTransaction()
>+{
>+    m_writeBatch->clear();
>+}
>+
>+bool LevelDBWriteOnlyTransaction::put(const LevelDBSlice& key, const Vector<char>& value)
>+{
>+    ASSERT(!m_finished);
>+    m_writeBatch->put(key, value);
>+    return true;
>+}
>+
>+bool LevelDBWriteOnlyTransaction::remove(const LevelDBSlice& key)
>+{
>+    ASSERT(!m_finished);
>+    m_writeBatch->remove(key);
>+    return true;
>+}
>+
>+bool LevelDBWriteOnlyTransaction::commit()
>+{
>+    ASSERT(!m_finished);
>+
>+    if (!m_db->write(*m_writeBatch))
>+        return false;
>+
>+    m_finished = true;
>+    m_writeBatch->clear();
>+    return true;
>+}
>+
>+void LevelDBWriteOnlyTransaction::rollback()
>+{
>+    ASSERT(!m_finished);
>+    m_finished = true;
>+    m_writeBatch->clear();
>+}
>+
> } // namespace WebCore
> 
> #endif // USE(LEVELDB)
>diff --git a/Source/WebCore/platform/leveldb/LevelDBTransaction.h b/Source/WebCore/platform/leveldb/LevelDBTransaction.h
>index 6d22850c2b7a8c280e39c674938b7968e74b3dd3..de93a059d3361e9bb7d5563192763d12511c021f 100644
>--- a/Source/WebCore/platform/leveldb/LevelDBTransaction.h
>+++ b/Source/WebCore/platform/leveldb/LevelDBTransaction.h
>@@ -43,6 +43,7 @@
> namespace WebCore {
> 
> class LevelDBDatabase;
>+class LevelDBWriteBatch;
> 
> using WTF::AVLTree;
> 
>@@ -169,6 +170,26 @@ private:
>     HashSet<TransactionIterator*> m_iterators;
> };
> 
>+class LevelDBWriteOnlyTransaction : public RefCounted<LevelDBWriteOnlyTransaction> {
>+public:
>+    static PassRefPtr<LevelDBWriteOnlyTransaction> create(LevelDBDatabase*);
>+
>+    ~LevelDBWriteOnlyTransaction();
>+    bool put(const LevelDBSlice& key, const Vector<char>& value);
>+    bool remove(const LevelDBSlice& key);
>+    bool commit();
>+    void rollback();
>+
>+private:
>+    LevelDBWriteOnlyTransaction(LevelDBDatabase*);
>+
>+    bool set(const LevelDBSlice& key, const Vector<char>& value, bool deleted);
>+
>+    LevelDBDatabase* m_db;
>+    OwnPtr<LevelDBWriteBatch> m_writeBatch;
>+    bool m_finished;
>+};
>+
> }
> 
> #endif // USE(LEVELDB)

Actions: View | Formatted Diff | Diff

Attachments on bug 96036: 162616 | 162826 | 163227