1375287 - TimeoutManager::SetTimeout should cache all preferences it needs

Status: RESOLVED FIXED

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

Description

[Mats Palmgren (inactive)]

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.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

I'll look tomorrow.  We already use cached prefs in this file, so it should be an easy fix.

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Make TimeoutManager use a pref cache variable for dom.disable_open_click_delay. r=ehsan

https://treeherder.mozilla.org/#/jobs?repo=try&revision=781fbdfc086a7f80b5c147ca1ac96280fff56c99

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

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.

Comment 4

[(no longer active)]

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.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c39e7022a80c
Make TimeoutManager use a pref cache variable for dom.disable_open_click_delay. r=ehsan

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c39e7022a80c