[webkit-reviews] review requested: [Bug 36251] Turn on AUTO_VACUUM for all HTML5 DBs : [Attachment 50988] patch

[bugzilla-daemon at webkit.org]

Dumitru Daniliuc <dumi at chromium.org> has asked	for review:
Bug 36251: Turn on AUTO_VACUUM for all HTML5 DBs
https://bugs.webkit.org/show_bug.cgi?id=36251

Attachment 50988: patch
https://bugs.webkit.org/attachment.cgi?id=50988&action=review

------- Additional Comments from Dumitru Daniliuc <dumi at chromium.org>
Changed the patch to turn on AUTO_VACUUM only for HTML5 DBs.

According to sqlite engineers, the overhead of AUTO_VACUUM = FULL should be
unnoticeable. Do you have a benchmark or a usage pattern that makes this
overhead significant on Mac OS X? I'll try to run some tests with AUTO_VACUUM =
FULL and AUTO_VACUUM = NONE and see what numbers I get. If we have to, we could
set AUTO_VACUUM = INCREMENTAL and call PRAGMA incremental_vacuum when
freelist_count > N, but I'd rather have sqlite take care of everything for us,
if possible.

As far as the VACUUM call goes, I don't think it's a problem. According to
Richard Hipp, when sqlite opens a database, it sets the AUTO_VACUUM flag to
whatever value it had when the DB was last updated. So for new databases, this
VACUUM call is pretty much a no-op. And for existing databases, we will run
VACUUM at most once per database, to "convert" it from AUTO_VACUUM = NONE to
AUTO_VACUUM = FULL. Taking into account that the default quota limit in Safari
and Chromium is set to 5MB right now, I don't expect any user to have databases
bigger than a few MB at this time. Running VACUUM on such a database will take
a couple of seconds, which I think is an acceptable one-time performance hit.