38972 2010-05-11 22:32:14 -0700 Do not check if a database needs to be vacuumed after read-only transactions 2010-05-12 14:08:43 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 dumi dumi dglazkov ericu michaeln oldest_to_newest 224417 0 dumi 2010-05-11 22:32:14 -0700 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. 224424 1 55809 dumi 2010-05-11 22:53:35 -0700 Created attachment 55809 patch 224648 2 55809 dglazkov 2010-05-12 08:46:39 -0700 Comment on attachment 55809 patch ok. 224763 3 michaeln 2010-05-12 11:39:18 -0700 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); } 224857 4 dumi 2010-05-12 13:43:10 -0700 (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. 224876 5 dumi 2010-05-12 14:08:43 -0700 Landed as r59267. 55809 2010-05-11 22:53:35 -0700 2010-05-12 08:46:38 -0700 patch patch text/plain 1421 dumi SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1OTIwOSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTMgQEAKKzIwMTAtMDUtMTEgIER1bWl0cnUgRGFuaWxpdWMgIDxkdW1pQGNocm9t aXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBEbyBub3QgY2hlY2sgaWYgYSBkYXRhYmFzZSBuZWVkcyB0byBiZSB2YWN1dW1lZCBhZnRlciBh IHJlYWQtb25seSB0cmFuc2FjdGlvbi4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcv c2hvd19idWcuY2dpP2lkPTM4OTcyCisKKyAgICAgICAgKiBzdG9yYWdlL1NRTFRyYW5zYWN0aW9u LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlNRTFRyYW5zYWN0aW9uOjpwb3N0ZmxpZ2h0QW5kQ29t bWl0KToKKwogMjAxMC0wNS0xMSAgVG9ueSBDaGFuZyAgPHRvbnlAY2hyb21pdW0ub3JnPgogCiAg ICAgICAgIFJldmlld2VkIGJ5IERhcmluIEZpc2hlci4KSW5kZXg6IFdlYkNvcmUvc3RvcmFnZS9T UUxUcmFuc2FjdGlvbi5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9zdG9yYWdlL1NRTFRyYW5z YWN0aW9uLmNwcAkocmV2aXNpb24gNTkyMDQpCisrKyBXZWJDb3JlL3N0b3JhZ2UvU1FMVHJhbnNh Y3Rpb24uY3BwCSh3b3JraW5nIGNvcHkpCkBAIC00NjYsOCArNDY2LDExIEBAIHZvaWQgU1FMVHJh bnNhY3Rpb246OnBvc3RmbGlnaHRBbmRDb21taXQKICAgICAgICAgcmV0dXJuOwogICAgIH0KIAot ICAgIC8vIFRoZSBjb21taXQgd2FzIHN1Y2Nlc3NmdWwsIHNvIHZhY3V1bSB0aGUgZGF0YWJhc2Ug aWYgbmVlZGVkCi0gICAgbV9kYXRhYmFzZS0+aW5jcmVtZW50YWxWYWN1dW1JZk5lZWRlZCgpOwor ICAgIC8vIFRoZSBjb21taXQgd2FzIHN1Y2Nlc3NmdWwsIHNvIHZhY3V1bSB0aGUgZGF0YWJhc2Ug aWYgbmVlZGVkLgorICAgIC8vIFdlIGRvbid0IG5lZWQgdG8gZG8gYW55dGhpbmcgZm9yIHJlYWQt b25seSB0cmFuc2FjdGlvbnMsCisgICAgLy8gYmVjYXVzZSB0aGV5IGNhbid0IGNoYW5nZSB0aGUg c3RhdGUgb2YgdGhlIGRhdGFiYXNlLgorICAgIGlmICghbV9yZWFkT25seSkKKyAgICAgICAgbV9k YXRhYmFzZS0+aW5jcmVtZW50YWxWYWN1dW1JZk5lZWRlZCgpOwogCiAgICAgLy8gVGhlIGNvbW1p dCB3YXMgc3VjY2Vzc2Z1bCwgbm90aWZ5IHRoZSBkZWxlZ2F0ZXMgaWYgdGhlIHRyYW5zYWN0aW9u IG1vZGlmaWVkIHRoaXMgZGF0YWJhc2UKICAgICBpZiAobV9tb2RpZmllZERhdGFiYXNlKQo=