ExpireItems(EXPIRATION_COUNT_PER_RUN) takes about 700ms (down from 8 seconds) but could be faster? note, this is with the dreaded ispiked places.sqlite. after marco's fix for bug #407124, ExpireItems() is down to about 700ms. FindVisits() takes between 0 - 15ms. I haven't figured out where the 700ms is coming from. note, we call this code after visiting a page: upon visiting a page, we'll call nsNavHistoryExpire::OnAddURI() > places.dll!nsNavHistoryExpire::OnAddURI(__int64 aNow=1196930465625000) Line 161 C++ places.dll!nsNavHistory::AddURI(nsIURI * aURI=0x08c0a538, int aRedirect=0, int aToplevel=1, nsIURI * aReferrer=0x00000000) Line 3097 C++ docshell.dll!nsDocShell::AddToGlobalHistory(nsIURI * aURI=0x08c0a538, int aRedirect=0, nsIChannel * aChannel=0x08b41be0) Line 8586 + 0x40 bytes C++ that kicks of the expire timer, and when that timer fires, we will call DoPartialExpiration(), which will call ExpireItems() which will call FindVisits()

i think that something can be done in eraseXXX functions to have faster queries... will post a patch with some modified query to test out

these queries perform faster from my tests, notice that this will not however be a great perf win, because with deletes most of the time is taken by disk access... I think that these will however be faster than previous on big histories, because using joins and a where we limit on the actual IDs that we want to investigate(delete) on, and then apply to them other options. Previously we were instead filtering IDs against other non filtered queries. Seth or Dietrich, if you can get some time, try to make some testing on these please

Would it be possible to precompile those queries and bind aNumToExpire when we're going to execute ? We would need 2 versions though, 1 with and 1 without the where-clause.

i'm not sure if precompiling will gain something but could be something to try out as a second step, before moving on we need to know if queries are better. i think we should wait for Seth having the time to test patch on ispiked profile.

against ispiked's places.sqlite: unpatched: EraseHistory(): 669ms EraseHistory(): 961ms (shutdown) patched: EraseHistory(): 60ms EraseHistory(): 179ms (shutdown)

Comment on attachment 292181 [details] [diff] [review] first attempt to better queries >Index: toolkit/components/places/src/nsNavHistoryExpire.cpp >=================================================================== >RCS file: /cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryExpire.cpp,v >retrieving revision 1.30 >diff -u -8 -p -r1.30 nsNavHistoryExpire.cpp >--- toolkit/components/places/src/nsNavHistoryExpire.cpp 7 Dec 2007 23:13:52 -0000 1.30 >+++ toolkit/components/places/src/nsNavHistoryExpire.cpp 8 Dec 2007 03:11:17 -0000 >@@ -426,17 +426,17 @@ nsNavHistoryExpireRecord::nsNavHistoryEx > nsresult > nsNavHistoryExpire::FindVisits(PRTime aExpireThreshold, PRUint32 aNumToExpire, > mozIStorageConnection* aConnection, > nsTArray<nsNavHistoryExpireRecord>& aRecords) > { > // Select moz_places records, including whether visited and whether the URI > // is bookmarked or not. > nsCAutoString sqlBase; >- sqlBase.AssignLiteral( >+ sqlBase.AssignLiteral( thanks marco, looks good except for this whitespace hunk. r=me.

Comment on attachment 292181 [details] [diff] [review] first attempt to better queries drivers: this patch makes an oft-hit expiration query 90% faster.

a note about the favicon query change: while the change looks ok, i don't think we have any test coverage of favicon expiration. we should add some for this bug.

fixed space, bringing over dietrich's review

Comment on attachment 293112 [details] [diff] [review] fixed trailing spaces hells_yes=beltzner

Checking in toolkit/components/places/src/nsNavHistoryExpire.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryExpire.cpp,v <-- nsNavHistoryExpire.cpp new revision: 1.32; previous revision: 1.31 done

Trunk now hangs for more than 3 minutes after loading first page.

(In reply to comment #13) > Trunk now hangs for more than 3 minutes after loading first page. > is that with your 200 MB history file from bug 407782 ?

Yes.

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