Open Bug 412736 Opened 17 years ago Updated 2 years ago

in the case of a frecency tie, break it with h.typed and h.visit_count


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






(Reporter: dietrich, Unassigned)




(1 file)

if I remember correctly, this caused performance issues (but I am sure a SQL guru might be able to figure that out.)
ORDER BY frecency DESC, typed DESC, visit_count DESC

I don't see why it would cause that much of a performance hit unless there are a ton of ties.

"typed" would only be considered if two items had the same frecency. Similarly, "visit_count" would only be looked at if both frecency and typed were the same for two places.

> ORDER BY frecency DESC, typed DESC, visit_count DESC

that's precisely what I did but I noticed performance issues.

Note, it's really:

ORDER BY frecency DESC, typed DESC, visit_count DESC LIMIT 100

yes, the added order by should be used only if the first one is equal, clearly the order by is faster if there is an index on the column, you could use a frecency,typed index (should be enough). that should be tested with a big sqlite though, usually ordering an int column is not so expensive. 
i haven't looked up to frecency queries yet, but edilee is right

keep in mind, with a lot of bookmarks, clear all private data can leave a lot of places with frecency = -1 (until we fix on idle)

additionally, migration from fx 2 -> fx 3 (or fx3b2 -> trunk) can end up with a lot of places with with frecency = -1 (until we fix on idle)
mh, that should be tested locally with different indexes, having a bunch of items with frecency = -1 will make the first order by less important, and an index on typed,visit_Count could come in help. but having less indexes is better than having more for db size, so the difference must be enough to apply such a change
How often will we have -1 frecency? We'll have at least one batch of non -1 frecencies when migrating now, but this would still be an issue on clearing history with many bookmarks. (I would assume that would have to be tons and tons of bookmarks..)

Would we want some way to force a recalculate if there aren't enough non -1 frecencies? That way we pay the cost once and autocomplete should perform faster in the common case.
Should we just put in the ORDER BY frecency, typed, visit_count because I think a lot of unhappiness about Firefox 3's location bar results when just starting to use it comes from a lot of uncalculated frecencies.
Attached patch v1Splinter Review
I suggest we add in the typed and visit_count sorting for 2 reasons:

1) Most pages won't have frecency ties in the common case usage -- especially for those already using Firefox 3
2) The original slowness caused by sorting is probably caused by how global frecency was incorrectly chunking data -- it would look for 100 *matching results* before returning whereas it was fixed in bug 414507 to find the top 100 frecent pages

Original code here when this bug was filed:
Assignee: nobody → edilee
Attachment #330359 - Flags: review?(dietrich)
Comment on attachment 330359 [details] [diff] [review]

r=me. lets roll with this, and pay attention to feedback in the build forum.
Attachment #330359 - Flags: review?(dietrich) → review+
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.1a1
There appears to be a considerable lag in typing in the urlbar on the latest nightly, last night's build was fast on lookups with about 28'000 history items, this is the only change I can see that may have affected it.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a1pre) Gecko/2008072203 Minefield/3.1a1pre - Core 2 Duo 2.2 Ghz.
Backed out.
Resolution: FIXED → ---
Target Milestone: Firefox 3.1a1 → Firefox 3.1
not going to make Fx3.1, actual design with temp tables makes this worse too, so need to look at what we can do for future.
Target Milestone: Firefox 3.1 → ---
Target Milestone: --- → Firefox 3.2a1
Is this still desired now that bug 476298 is fixed, we're less likely to show unwanted results after migration. But I suppose it doesn't help with clearing history and pages that calculate to negative yet.
Depends on: 476298
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.

Component: Places → Bookmarks & History
QA Contact: places → bookmarks
actually with the fix for bug 552023 and a dedicated index, we could easily do this, tested it and it's pretty fast (with the index)
Depends on: 552023
Target Milestone: Firefox 3.6a1 → ---
Assignee: edilee → nobody
Per policy at If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Closed: 16 years ago6 years ago
Resolution: --- → INACTIVE
Priority: -- → P3
Resolution: INACTIVE → ---
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.