412758 - Erase functions in nsNavHistoryExpire iterate all history visits many times

Status: RESOLVED FIXED

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

Description

[Ondrej Brablc]

Bug 412438 fixed duplicate IDs in SQL queries, however, it can still be improved - at the moment we get an array of all visits in FindVisits. This array is later 5 times iterated to build different lists of IDs which are used to delete data.

It should be possible to iterate this array only once, actually it should be done at the moment when we are fetching the data from the database and all the lists should be produced here. We can probably go as far as not creating the array at all, we should only produce the lists.

However, it is questionable, whether we should do this, or avoid calling FindVisits at all as suggested in bug 409723.

Comment 1

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

yes, i've not changed all functions to use a different array because this has still to be investigated, probably the best thing would be replace or remove findvisits but if that's not possible we should do this. 

As you said the array could be changed to a simple PRInt64 array of place ids (checked only once) and passed it to clear functions insted of passing the array of results. 

At least we have mitigated the huge queries problem.

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

marking as perf, this is however not blocking imho

Comment 3

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

notice that it's true that visits are looped in each function, but it's also true that they are filtered differently (this filtering happens before query and dupes filtering, so it could improve query speed, or be totally useless, should have real numbers on this).

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

This will not block the final release of Firefox 3.

Comment 5

[Gervase Markham [:gerv]]

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

Comment 6

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

fixed with async expiration bug 520165