Closed
Bug 723077
Opened 9 years ago
Closed 9 years ago
Bad performance with a lot of options in a select list
Categories
(Firefox for Android Graveyard :: General, defect, P3)
Tracking
(firefox11 affected, firefox12 affected, firefox13 verified, fennec+)
RESOLVED
FIXED
Firefox 13
People
(Reporter: martijn.martijn, Assigned: mbrubeck)
Details
(Keywords: perf, testcase, Whiteboard: [has patch])
Attachments
(2 files)
641 bytes,
text/html
|
Details | |
3.41 KB,
patch
|
wesj
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
See testcase, steps to reproduce: - Tap on combobox - Tap on one option 1 (or higher) in the select helper popup Expected result: - Select helper popup opens up immediately, after tapping on an option, the value in the combobox is changed immediately. Actual result: - Select helper popup opens up after 500ms or so, after tapping on an option, the value in the combobox is changed after 2s or so. This was tested on the LG Optimus Black, Android 2.2.2, portrait mode. On stock browser, this is much quicker. Although, it has to be said, that the stock browser has difficulty loading the page at all, it seems. On my EeeTransformer TF101, Fennec seems to crash when quickly scrolling through the options in the select helper popup.
Comment 1•9 years ago
|
||
Reproducible on the Galaxy Nexus, quite a slow-down. (Nightly 02/01 13.0a1).
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → mbrubeck
Updated•9 years ago
|
Priority: -- → P3
Comment 2•9 years ago
|
||
It seems like something is ballooning in memory in creating the select lists and causing what I continue to get below with decently sized select lists: For example, just on the language selection menu on http://www.wikipedia.org/, triggers tons of these E/dalvikvm-heap( 1884): Out of memory on a 316944-byte allocation. I/dalvikvm( 1884): "GeckoLooper Thread" prio=5 tid=10 RUNNABLE I/dalvikvm( 1884): | group="main" sCount=0 dsCount=0 obj=0x41826118 self=0x1f3698 I/dalvikvm( 1884): | sysTid=1896 nice=0 sched=0/0 cgrp=default handle=1365800 I/dalvikvm( 1884): | schedstat=( 0 0 0 ) utm=3540 stm=285 core=1 I/dalvikvm( 1884): at android.graphics.Bitmap.nativeCreate(Native Method) I/dalvikvm( 1884): at android.graphics.Bitmap.createBitmap(Bitmap.java:605) I/dalvikvm( 1884): at android.graphics.Bitmap.createBitmap(Bitmap.java:585) I/dalvikvm( 1884): at android.view.View.buildDrawingCache(View.java:10528) I/dalvikvm( 1884): at android.view.View.getDrawingCache(View.java:10378) I/dalvikvm( 1884): at android.view.ViewGroup.drawChild(ViewGroup.java:2756) I/dalvikvm( 1884): at android.widget.ListView.drawChild(ListView.java:3223) I/dalvikvm( 1884): at android.view.ViewGroup.dispatchDraw(ViewGroup.java:2495) I/dalvikvm( 1884): at android.widget.AbsListView.dispatchDraw(AbsListView.java:2092) I/dalvikvm( 1884): at android.widget.ListView.dispatchDraw(ListView.java:3218) I/dalvikvm( 1884): at android.view.View.draw(View.java:10985) I/dalvikvm( 1884): at android.widget.AbsListView.draw(AbsListView.java:3398) I/dalvikvm( 1884): at android.view.ViewGroup.drawChild(ViewGroup.java:2899) I/dalvikvm( 1884): at android.view.ViewGroup.dispatchDraw(ViewGroup.java:2495) I/dalvikvm( 1884): at android.view.View.draw(View.java:10883) I/dalvikvm( 1884): at android.view.ViewGroup.drawChild(ViewGroup.java:2899) I/dalvikvm( 1884): at android.view.ViewGroup.dispatchDraw(ViewGroup.java:2495) I/dalvikvm( 1884): at android.view.ViewGroup.drawChild(ViewGroup.java:2897) I/dalvikvm( 1884): at android.view.ViewGroup.dispatchDraw(ViewGroup.java:2495) I/dalvikvm( 1884): at android.view.ViewGroup.drawChild(ViewGroup.java:2897) I/dalvikvm( 1884): at android.view.ViewGroup.dispatchDraw(ViewGroup.java:2495) I/dalvikvm( 1884): at android.view.ViewGroup.drawChild(ViewGroup.java:2897) I/dalvikvm( 1884): at android.view.ViewGroup.dispatchDraw(ViewGroup.java:2495) I/dalvikvm( 1884): at android.view.View.draw(View.java:10883) I/dalvikvm( 1884): at android.widget.FrameLayout.draw(FrameLayout.java:450) I/dalvikvm( 1884): at com.android.internal.policy.impl.PhoneWindow$DecorView.draw(PhoneWindow.java:2106) I/dalvikvm( 1884): at android.view.ViewRootImpl.draw(ViewRootImpl.java:2005) I/dalvikvm( 1884): at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:1613) I/dalvikvm( 1884): at android.view.ViewRootImpl.handleMessage(ViewRootImpl.java:2418) I/dalvikvm( 1884): at android.os.Handler.dispatchMessage(Handler.java:99) I/dalvikvm( 1884): at android.os.Looper.loop(Looper.java:137) I/dalvikvm( 1884): at org.mozilla.gecko.GeckoAppShell$LooperThread.run(GeckoAppShell.java:185) I/dalvikvm( 1884):
Updated•9 years ago
|
tracking-fennec: --- → ?
Updated•9 years ago
|
tracking-fennec: ? → +
Comment 3•9 years ago
|
||
Tried this out out curiosity on Chrome Beta, non issue. Tested with the same language menu selection on Wikipedia.
Assignee | ||
Comment 4•9 years ago
|
||
On my phone, the test case takes about 2 seconds on average, including 1.5 seconds looping through the <option> elements in JavaScript and 0.5 seconds of JSON generation / parsing and/or message-passing latency. This patch does some micro-optimizations to the JS code, reducing the DOM walking time from 1.5 seconds to about 0.5 seconds (so the total time is reduced from 2 seconds to about 1 second): * Inside the loop, swap the "if" cases so the more common one is first. * Inline some functions. * Pass isGroup/inGroup back to the caller to avoid computing them twice * Cache the .length properties to avoid fetching them on each iteration It would be nice to improve this further, but I think the next big step should be to speed up our JSON messaging code.
Attachment #596050 -
Flags: review?(wjohnston)
Updated•9 years ago
|
Attachment #596050 -
Flags: review?(wjohnston) → review+
Assignee | ||
Comment 5•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a45cecfa8801 Marking this as fixed although there is still some noticeable slowness. We can do further optimizations in follow-up bugs.
Status: NEW → ASSIGNED
Whiteboard: [has patch]
Target Milestone: --- → Firefox 13
Comment 6•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a45cecfa8801
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment 7•9 years ago
|
||
Aurora nomination? Looks good on M-C. Verified on M-C Nightly (02/12) - Samsung Nexus S Mozilla/5.0 (Android; Mobile: rv:13.0a1) Gecko/13.0a1 Firefox/13.0a1
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 596050 [details] [diff] [review] patch [Approval Request Comment] User impact if declined: Performance when using some web forms. Testing completed (on m-c, etc.): Landed on m-c February 10. Risk to taking this patch (and alternatives if risky): Low-risk and mobile-only; this just refactors some functions for speed, with no functional changes. String changes made by this patch: None.
Attachment #596050 -
Flags: approval-mozilla-aurora?
Comment 9•9 years ago
|
||
Comment on attachment 596050 [details] [diff] [review] patch [Triage Comment] Mobile only - approved for Aurora 12.
Attachment #596050 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•2 months ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•