Bug 38972 – Do not check if a database needs to be vacuumed after read-only transactions

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dumitru Daniliuc]

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.

[Dumitru Daniliuc]

Created attachment 55809 [details]
patch

[Dimitri Glazkov (Google)]

Comment on attachment 55809 [details]
patch

ok.

[Michael Nordman]

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);
    }

[Dumitru Daniliuc]

(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.

[Dumitru Daniliuc]

Landed as r59267.