Last Comment Bug 804968 - Adjusting the height of the awesomebar results popup requires too many layout flushes
: Adjusting the height of the awesomebar results popup requires too many layout...
Status: RESOLVED FIXED
[Snappy][sps]
: perf
Product: Toolkit
Classification: Components
Component: Autocomplete (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla19
Assigned To: Jared Wein [:jaws] (please needinfo? me)
:
Mentors:
Depends on: 901946 1269333
Blocks: 735588
  Show dependency treegraph
 
Reported: 2012-10-24 02:43 PDT by Jared Wein [:jaws] (please needinfo? me)
Modified: 2016-05-02 05:53 PDT (History)
4 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (2.06 KB, patch)
2012-10-24 02:43 PDT, Jared Wein [:jaws] (please needinfo? me)
jaws: review+
Details | Diff | Review
Patch v2 (2.87 KB, patch)
2012-10-24 16:35 PDT, Jared Wein [:jaws] (please needinfo? me)
no flags Details | Diff | Review

Description Jared Wein [:jaws] (please needinfo? me) 2012-10-24 02:43:30 PDT
Created attachment 674604 [details] [diff] [review]
Patch

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
Comment 1 Neil Deakin 2012-10-24 04:31:29 PDT
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.
Comment 2 Jared Wein [:jaws] (please needinfo? me) 2012-10-24 15:30:01 PDT
Comment on attachment 674604 [details] [diff] [review]
Patch

I'll change this so it only affects the awesomebar.
Comment 3 Jared Wein [:jaws] (please needinfo? me) 2012-10-24 16:35:08 PDT
Created attachment 674893 [details] [diff] [review]
Patch v2
Comment 4 Dão Gottwald [:dao] 2012-10-24 21:38:57 PDT
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.
Comment 5 Jared Wein [:jaws] (please needinfo? me) 2012-10-25 12:35:15 PDT
Comment on attachment 674604 [details] [diff] [review]
Patch

Re-setting the r+ from enn.

https://hg.mozilla.org/integration/mozilla-inbound/rev/0fc382c36b4c
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-10-25 18:27:23 PDT
https://hg.mozilla.org/mozilla-central/rev/0fc382c36b4c

Note You need to log in before you can comment on or make changes to this bug.