Closed Bug 476298 Opened 11 years ago Closed 11 years ago

Switch RecalculateFrecencies to just fix invalid frecencies

Categories

(Toolkit :: Places, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: Mardak, Assigned: Mardak)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [fixes bug 476300, bug 482351])

Attachments

(1 file)

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.
Blocks: 476299
Blocks: 476300
Attached patch v1Splinter Review
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
Assignee: nobody → edilee
Status: NEW → ASSIGNED
Attachment #359943 - Flags: review?(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 on attachment 359943 [details] [diff] [review]
v1

r=me, on the code change, still needs a test though.
Attachment #359943 - Flags: review?(dietrich) → review+
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.
Blocks: 482351
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Whiteboard: [fixes bug 476300, bug 482351]
Target Milestone: --- → mozilla1.9.2a1
No longer blocks: 482351
Blocks: 482351
Blocks: 412736
Blocks: 487809
Blocks: 487810
Blocks: 487813
You need to log in before you can comment on or make changes to this bug.