Closed
Bug 1375287
Opened 4 years ago
Closed 4 years ago
TimeoutManager::SetTimeout should cache all preferences it needs
Categories
(Core :: DOM: Core & HTML, enhancement, P3)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla56
| Tracking | Status | |
|---|---|---|
| firefox56 | --- | fixed |
People
(Reporter: mats, Assigned: bkelly)
Details
(Keywords: perf, Whiteboard: [qf:p3])
Attachments
(1 file)
|
2.94 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
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.
| Assignee | ||
Comment 1•4 years ago
|
||
I'll look tomorrow. We already use cached prefs in this file, so it should be an easy fix.
Flags: needinfo?(bkelly)
| Assignee | ||
Comment 2•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=781fbdfc086a7f80b5c147ca1ac96280fff56c99
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Flags: needinfo?(bkelly)
| Assignee | ||
Comment 3•4 years ago
|
||
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)
Updated•4 years ago
|
Whiteboard: [qf] → [qf:p3]
Comment 4•4 years ago
|
||
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
Comment 6•4 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/c39e7022a80c
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•