Closed
Bug 487809
Opened 16 years ago
Closed 13 years ago
Stop using visit_count to invalidate frecencies
Categories
(Toolkit :: Places, defect)
Toolkit
Places
Tracking
()
RESOLVED
FIXED
mozilla9
People
(Reporter: Mardak, Assigned: mak)
References
Details
(Whiteboard: [inbound])
Attachments
(1 file)
(deleted),
patch
|
dietrich
:
review+
|
Details | Diff | Splinter Review |
Bug 416313 added the original setting of frecency to -visit_count, and this included among other changes.. reordering some queries so that we can grab visit_count before we wipe the visits.
Bug 476298 made it so that we just recalculate all invalid frecencies in one go, so there's no need to order by the negative frecencies.
Reporter | ||
Comment 1•16 years ago
|
||
Oh, I forgot to emphasize the main reason :p
Because we do frecency = -MAX(visit_count, 1), some queries are more complex than necessary especially while expiring. Such as getting the visit count of pages that we *won't* expire, then expire the pages. Whereas we could just expire pages and set the remaining pages' frecency to -1.
Assignee | ||
Comment 2•13 years ago
|
||
This is a sensible improvement to do, taking since I just hit this code in another bug and remembered there was a bug filed about this.
Assignee: nobody → mak77
Assignee | ||
Updated•13 years ago
|
Summary: Determine if we still want to set frecency to -visit_count on clearing → Stop using visit_count to invalidate frecencies
Assignee | ||
Comment 3•13 years ago
|
||
First use of UPDATE WHEN CASE END, I originally thought it was only for SELECTs, I'll find more uses for it.
Btw, before we were setting frecency to -visit_count for all entries that we evaluated to stick after cleanup, removing pages, then fixing frecencies for unvisited livemarks and place: queries.
With the patch we just asynchronously set frecency to -1 or 0 on the remaining entries after the cleanup, this has some advantages: query is simpler, 1 query rather than 2, async IO.
This bug depends on bug 674210 from a code-merge point of view.
Attachment #556676 -
Flags: review?(dietrich)
Updated•13 years ago
|
Attachment #556676 -
Flags: review?(dietrich) → review+
Assignee | ||
Comment 4•13 years ago
|
||
Whiteboard: [inbound]
Assignee | ||
Updated•13 years ago
|
Flags: in-testsuite+
Assignee | ||
Comment 5•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
You need to log in
before you can comment on or make changes to this bug.
Description
•