Closed Bug 334010 Opened 14 years ago Closed 11 years ago

Need a "Compact history database" button in options

Categories

(Firefox :: Bookmarks & History, defect, P3)

defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: brettw, Unassigned)

References

Details

Attachments

(2 files, 2 obsolete files)

In the options dialog under history, we should add a "compact history database" option. This will run a sqlite VACUUM command that will reclaim unused space in the database.

Most people won't use this but some people will really need it.
Priority: -- → P3
Target Milestone: --- → Firefox 2 beta1
http://caspar.regis.free.fr/various/Vacuum-0.1.tgz

might be way too use regis code in this
Attached patch patch v1 (proposal) (obsolete) — Splinter Review
This patch propose to add a new service named "Places DB Compacter" (contractID: "@mozilla.org/browser/places-compacter;1"), which allow 'bookmarks_history.sqlite' compaction using a VACUUM query. I added a tab to the "Advanced" panel of "Options" window named "Places" with a description into it and a button allowing DB compaction (will attach a screenshot). 

What need to be discussed is:
 - Where should GUI things go (another place?)
 - Are the strings used clear
 - Are licenses boiler-plates correct
Attachment #223686 - Flags: review?(brettw)
Attached image Screenshot ("Options" window / patch v1) (obsolete) —
Isn't this something that should just happen on a regular basis anyway or is there some massive hit to this?
(In reply to comment #4)
> Isn't this something that should just happen on a regular basis anyway or is
> there some massive hit to this?
This has been discussed on http://forums.mozillazine.org/viewtopic.php?t=421177 and the problem is the time it can consume when the DB is large.

Target Milestone: Firefox 2 beta1 → Firefox 3
I think a button to do this is a very bad idea. First it is UI clutter since it won't be used by a lot of people. And probebly won't be understood by a lot of the general public either. Another issue with it is that it shows that there are some things clearly wrong with the way Firefox uses it's dbase. This may be true for now but it should never be shown to the customers; it's like having a button that says "Click here to clean up our quick and dirty code".

So I propose something like a periodic checkup, wheter this is based upon the sqlite filesize or simply by a given interval of perhaps a week is debatable. 

Alternatively if periodics are a great hit on performance or cause trouble otherwise an boolean entry like "browser.cleanup_sqlite.onstartup" in about:config could also be concidered
(In reply to comment #6)
> I think a button to do this is a very bad idea. First it is UI clutter since it
> won't be used by a lot of people. And probebly won't be understood by a lot of
> the general public either. Another issue with it is that it shows that there
> are some things clearly wrong with the way Firefox uses it's dbase. This may be
> true for now but it should never be shown to the customers; it's like having a
> button that says "Click here to clean up our quick and dirty code".
> 
> So I propose something like a periodic checkup, wheter this is based upon the
> sqlite filesize or simply by a given interval of perhaps a week is debatable. 
> 
> Alternatively if periodics are a great hit on performance or cause trouble
> otherwise an boolean entry like "browser.cleanup_sqlite.onstartup" in
> about:config could also be concidered
> 

The problem with an automatic vacuum is as Regis stated in the thread, is that no matter if you run the vacuum right after a previous vacuum, it still takes at least 30 seconds on a very small database. So this eliminated auto vacuum on startup or any interval. It has to be a user initiated command. And I for one am for it. 

Thunderbird also has a manual cleanup feature for deleting emails from their database. If the user deletes email it isn't actually deleted, just basically hidden from the user until the compact folder command is initiated which also takes some time to do.
(In reply to comment #6)
>I think a button to do this is a very bad idea. First it is UI clutter since it
>won't be used by a lot of people. And probebly won't be understood by a lot of
>the general public either.
Perhaps more explanation or a link to an explanation page would be nice.

> Another issue with it is that it shows that there are some things clearly 
> wrong with the way Firefox uses it's dbase. This may be true for now but it
> should never be shown to the customers; it's like having a button that says 
> "Click here to clean up our quick and dirty code".
There is no bad code involved here, you should read http://developer.mozilla.org/en/docs/Storage:Performance#Vacuuming_and_zero-fill and http://www.sqlite.org/lang_vacuum.html. By analogy, we could compare the way sqlite file works to a filesystem (lets say NTFS) and no one is shocked when viewing the defrag tools in WinNT in the Start Menu. 

An alternative to this vacuum thing could be to set mozilla to use auto-vacuum (http://www.sqlite.org/pragma.html#pragma_auto_vacuum) but I don't know if this is compatible with SQLITE_SECURE_DELETE.

Additionally, I thought a little about this pane my patch add and here are some more possible enhancements:
 - Mad_Overclocker on f.m.o asked me for a way to clean favicons from the DB (for my vacuum external program), perhaps a button for that could be nice.
 - Perhaps a way to the list see all keyword already set (button that open a popup dialog ala search engine manager)

>So I propose something like a periodic checkup, wheter this is based upon the
>sqlite filesize or simply by a given interval of perhaps a week is debatable. 
>Alternatively if periodics are a great hit on performance or cause trouble
>otherwise an boolean entry like "browser.cleanup_sqlite.onstartup" in
>about:config could also be concidered
As already said on f.m.o, I don't think this is the best way, personally, I don't want neither my startup to be slowed nor some non responsive browser once a week. 
AFAIK, the actual mozStorage code lock the DB and "vacuum'ing" needs to remove this lock, this leads to no usability (?) of the browser during the vacuum.

In conclusion, if the button is not the chosen way, I think auto-vacuum should be the way (if possible)
From the sqlite manual, space left from deleted items is reusable. So while vacuuming will free up space, it's only space that may be used up by more history items. Assuming you keep about the same number of history items and bookmarks I guess the size of the database should end up remaining more or less the same.
I was thinking it could go in the history tab; there is already plenty of room. We can always change it later.

I doubt we're going to surface the string "Places" in the UI much, it will be confusing to many users. I would suggest just "Compact history database".

Personally, having a new interface seems like overkill to me. I suggest that you just add it to nsINavHistory and to the same file. This service has all the "shared" places functions already, for example, for querying.

We need some UI to go along with compaction becuase it can take a really long time. On my networked home directory at work, my <10MB file took over a minute. Most users won't see this.

At least for now, we should pop up a dialog box with something like "This function will remove unused spaces from your history file. It may take over one minute to complete." [OK] [Cancel]

There is some chance I'll move all the history DB functions to another thread, in which case we can make some UI to actually go along with compaction.
Comment on attachment 223686 [details] [diff] [review]
patch v1 (proposal)

(In reply to comment #10)
> I was thinking it could go in the history tab; there is already plenty of room.
> We can always change it later.
I didn't put it here because the history tab is under the privacy panel and compacting database has nothing to do with privacy. (And this compact bookmarks too..)

> I doubt we're going to surface the string "Places" in the UI much, it will be
> confusing to many users. I would suggest just "Compact history database".
OK

> Personally, having a new interface seems like overkill to me. I suggest that
> you just add it to nsINavHistory and to the same file. This service has all the
> "shared" places functions already, for example, for querying.
Will do that
 
> At least for now, we should pop up a dialog box with something like "This
> function will remove unused spaces from your history file. It may take over one
> minute to complete." [OK] [Cancel]
Will add that in the JS part of the code, with a checkbox to disable the prompt :)

> There is some chance I'll move all the history DB functions to another thread,
> in which case we can make some UI to actually go along with compaction.
Could makes UI more consistent. 

Will resubmit another patch quickly taking into account your comments, thanks.
Attachment #223686 - Flags: review?(brettw)
Attached patch patch v2Splinter Review
patch v2: Moved the button into Privacy->History, moved the c++ part into nsNavHistory, added a prompt (see comments 10-11)

Will attach a new screenshot.
Attachment #223686 - Attachment is obsolete: true
Attachment #223687 - Attachment is obsolete: true
Attachment #223713 - Flags: review?(brettw)
Compact History Database is not something users will understand/care about, and its surfacing an implementation detail, which we should never do.

Comment 9 suggests, at least to me, that unused space will be minimal.  We should detect that this cleanup needs to happen, and directly prompt the user on shutdown once a week, instead of exposing a button for this case.  This is UI that should never ship in a final product.
Just for the records: http://web.utk.edu/~jplyon/sqlite/SQLite_optimization_FAQ.html#compact

(In reply to comment #14)
> We should detect that this cleanup needs to happen, and directly prompt the 
> user on shutdown once a week, instead of exposing a button for this case.  
> This is UI that should never ship in a final product.
The important things here is to have a way to do the vacuum. I personally prefer the button, mainly because it doesn't bother me once a week :) but I perfectly understand that this could perturb end-users. 
Why not adding another pref. to hide this button? This way people needing to do the compaction can set a pref. and then use the button, and others don't see anything.

I think we need to have this fuction somewhere. If you clear your history, the file won't get any smaller. This will freak some people out (not that it's reasonable). It will also be important in some cases. For example, some people have 100MB histories. Without this function, this means their file can never go below 100MB, no matter what changes they make to their settings.

The free space in the database will also get increasingly fragmented. When something is deleted, placing something new probably isn't the same size. We have a lot of churn in the history database as things get added and expired. The result is a file that grows slowly, even if you have the same number of history entries. 

I'm not happy about having this button. I would like to do this automatically when you clear your history and perhaps "from time to time" automatically to flush out pockets of unused space. However, the performance of this operation prevents this.

Thunderbird exposes "compact mailbox".
(In reply to comment #16)
> I'm not happy about having this button. I would like to do this automatically
> when you clear your history and perhaps "from time to time" automatically to
> flush out pockets of unused space. However, the performance of this operation
> prevents this.

Do it once every three shutdowns? Detect idle time and do it then? There are ways to get around that performance hit, I'd think, and I agree that it's better done automatically (since it's really a DB maintenance task) than explicitly.
On my system (networked home directory) it has taken well over a minute to do this for only a 10MB database. Most computers will be much faster, but we're still talking about a many second operation, and the people wityh 100MB histories on networked drives will be waiting many minutes,

I'm not sure what the bottleneck is. Perhaps it's easy to make faster.

Given the shutdown problem places has when expiring history, I think people would not tolerate vacuuming on shutdown.
(In reply to comment #16)
> The free space in the database will also get increasingly fragmented. When
> something is deleted, placing something new probably isn't the same size. We
> have a lot of churn in the history database as things get added and expired.
> The result is a file that grows slowly, even if you have the same number of
> history entries. 

I'm making a couple of assumptions about how sqlite splits things up, but I'd guess that since we're not storing anything particularly large in the database (apart from favicons I guess) the fragmentation becomes less of an issue.

I'd be interested to see what sort of win in the file size we're expecting here. My database thats been in active use for basically since places became usable shrunk from 14.7MB to 13.5MB in about 5 seconds. I wouldn't particularly consider that a worthwhile win.
Regis: I forgot, you need to update the IID of the interface. I wouldn't bother now while we see whether we want to do this or not.
(In reply to comment #19)
> I'd be interested to see what sort of win in the file size we're expecting
> here. My database thats been in active use for basically since places became
> usable shrunk from 14.7MB to 13.5MB in about 5 seconds. I wouldn't particularly
> consider that a worthwhile win.

I've definitely seen it do a lot more than that. I think the main application is to do it after you clear your history. Maybe we should consider compacting it when you clear history always, and not worry about the extra time.
(In reply to comment #21)
> I've definitely seen it do a lot more than that. I think the main application
> is to do it after you clear your history. Maybe we should consider compacting
> it when you clear history always, and not worry about the extra time.

Sounds good to me.
(In reply to comment #22)
> (In reply to comment #21)
> > I've definitely seen it do a lot more than that. I think the main application
> > is to do it after you clear your history. Maybe we should consider compacting
> > it when you clear history always, and not worry about the extra time.
> 
> Sounds good to me.
> 

For what it's worth, I second that.
As soon as there is an XPCOM method to do it, I'm happy :)
it'd be cool to have a utility method that opened a small window with a string and a progressmeter/activity indicator that took a string of your choice.  i.e.

___________________________________________
| Please Wait                            X|
-------------------------------------------
| Firefox is clearing your history, this  |
| may take a few minutes                  |
|                                         |
| [||||||||||||_________________________] |
|                                         |
-------------------------------------------

Would be the right thing to display for the history clearing action.  We should absolutely compact on clear history (maybe a hidden bool pref to disable this, for the type of people who'd use an extension to do this).
That utility method would be cool, but does compacting the database block the UI, or merely slow down Firefox and cause the disk to spin? If the latter, I'd rather that status indicator only appear as a dialog when the user requests to close down the browser in the middle of the operation, and hopefully we could have a cancel command on it that would immediately stop and backout.

(cc me on the bug for the utility method, if you file it)
(In reply to comment #8)
> An alternative to this vacuum thing could be to set mozilla to use auto-vacuum
> (http://www.sqlite.org/pragma.html#pragma_auto_vacuum) but I don't know if this
> is compatible with SQLITE_SECURE_DELETE.
I'm currently using an auto-vacuum enabled db on my daily-used profile. It works nicely (for now). I don't know all the places features, so I can't say if absolutely everything is fine, but it looks like it is.
Here's a way to enable auto-vacuum using sqlite3 shell:
$ mv bookmarks_history.sqlite _bookmarks_history.sqlite
$ sqlite3 bookmarks_history.sqlite
sqlite> .out bm.sql
sqlite> .dump
sqlite> .quit
$ sqlite3 db.sqlite
sqlite> PRAGMA auto_vacuum = 1;
sqlite> .read bm.sql
sqlite> .quit
$ rm bookmarks_history.sqlite
$ mv db.sqlite bookmarks_history.sqlite

What do you think about that?
I noticed auto vacuuming was pretty slow, and would only make our shutdown problems worse. We do not want to vacuum on shutdown, although it might be worth getting hard numbers to see how much of a difference it makes.
(In reply to comment #28)
> I noticed auto vacuuming was pretty slow, and would only make our shutdown
> problems worse. We do not want to vacuum on shutdown, although it might be
> worth getting hard numbers to see how much of a difference it makes.
Brett, actually, my shutdown is faster with auto-vacuum set. And according to the sqlite page, auto-vacuum pragma makes "the database file shrinks when a transaction that deletes data is committed". Is there any deletion on close?



Things are expired from history. Under typical usage, we don't do a whole lot of expiration at shutdown (it happens at runtime because we were having so many problems with shutdown speed) but it does some paranoid cleanups at this time.
(In reply to comment #30)
> Things are expired from history. Under typical usage, we don't do a whole lot
> of expiration at shutdown (it happens at runtime because we were having so many
> problems with shutdown speed) but it does some paranoid cleanups at this time.
Thanks for your answer. This explain why I didn't see any effects (no history entries should have expired between my Firefox start).
What could be possible if history entries are stored with a full time (I mean something that include an hour) would be to do the history paranoid cleanup at runtime every hour. The auto-vacuum makes no need to vacuum and the deletion (i.e. the cleanup) would only concern a few entries.
Attachment #223713 - Flags: review?(brettw)
Target Milestone: Firefox 3 → ---
Marking WONTFIX. Fx3 was released, and haven't seen major issues that would be fixed by a vacuum.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WONTFIX
Duplicate of this bug: 403702
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.