Closed
Bug 891104
Opened 10 years ago
Closed 10 years ago
Skip calling onOverflow during startup if there wasn't any overflowed content before the toolbar is fully initialized
Categories
(Firefox :: Theme, defect)
Firefox
Theme
Tracking
()
RESOLVED
FIXED
Firefox 28
People
(Reporter: MattN, Assigned: jaws)
References
(Depends on 1 open bug)
Details
(Keywords: perf, Whiteboard: [Australis:M8])
Attachments
(1 file, 1 obsolete file)
3.71 KB,
patch
|
mconley
:
review+
|
Details | Diff | Splinter Review |
Perhaps this isn't possible but there may be heuristics we can use that don't require a reflow. Try push: https://tbpl.mozilla.org/?tree=Try&rev=73fccc85c313
Assignee | ||
Comment 2•10 years ago
|
||
This patch only runs the onOverflow code if we've actually received an overflow event. Try push with patch: https://tbpl.mozilla.org/?tree=Try&rev=caa53f9af310 Try push w/out patch: https://tbpl.mozilla.org/?tree=Try&rev=3ea895acccce
Attachment #772310 -
Attachment is obsolete: true
Reporter | ||
Comment 3•10 years ago
|
||
My try push to skip this altogether showed slight tpaint wins on most platforms but it also regressed tresize and dirty places by a larger magnitude. Perf: http://perf.snarkfest.net/compare-talos/index.html?oldRevs=515129449f3e&newRev=3c89703e6993&submit=true Try run: https://tbpl.mozilla.org/?tree=Try&rev=3c89703e6993 Patch: https://hg.mozilla.org/try/rev/22bb2d9fabb8 Any ideas why that would cause regressions? Perhaps your version won't show the same.
Assignee | ||
Comment 4•10 years ago
|
||
Running this using mconley's local tpaint with 1,000 iterations: With patch: 197.31 (175 - 385.59) W/out patch: 213.45 (177.38 - 437.45)
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Matthew N. [:MattN] from comment #3) > My try push to skip this altogether showed slight tpaint wins on most > platforms but it also regressed tresize and dirty places by a larger > magnitude. > > Any ideas why that would cause regressions? Perhaps your version won't show > the same. Well, your patch completely disables the overflow code. I could take a guess that when resizing to a smaller window, once we place an element in the overflow widget then resizing is less work as there are less elements to move around.
Assignee | ||
Updated•10 years ago
|
Attachment #781958 -
Flags: review?(mconley)
Assignee | ||
Updated•10 years ago
|
Summary: Consider skipping onOverflow to avoid a reflow on startup → Skip calling onOverflow during startup if there wasn't any overflowed content before the toolbar is fully initialized
Comment 6•10 years ago
|
||
Comment on attachment 781958 [details] [diff] [review] Patch Review of attachment 781958 [details] [diff] [review]: ----------------------------------------------------------------- I think this works really nicely. Update that one comment I pointed out, and it's r=me. Thanks jaws! ::: browser/components/customizableui/src/CustomizableUI.jsm @@ +2211,3 @@ > > // The toolbar could initialize in an overflowed state, in which case > // the 'overflow' event may have been fired before the handler was registered. This comment isn't really reflective of the code anymore. Instead, it should say that the 'overflow' event may have been fired before init was called.
Attachment #781958 -
Flags: review?(mconley) → review+
Assignee | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/projects/ux/rev/8637eb509182
Whiteboard: [Australis:M8][fixed-in-ux]
Comment 8•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8637eb509182
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M8][fixed-in-ux] → [Australis:M8]
Target Milestone: --- → Firefox 28
You need to log in
before you can comment on or make changes to this bug.
Description
•