Bug 38972 - Do not check if a database needs to be vacuumed after read-only transactions
Summary: Do not check if a database needs to be vacuumed after read-only transactions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Dumitru Daniliuc
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-11 22:32 PDT by Dumitru Daniliuc
Modified: 2010-05-12 14:08 PDT (History)
3 users (show)

See Also:


Attachments
patch (1.39 KB, patch)
2010-05-11 22:53 PDT, Dumitru Daniliuc
dglazkov: review+
dumi: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dumitru Daniliuc 2010-05-11 22:32:14 PDT
Read-only transactions don't change the state of the database. Therefore, there's no point in checking if a database needs to be vacuumed after such a transaction.

Checking if a database needs to be vacuumed involves calling 'PRAGMA freelist_count' and 'PRAGMA max_page_count', which adds an unnecessary overhead.
Comment 1 Dumitru Daniliuc 2010-05-11 22:53:35 PDT
Created attachment 55809 [details]
patch
Comment 2 Dimitri Glazkov (Google) 2010-05-12 08:46:39 PDT
Comment on attachment 55809 [details]
patch

ok.
Comment 3 Michael Nordman 2010-05-12 11:39:18 PDT
Does this put a dent in the perf regression? Just checking.

Consider putting the call to vacuumIfNeeded() in the block that runs for transactions that actually mutated something to limit how often we do this function even further (ala, don't do it for 'write' transactions that didn't really write anything).

    // The commit was successful, notify the delegates if the transaction modified this database
    if (m_modifiedDatabase) {
        m_database->incrementalVacuumIfNeeded();
        m_database->transactionClient()->didCommitTransaction(this);
    }
Comment 4 Dumitru Daniliuc 2010-05-12 13:43:10 PDT
(In reply to comment #3)
> Does this put a dent in the perf regression? Just checking.
> 
> Consider putting the call to vacuumIfNeeded() in the block that runs for transactions that actually mutated something to limit how often we do this function even further (ala, don't do it for 'write' transactions that didn't really write anything).
> 
>     // The commit was successful, notify the delegates if the transaction modified this database
>     if (m_modifiedDatabase) {
>         m_database->incrementalVacuumIfNeeded();
>         m_database->transactionClient()->didCommitTransaction(this);
>     }

done.
Comment 5 Dumitru Daniliuc 2010-05-12 14:08:43 PDT
Landed as r59267.