Closed
Bug 403702
Opened 17 years ago
Closed 15 years ago
add "Compact" to "Organize" menu in the bookmarks organizer that does VACUUM (with progress)
Categories
(Firefox :: Bookmarks & History, enhancement)
Firefox
Bookmarks & History
Tracking
()
RESOLVED
DUPLICATE
of bug 334010
People
(Reporter: moco, Unassigned)
References
Details
add "Compact" to "Organize" menu in the bookmarks organizer that does VACUUM (with progress)
as discussed repeately (see bug #395020 and bug #394379), we aren't doing VACUUM yet as it blocks the UI.
additionally, while vacuuming defrags the database, sqlite is good at reusing the freed page list, and I claim the typical user won't see / won't care about the size of the places.sqlite file on disk.
but, that said, we might find ourselves needed to recommend vacuum for users after we ship firefox 3.
note, we'd need a progress window for this (see bug #329736) as it can block the UI for a long period of time.
an irc discussion about this:
sspitzerMsgMe I just makred a vacuum bug from p1/m10 to p3/m11
dietrich yep, was just about to comment
dietrich that's fine, given our new understanding of vacuum
sspitzerMsgMe you should have gotten bug email. given what we now know
sspitzerMsgMe right
sspitzerMsgMe ok, coolness
dietrich we really should get the "vacuum on upgrade" done though
sspitzerMsgMe (glad we are on the same page)
sspitzerMsgMe why do you think that is critical?
dietrich as well as the move those inc. vacuum folks off of that
sspitzerMsgMe well, keep in mind, that won't work.
sspitzerMsgMe unless we fix vacuum
sspitzerMsgMe to allow resetting of pragmas
dietrich right, drh's suggestion of change pragma
dietrich here's my thoughts: the most vocal and prolific testers we have all have inc. vacuum
sspitzerMsgMe so, we'd need that first. and keep in mind, we'd have to vacuum all profiles (or do you mean, vacuum after first run after upgrade)
dietrich and they'll continually have probs
sspitzerMsgMe keep in mind, they might be configured for incremental vacuum, but we are no longer doing the incremental vacuum
sspitzerMsgMe (we were not set up for auto)
sspitzerMsgMe so they have databases configured for it, but we don't do it.
dietrich oh right, regular vacuum will still work
sspitzerMsgMe (anymore, on idle)
dietrich we don't need to un-inc.vacuum them
sspitzerMsgMe right.
dietrich ok, maybe my concern is unfounded.
dietrich i worry that ongoing usage will increase fragmentation to a point where vacuum will take so long that we don't want to do it
dietrich and then there'll be really popular extensions for vacuuming
dietrich :P
sspitzerMsgMe I think we discussed this:
sspitzerMsgMe some sort of file | compact
sspitzerMsgMe in the organizer
sspitzerMsgMe once we get prgress working
dietrich yes we did
dietrich maybe we should morph that bug
dietrich to cover that
sspitzerMsgMe or should I start a new one?
dietrich yes, that's a better plan
sspitzerMsgMe ok, doing that now.
sspitzerMsgMe also:
sspitzerMsgMe if we had a vacuum that could change pragmas, and if we had progress ui, we could do a vacuum on startup (do a schema bump). I don't think we want to add this code to software update / installer.
sspitzerMsgMe multiple profiles on a machine or multiple profiles for a given user makes this very messy.
sspitzerMsgMe keep in mind, all we are gaining is a smaller places.sqlite file on disk.
sspitzerMsgMe (and sqlite should be re-using freed pages)
sspitzerMsgMe I guess, there is a price to be paid if you have a place.sqlite site up for incremental vacuum
sspitzerMsgMe dr. hipp tells me there is some overhead
dietrich yes, and new users should already have that smaller places.sqlite
sspitzerMsgMe but he made it sound like it was minimal.
sspitzerMsgMe as long as you don't do an incremental vacuum
dietrich hm
sspitzerMsgMe (or have auto enabled, and we don't)
sspitzerMsgMe I think we should log the bug about "file | compact" and punt on vacuum. I could defniitely ship b2 even rtm without vacuum.
dietrich ok, i think we should change bug 395020 to be only about single-profile-vacuum-on-upgrade, as you just described
dietrich and block it on the progress UI bug
sspitzerMsgMe ok, will do
sspitzerMsgMe I'll post this chat log
sspitzerMsgMe in the bug as well
sspitzerMsgMe for reference.
sspitzerMsgMe ok?
dietrich right, new bug for vacuum on organizer
sspitzerMsgMe right, will do that too.
dietrich change bug 395020 and recommend de-blockerizing it
Comment 1•17 years ago
|
||
I believe you get more than just a smaller file - since it completely rebuilds you should have a very unfragemented file - which may over the long haul impact perf. I know it does on postgress if you don't vacuum.
I don't think we need a progress UI at all .. just a spinning "i'm working" update with a cancel button would do it for me. There are plenty of progress dialogs (like the updater dialog) that are so non-linear that they don't provide much value at all.
So anyway - giving users an easy way to vacuum with a simple cancel button seems like a big + easy win to me...
Reporter | ||
Comment 2•17 years ago
|
||
> I believe you get more than just a smaller file
yes, you'd get a smaller file.
as marco mentions in another bug: "since sqlite uses freelists to speed up inserts", so that's something to keep in mind.
> I don't think we need a progress UI at all .. just a spinning "i'm working" update with a cancel button
> would do it for me.
that's the sort of progress UI I'm talking about.
We won't know how long vacuum will take (dr hipp had some ideas on how to "guess", but he quickly pointed out the flaws with the suggestion). But, we can get called back on every <n> SQLite virtual machine opcodes to prevent starving the UI thread (and to process the "cancel".
> So anyway - giving users an easy way to vacuum with a simple cancel button
> seems like a big + easy win to me...
not sure how easy it would be, but I agree, I think this could pay dividends later, if we ever realize that performance problem xyz has workaround: vacuum.
(note, if you google for vacuum and Mail.app, you'll see this sort of thing has happened before.)
Comment 3•17 years ago
|
||
I'm not a big fan of adding UI to let users work around bugs :/
Can we estimate the amount of space that will be saved, and either show that in the UI or to decide when to vacuum automatically?
Can we fix SQLite so that it can vacuum (or do something very similar to vacuuming) without blocking? Or fix its "incremental vacuum" feature to not suck?
Why only bookmarks and not our other databases?
See also bug 205756, "Compact folders should be enabled by default" for mail.
Comment 4•17 years ago
|
||
some note:
vacuum time is not connected to how much the file is fragmented.
Vacuum takes its time based on the size of the DB and the number of indexes on it, since everything it does is creating a new db, attaching it to the current connection, moving everything to it, then replacing the old db with the new. If you defragment non fragmented db it will take "about" the same time.
places.sqlite is the worst fragmentation case because in moz_places and moz_history and moz_favicons there will be a lot of inserts and deletes, while in other DBs they will be a limited amount. Also since places.sqlite contains a high number of tables with uris, titles, and favicons it is big, and it is a better target for vacuuming. but it should not be done too frequently, sqlite does already a good work with freelist.
Imho the idea to do a vacuum on upgrade is the best since the user is ready to wait some minute, but surely with a progress bar!
Definetely all files will need a vacuum sometime in the future, only it's not now, could be in a year or two, since they are small files it will take a few seconds on each...
There is some way to guess the fragmentation percentage, the tool
POPFile SQLite Database Analyser Utility (http://www.sugelan.co.uk/popfile/utilities.html#analysesqlite) makes some move toward this, it can count orphaned pages, freelists and fragmentation into a db, unfortunately it's not open source, so don't know if it uses sqlite_analize or raw access.
Some similar way could be achieved, but the best thing would be sqlite itself provide a function to guess the fragmentation of a table or a db, since it probably requires raw access to the file contents, layers and pages.
Comment 5•17 years ago
|
||
I don't like the idea of automatically vacuuming as part of the upgrade process.
* Security updates are important, and I don't want users to put them off because they're slow. The fact that the users already have to wait a minute during Firefox updates is a bug, IMO; see bug 307181.
* Performance shouldn't rely on updates being regular. There's currently no guarantee that they'll be regular.
Comment 6•17 years ago
|
||
humm dupe of Bug 334010?
Comment 7•15 years ago
|
||
i'm duping, we are handling vacuum different way btw (bug 512854)
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → DUPLICATE
Comment 8•15 years ago
|
||
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".
In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body contains places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.
Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.
Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in
before you can comment on or make changes to this bug.
Description
•