476298 - Switch RecalculateFrecencies to just fix invalid frecencies

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Ed Lee :Mardak]

This is part 2 of 3 to address bug 458801.

Instead of recalculating old and invalid frecencies, we can just focus on the invalid ones to make them valid and estimate a frecency for the old ones.

Comment 1

[Ed Lee :Mardak]

Attachment: v1

Get rid of the prefs and RecalcFrecencies* functions.

About the comments for "maybe run on idle to do some work".. We actually do run FixInvalidFrecencies() down the way when we migrate/upgrade anyway.

I figured the query for finding invalid didn't need to be cached in the class. It'll be used once a day.. when the user is idle anyway.

Delete delete delete!

Some questions/bugs?

XXX does frecency need to be -visit_count anymore?
XXX pages with invalid frecencies tend to be those that have more than 2 redirects in a row; mDBVisitsForFrecency tries to find parent of redirect, but probably should have a helper function that finds the right source

Comment 2

[Dietrich Ayala (:dietrich)]

(In reply to comment #1)
> Created an attachment (id=359943) [details]
> v1
> 
> Get rid of the prefs and RecalcFrecencies* functions.
> 
> About the comments for "maybe run on idle to do some work".. We actually do run
> FixInvalidFrecencies() down the way when we migrate/upgrade anyway.
> 
> I figured the query for finding invalid didn't need to be cached in the class.
> It'll be used once a day.. when the user is idle anyway.
> 
> Delete delete delete!
> 
> Some questions/bugs?
> 
> XXX does frecency need to be -visit_count anymore?

no, not since we're updating them all in one shot.

> XXX pages with invalid frecencies tend to be those that have more than 2
> redirects in a row; mDBVisitsForFrecency tries to find parent of redirect, but
> probably should have a helper function that finds the right source

sure, file a followup.

Comment 3

[Dietrich Ayala (:dietrich)]

Comment on attachment 359943 [details] [diff] [review]
v1

r=me, on the code change, still needs a test though.

Comment 4

[Ed Lee :Mardak]

http://hg.mozilla.org/mozilla-central/rev/0bf1c26438e1

Create a new method FixInvalidFrecencies that finds invalid (negative) frecencies and recalculates them. Use it for handling creating/migrating DBs as well as recalculating invalid places on daily idle (place frecencies are already estimated by decay). This obviates a few preferences, queries and methods related to recalculating on idle. The test uses mork history with a number of pages that now all get their frecencies calculated on migrate, where before this fix, the test fails with a bunch of pages still with negative frecencies.