Closed Bug 804968 Opened 7 years ago Closed 7 years ago

Adjusting the height of the awesomebar results popup requires too many layout flushes

Categories

(Toolkit :: Autocomplete, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19

People

(Reporter: jaws, Assigned: jaws)

References

Details

(Keywords: perf, Whiteboard: [Snappy][sps])

Attachments

(1 file, 1 obsolete file)

Attached patch PatchSplinter Review
STR:
1) Open a tab with an average size data URI as the address, for example the cat image from http://dopiaza.org/tools/datauri/examples/
2) Open a new tab, and start typing 'data' in to the address bar.

The SPS profiler shows that a ton of time is spent within adjustHeight of the rich-result-popup in autocomplete.xml. The code within adjustHeight calls getBoundingClientRect for two rows, and then computes the difference between the top of the first row and the bottom of the last row.

The heights of these rows shouldn't change while the program is running, so we should be able to cache the height of the row the first time this method is called, and then use the cached value from then-on.

See these two profiles for examples of the jank I am seeing (filter on adjustHeight):
http://people.mozilla.com/~bgirard/cleopatra/?report=f8d697a5720acd9d00d0fabc01c8a26ff88256c9
http://people.mozilla.com/~bgirard/cleopatra/?report=7331ec1c982940482150016f005b77c16e8b75ec
Attachment #674604 - Flags: review?(enndeakin)
Comment on attachment 674604 [details] [diff] [review]
Patch

This patch doesn't allow the row height to change nor allows having rows of different heights. I assume that is ok.
Attachment #674604 - Flags: review?(enndeakin) → review+
Comment on attachment 674604 [details] [diff] [review]
Patch

I'll change this so it only affects the awesomebar.
Attachment #674604 - Flags: review+
Summary: Adjusting the height of a rich result popup requires too many reflows and layouts → Adjusting the height of the awesomebar requires too many reflows and layouts
Attached patch Patch v2 (obsolete) — Splinter Review
Attachment #674604 - Attachment is obsolete: true
Attachment #674893 - Flags: review?(dao)
Comment on attachment 674893 [details] [diff] [review]
Patch v2

Firefox is the primary consumer of the rich-results binding, so I think we should take the first patch.
Attachment #674893 - Flags: review?(dao)
Keywords: perf
Summary: Adjusting the height of the awesomebar requires too many reflows and layouts → Adjusting the height of the awesomebar results popup requires too many layout flushes
Attachment #674893 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/0fc382c36b4c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Blocks: 735588
You need to log in before you can comment on or make changes to this bug.