Closed Bug 32920 Opened 20 years ago Closed 20 years ago

[FIX]Extremely slow dropdown with large <select>s

Categories

(Core :: Layout: Form Controls, defect, P2, major)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: bero, Assigned: rods)

References

()

Details

(Keywords: perf, Whiteboard: [rtm++]Fix in hand)

Attachments

(1 file)

Bugzilla takes forever (and probably hangs at some point) when trying to access
http://bugzilla.redhat.com/bugzilla/query.cgi.
It looks like it's related to the rather huge <select>s used in there; but I'm
not 100% sure of that (didn't have the time to debug this).
Seems some image from the top of the page is getting redrawn 1000s of times.
 I'm not sure if I should be confirming this bug or opening a new
one. What happens for me in a current CVS build (source pulled just
before the tree opened today) is that the page renders very slowly:
first the top area (with the red Redhat logo appears) and the rest
of the page downloads; then it paints what appears to be mispositioned
forms controls or areas with the top left area of the page replicated
by them; then it thinks a while; then finally it repaints the real
versions in their proper positions in an eyeblink.

 If the reporter has a slower machine than mine, I can easily see
how my symptoms would match his, or be an evolved version of his
with more efficient forms rendering code.

 This seems more HTML Form Controls to me than HTML Elements, so I'm
pushing it off thataway too.
Assignee: rickg → rods
Status: UNCONFIRMED → NEW
Component: HTML Element → HTML Form Controls
Ever confirmed: true
QA Contact: petersen → ckritzer
moving to M17
Target Milestone: --- → M17
Marking perf.
Keywords: perf
html32 forms UE b.c. nsbeta2 6/1-.
Keywords: nsbeta2
Could this be related to bug 32827, "Pull down lists with scrollbars don't 
operate correctly"?
solution would be too high of a risk for nsbeta2
Status: NEW → ASSIGNED
Summary: Probably problem with displaying huge <select>s → Problem with displaying huge <select>s
*** Bug 34030 has been marked as a duplicate of this bug. ***
Putting on [nsbeta2-] radar.  
Whiteboard: [nsbeta2-]
Comboboxes now dropdown rather quickly, is this still a bug? Or are there other 
issues?

*** This bug has been marked as a duplicate of 27233 ***
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → DUPLICATE
This isn't a dup of Bug 27233.  27233 refers specifically to JS fill-ins.  This 
bug refers to the performance of large comboboxes in general.  Check out the 
example from bug 34077, which eventually traces back to this bug:

http://www.unclaimeddomains.com/freesample.html

Dropping down that combobox on my PII-333 can still take up to five seconds at 
100% CPU.  That's not acceptable, especially since it's instantaneous on 4.x.

Someone please a) re-open this bug and b) change the OS to all, as I run WinNT 
and 
experience this.
The performance of the combobox on the sample page 
(http://www.unclaimeddomains.com/freesample.html) is still not acceptable.  This 
bug needs to be re-opened.
Updating QA contact.
QA Contact: ckritzer → bsharma
Based on my comments from July 12, and given that the freesample.html page from 
those comments still takes about five seconds to drop down (2000092608), I'm 
re-opening this.
Status: RESOLVED → REOPENED
OS: Linux → All
Hardware: PC → All
Resolution: DUPLICATE → ---
The problem is the rule that is nede to display focus causes a ReResolveStyle to 
be called on the combobox which ends up ReRsolvingStyle on all the options. If 
there are more than a small number of option the dropdown time gets worse and 
worse.

:focus:-moz-display-comboboxcontrol-frame {
  background-color: Highlight;
  color: HighlightText;
  -moz-outline: 1px dotted HighlightText;
}

Currently, the combobox contains code to draw in the focus rect, because we 
didn't have any way to do it via style. Now that we can do it thru style it is 
too slow. I think in the next release we can find away to optimize it.

In the mean time, it can be fixed by removing the rule above. and adding a call 
to "Invalidate" in the SetFocus method of the Combobox. Here are the diffs:

Index: html.css
===================================================================
RCS file: /cvsroot/mozilla/layout/html/document/src/html.css,v
retrieving revision 3.109
diff -u -r3.109 html.css
--- html.css    2000/09/21 10:20:18     3.109
+++ html.css    2000/09/27 19:36:25
@@ -505,12 +505,6 @@
   -moz-user-select: none;
 }

-:focus:-moz-display-comboboxcontrol-frame {
-  background-color: Highlight;
-  color: HighlightText;
-  -moz-outline: 1px dotted HighlightText;
-}
-
 option {
   min-height: 1em;
   display: block;

Index: nsComboboxControlFrame.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/forms/src/nsComboboxControlFrame.cpp,v
retrieving revision 1.148
diff -u -r1.148 nsComboboxControlFrame.cpp
--- nsComboboxControlFrame.cpp  2000/09/09 22:15:57     1.148
+++ nsComboboxControlFrame.cpp  2000/09/27 19:40:45
@@ -487,7 +487,10 @@
 void
 nsComboboxControlFrame::SetFocus(PRBool aOn, PRBool aRepaint)
 {
-  //XXX: TODO Implement focus for combobox.
+  // Thiis is needed on a temporary basis. It causes the focus
+  // rect to be drawn. This is much faster than ReResolvingStyle
+  // Bug 32920
+  Invalidate(mPresContext, nsRect(0,0,mRect.width,mRect.height), PR_FALSE);
 }

 void

Nominating for RTM because the performance it so bad with large selects. Low 
risk fix.
Status: REOPENED → ASSIGNED
Keywords: nsbeta2rtm
Whiteboard: [nsbeta2-]
Summary of bug:
The problem is the rule that is needed to display focus causes a ReResolveStyle 
to be called on the combobox which ends up calling ReResolvingStyle on all the 
options. If there are more than a small number of option the dropdown time gets 
worse and worse.

On the testcase: http://www.unclaimeddomains.com/freesample.html is take 8 
seconds for the combobox to dropdown. With this fix it drops down almost 
immediately, I also notice a speed up on shorter lists also.

Summary of patch:
Basically, the nsComboxControlFrame will track whether it has focus or not and 
either draw a dotted rect to indicate focus or a solid rect to remove it. 
Instead of each frame tracking whether it has focus the nsComboboxControlFrame 
has a static class data member because only one combo frame can have focus at 
any one time.

SetFocus needs to get called on the combo frame for both gaining and losing 
focus and then it does an Invalidateto make sure it gets "painted in". the 
SetFocus was already being called in the event state manager, but when a blur 
event occurred code needed to be added to nsHTMLSelectElement to call SetFocus 
with false.

lower risk - big performace gain.
Whiteboard: Fix in hand
Attached patch patch fileSplinter Review
Summary: Problem with displaying huge <select>s → Extremely slow dropdown with large <select>s
Marking rtm+. Low risk fix which fixes extremely bad performance problem.
Really marking rtm+.
Whiteboard: Fix in hand → [rtm+]Fix in hand
Summary: Extremely slow dropdown with large <select>s → [FIX]Extremely slow dropdown with large <select>s
changing severity to major, priority to P2.  Without this fix, some pages are
nearly unusable.  I'm in the process of reviewing now, so far, so good.
Severity: normal → major
Priority: P3 → P2
I don't get this part of the diff:
 NS_NewComboboxControlFrame(nsIPresShell* aPresShell, nsIFrame** aNewFrame,
PRUint32 aStateFlags)
 {
@@ -248,6 +251,7 @@
   mItemDisplayWidth             = 0;

   mGoodToGo = PR_FALSE;
+  mFocused  = nsnull;


Why is mFocused set to null in the combobox factory method?  What would happen
if the user clicked on a combobox and somehow the DOM caused a new combobox to
get created on the page?  Does focus get yanked from the first combobox in this
case? If so, wouldn't the blur message null out the arg for us.  And if not,
won't we have the case where the combobox has focus, but now paints as if it
does not?
Steve, this is why we have reviews. You are right, this line was left over from 
when I had it be an instance variable and forgot to remove it. Consider it 
removed and I now am initializing the static class variable like this:

nsComboboxControlFrame * nsComboboxControlFrame::mFocused = nsnull;

Sorry, that should take care of it.
with that change, a=buster
r=attinasi (with the fix for the class-variable initialization)
Marking rtm++.  Please check this in as soon as possible.
Whiteboard: [rtm+]Fix in hand → [rtm++]Fix in hand
fixed
Status: ASSIGNED → RESOLVED
Closed: 20 years ago20 years ago
Resolution: --- → FIXED
Build: 2000-10-08-09 MN6
Platform: Win98
Status: RESOLVED → VERIFIED
*** Bug 44312 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.