Closed Bug 416988 Opened 16 years ago Closed 15 years ago

Backup of bookmarks to HTML on shutdown does too many queries

Categories

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

defect

Tracking

()

RESOLVED INVALID

People

(Reporter: ondrej, Unassigned)

References

Details

(Keywords: perf)

During implementation of bug 385245 with logging of SQL statements turned on I have see thousands of queries to be performed on shutdown. This was slowing Firefox considerably. I have found that there are 3 to 4 queries made for each bookmark when saving it to the backup file:

- Get favicon (mDBGetURL)
- Get keyword (mDBGetKeywordForBookmark)
- Has annotation (mDBGetAnnotationFromItemId) 
- Get annotation (mDBGetAnnotationFromItemId)

With very low effort it should be possible to avoid the duplicated call to mDBGetAnnotationFromItemId (one to check whether data is there and another to read the data). This means 3 queries per bookmark. Bookmarks are written recursively, this means that it should be possible to read the keywords, annotations and favicons from database for all items inside a folder and store them in a folder level cache.

This means reduction from:
  3*(select count(*) from moz_bookmarks)

To:
  3*(select count(distinct parent) from moz_bookmarks)

With my real life bookmarks this means reduction down to 17% of the original number of queries. We could reduce it even down to 3 queries, but this would require to load all the favicons, keywords and annotations to memory. If we knew that the average number of bookmarks is not big, it would be worth doing. However, we should then decide about the level of the cache based on the number of bookmarks.
Flags: blocking-firefox3?
See also bug 415201.
OS: Windows XP → All
Hardware: PC → All
Yes, the annotation fix is easy, and we should try that for beta 4.

However, the others are more complicated. I don't think that the large changes required to get all bookmark properties in a single query are worth attempting in the beta 4 cycle, given that bug 415201 should alleviate a large part of the problem.

If we have further problems with export perf after that bug lands, then we should explore the "all at once" strategy.
Priority: -- → P3
Blocks: 418166
(In reply to comment #2)
> Does the problem also occur without any extensions involved?
> 

After doing a lot of testing testing, I see there is a correlation between the number (not quality) of extensions installed (plus number of tabs) in the closing delay of places.sqlite.  I have additional data points, but I think this is enough to show the pattern:

1. 54 extensions enabled, session resuming disabled: 6.8 sec shutdown (3.2 sec for places.sqlite)
2. All but default extension disabled: 3 sec (0.7 sec for places.sqlite)
3. 3 extensions enabled: 3.2 sec (1.1 sec for places.sqlite)
4. 11 extensions enabled: 4.2 sec (1.5 sec for places.sqlite)
5. 30 extensions enabled: 5.3 sec (2.5 sec for places.sqlite)
6. 50 extensions enabled: 6.4 sec (3.7 sec for places.sqlite)
7. All 54 extensions reenabled: 6.4 sec (3.4 sec for places.sqlite)
8. Reenabling session resuming did not make any meaningful difference
9. 54 extensions, resuming enabled, two windows, 16 tabs: 8.8 sec (4.8 sec for places.sqlite)
10. 54 extensions, no resuming, 5 windows, not tabs: 8.5 sec (4.8 sec for places.sqlite)
11. Per #10, using browser a bit then shutting down before typing this line: 11.3 sec (7.5 sec for places.sqlite)
12. Repeat shutdown shortly after: 9.1 sec (5 sec for places.sqlite)
 
OOOPS!!  Sorry.  Posted to wrong bug. :-(  This is for Bug 418166
Not blocking as per comment 2; Dietrich, renom if bug 415201 didn't do everything you were hoping, perf-wise.
Flags: wanted-firefox3+
Flags: blocking-firefox3?
Flags: blocking-firefox3-
Depends on: 420078
Assignee: ondrej → nobody
No longer blocks: 418166
We no longer export HTML on shutdown -> INVALID.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → INVALID
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.