improve perf of the "most visited pages" pre-defined query for bug #385397, I added code to make "place:type=0&sort=4&maxResults=10" fast for large histories. for bug #387996, I think we could do a similar trick (but for sort == 8, or visit count descending)

note, with a large amount of history, opening this folder takes several seconds.

in #testday, this comment indicates that this bug has been mitigated: "Coce: The problem I had last time (freezing when opening some items in the "places" folder) has disappeared" likely a result of the massive history pruning implemented last week.

for place:queryType=0&sort=8&maxResults=10, the current query we will execute is: SELECT h.id, h.url, h.title, h.rev_host, h.visit_count, MAX(visit_date), f.url, null, null FROM moz_places h LEFT OUTER JOIN moz_historyvisits v ON h.id = v.place_id LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id WHERE (h.hidden <> 1 AND v.visit_type <> 4 AND v.visit_type <> 0 ) GROUP BY h.id ORDER BY 5 DESC LIMIT 10

a couple thoughts: if we trust h.hidden, we would not need to join against moz_historyvisits, because the menu doesn't use the visit_date: we could do something like SELECT h.id, h.url, h.title, h.rev_host, h.visit_count, null, f.url, null, null FROM moz_places h LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id WHERE h.hidden <> 1 GROUP BY h.id ORDER BY 5 DESC LIMIT 10 but, if you were to go into the places organizer and attempted to view the "Most Visited" item in the toolbar, and view the visit date column, it would be empty. Note, this goes for the special "History" menu query. if you were to add a saved search with the same special query that we use for the history menu, you'd have the same problem. this might be acceptable, but still worth logging.

now that we have the "Smart Bookmarks" folder on the personal toolbar, the first item is "Most Visited". for the dreaded "ispiked" account (or really, anyone with lots of history) attempting to open this folder is going to be really slow (see comments above). Note, if you immediately re-open the query it will be fast (thanks to work from mano), but as soon as you visit a page, the menu becomes invalid and we need to rebuild it. I think the "Smart Bookmarks" folder + "Most Visited" is really handy, but only if it is fast. Note, "Recently Bookmarked" performs well, and I don't have nearly enough tags to stress "Recent Tags".

on my test db your query takes up about 360ms this is an alternative way (similar to history menu trick) that takes up about 25ms SELECT h.id, h.url, h.title, h.rev_host, h.visit_count, null, f.url, null, null FROM moz_places h LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id WHERE h.id IN (SELECT id FROM moz_places WHERE hidden <> 1 ORDER BY visit_count DESC LIMIT 10) ORDER BY h.visit_count DESC

marco, that query looks good to me. I think all we'd need is to implement IsMostVisitedMenuQuery() in nsNavHistory.cpp and call it from nsNavHistory::ConstructQueryString() and use your query if we have a match. Feel free to take this bug.

back to nobody, but hoping marco has cycles for it.

i'll try (and hopefully do) that

the most visited menu is opening instantly with my test profile and this patch, while it takes about a second with the currenty nightly

thanks marco! I'm going to test and review this, and hopefully drivers will take it for m10 (fx3 / b2)

Heck ya we'll take this. Marked blocking p1 - if it works out you are clear to land.

> if you were to go into the places organizer and attempted to view the > "Most Visited" item in the toolbar, and view the visit date column, it would be > empty. this is true, so I've logged bug #407114 on this issue. > Note, this goes for the special "History" menu query. if you were to add a > saved search with the same special query that we use for the history menu, > you'd have the same problem. this is not true, as we do calculate the visit_date column. I was mistaken.

Comment on attachment 291836 [details] [diff] [review] minimal patch for m10, will spin off bug about type=0 / queryType=0 being not necessary > // for the very special query for the history menu > // we generate a super-optimized SQL query >- if (IsHistoryMenuQuery(aQueries, aOptions)) { >+ if (IsHistoryMenuQuery(aQueries, aOptions, nsINavHistoryQueryOptions::SORT_BY_DATE_DESCENDING)) { > // visit_type <> 4 == TRANSITION_EMBED >+ // for the most visited menu query >+ // we generate a super-optimized SQL query >+ else if (IsHistoryMenuQuery(aQueries, aOptions, nsINavHistoryQueryOptions::SORT_BY_VISITCOUNT_DESCENDING)) { nit: line-length r=me.

Comment on attachment 291840 [details] [diff] [review] fix test license boiler plate to be the same as the test I started with (test_385397.js) r=me w/ the nit from my previous comment fixed.

> Heck ya we'll take this. Marked blocking p1 - if it works out you are clear > to land. thanks for the approval to land, schrep. thanks to marco for the super-optimized query and the initial patch. note, I tested with the dreaded "ispiked" profile and it we go from several seconds (wall clock) to no delay. fixed. Checking in toolkit/components/places/src/nsNavHistory.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v <-- nsNavHis tory.cpp new revision: 1.211; previous revision: 1.210 done RCS file: /cvsroot/mozilla/toolkit/components/places/tests/unit/test_399266.js,v done Checking in toolkit/components/places/tests/unit/test_399266.js; /cvsroot/mozilla/toolkit/components/places/tests/unit/test_399266.js,v <-- tes t_399266.js initial revision: 1.1 done

verified with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5) Gecko/2008032606 Firefox/3.0b5 and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-GB; rv:1.9b5) Gecko/2008032604 Firefox/3.0b5

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