Don't trigger moz_hosts frecency update when updating frecency on idle

RESOLVED FIXED in Firefox 12

Status

()

Toolkit
Places
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mak, Assigned: mak)

Tracking

unspecified
mozilla13
Points:
---

Firefox Tracking Flags

(firefox11 unaffected, firefox12 fixed)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

In idle we update frecency for ALL pages. This is an expensive operation that a new frecency implementation like the one suggested in bug 704025 would solve, though, for now we can't avoid that.
The problem this is trying to solve is that, on frecency update, we now go updating the moz_hosts frecencies, and doing this when we update ALL the frecencies values may be quite expensive. if it takes a long time, it may cause larger brackets, where thread contention may happen.
Created attachment 594689 [details] [diff] [review]
patch v1.0

The right long-term fix is bug 704025, in the meanwhile we have to keep updating all frecencies on idle.
This patch adds barriers to the frecency trigger, so that we update the host frecency only if a page frecency changes significantly (more than 5%). So, on idle we practically skip the useless complete update of all the frecencies.

This should fix all the recent frecency on idle hangs we saw, and should be ported to Aurora asap.
Attachment #594689 - Flags: review?(dietrich)
Comment on attachment 594689 [details] [diff] [review]
patch v1.0

Review of attachment 594689 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, given that existing tests cover that ordering stays correct and that frecencies get updated often enough.
Attachment #594689 - Flags: review?(dietrich) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/8656d37fcebf
Target Milestone: --- → mozilla13
https://hg.mozilla.org/mozilla-central/rev/8656d37fcebf
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Comment on attachment 594689 [details] [diff] [review]
patch v1.0

[Approval Request Comment]
Regression caused by (bug #): bug 566489
User impact if declined: Long UI hangs on idle, that also means watching videos and such.
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): the patch reduces the amount of work we do by skipping some useless updates. risk is low and the affected feature is disabled for now
String changes made by this patch: no
Attachment #594689 - Flags: approval-mozilla-aurora?

Comment 6

6 years ago
(In reply to Marco Bonardo [:mak] from comment #5)
> Risk to taking this patch (and alternatives if risky): the patch reduces the
> amount of work we do by skipping some useless updates. risk is low and the
> affected feature is disabled for now

If the feature is currently disabled, I'm not sure we need to fix on Aurora. Are there plans to enable for FF12, or have we promoted the pref externally?
(In reply to Alex Keybl [:akeybl] from comment #6)
> (In reply to Marco Bonardo [:mak] from comment #5)
> > Risk to taking this patch (and alternatives if risky): the patch reduces the
> > amount of work we do by skipping some useless updates. risk is low and the
> > affected feature is disabled for now
> 
> If the feature is currently disabled, I'm not sure we need to fix on Aurora.
> Are there plans to enable for FF12, or have we promoted the pref externally?

the feature is disabled, but the part of the backend collecting data is not, for data coherence reasons, the fix is on that part.
To clarify, the fix is NEEDED on Aurora because only the frontend part of the feature is disabled, we cannot disable the backend part, that is what causes this bug.

Updated

6 years ago
Duplicate of this bug: 723387
Comment on attachment 594689 [details] [diff] [review]
patch v1.0

[Triage Comment]
Thanks for the background - approved for Aurora 12.
Attachment #594689 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/96584d3595fa
status-firefox11: --- → unaffected
status-firefox12: --- → fixed
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.