Closed Bug 1375287 Opened 3 years ago Closed 3 years ago

TimeoutManager::SetTimeout should cache all preferences it needs

Categories

(Core :: DOM: Core & HTML, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: mats, Assigned: bkelly)

Details

(Keywords: perf, Whiteboard: [qf:p3])

Attachments

(1 file)

I'm looking at perf log where TimeoutManager::SetTimeout leads to a lot of
hashtable lookups taking time, via a mozilla::Preferences::GetInt call.
I think it's this one (because it's the only one I can find in SetTimeout):
http://searchfox.org/mozilla-central/rev/ae94cfb36d804727dfb38f2750bfcaab4621df6e/dom/base/TimeoutManager.cpp#455

We should use Preferences::Add...VarCache there.
I'll look tomorrow.  We already use cached prefs in this file, so it should be an easy fix.
Flags: needinfo?(bkelly)
Comment on attachment 8880404 [details] [diff] [review]
Make TimeoutManager use a pref cache variable for dom.disable_open_click_delay. r=ehsan

Avoid pref lookups in setTimeout() by using a cached var.  Pretty straightforward.
Attachment #8880404 - Flags: review?(ehsan)
Whiteboard: [qf] → [qf:p3]
Comment on attachment 8880404 [details] [diff] [review]
Make TimeoutManager use a pref cache variable for dom.disable_open_click_delay. r=ehsan

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

::: dom/base/TimeoutManager.cpp
@@ +342,5 @@
>                                 DEFAULT_MAX_CONSECUTIVE_CALLBACKS_MILLISECONDS);
> +
> +  Preferences::AddIntVarCache(&gDisableOpenClickDelay,
> +                              "dom.disable_open_click_delay",
> +                              0);

Since you defined DEFAULT_DISABLE_OPEN_CLICK_DELAY I expect you meant to use it here instead of hardcoding 0.
Attachment #8880404 - Flags: review?(ehsan) → review+
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c39e7022a80c
Make TimeoutManager use a pref cache variable for dom.disable_open_click_delay. r=ehsan
https://hg.mozilla.org/mozilla-central/rev/c39e7022a80c
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.