If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Stop using visit_count to invalidate frecencies

RESOLVED FIXED in mozilla9

Status

()

Toolkit
Places
RESOLVED FIXED
9 years ago
6 years ago

People

(Reporter: Mardak, Assigned: mak)

Tracking

Trunk
mozilla9
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment)

(Reporter)

Description

9 years ago
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

9 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

6 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

6 years ago
Depends on: 674210
(Assignee)

Updated

6 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

6 years ago
Created attachment 556676 [details] [diff] [review]
patch v1.0

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)
Attachment #556676 - Flags: review?(dietrich) → review+
(Assignee)

Comment 4

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/fea3711b5489
Whiteboard: [inbound]
(Assignee)

Updated

6 years ago
Flags: in-testsuite+
(Assignee)

Comment 5

6 years ago
http://hg.mozilla.org/mozilla-central/rev/fea3711b5489
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.