Closed
Bug 154568
Opened 22 years ago
Closed 22 years ago
[DHTML][regression] Slider is extremely slow (pre Moz 1.0 performance)
Categories
(Core :: Layout, defect, P3)
Tracking
()
VERIFIED
FIXED
mozilla1.2beta
People
(Reporter: tunaman, Assigned: kinmoz)
References
()
Details
(Keywords: perf, regression, testcase, Whiteboard: [adt3])
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1a) Gecko/20020610
BuildID: 2002061408
There seems to be a serious regression going on with the builds after the 13th
regarding DHTML performance, to be short we are back on old pre Mozilla 1.0
performance level. At least in the link that I provided.
I posted my findings on Mozillazine.org and several people confirmed my suspicion.
I did some tests with the builds after the 13th up to the build from the 26th
and they all suck (with DHTML).
It seems that all the work Waterson and others put in to this was to no avail.
Can someone with knowledge about checkins that happened between the 13th and the
14th take a look at this and maybe find out what caused this serious regression.
It seems to affect Windows and Linux primarely, on my Mac I can't really see any
degrading performance, but i'll have to check that again with the latest builds.
Reproducible: Always
Steps to Reproduce:
1. go to URL
2. Use the slider
3. use the buttons
Actual Results: CPU going trough the roof (100%) updates about every 3 seconds
(on my Athlon XP 1900+ 768 MB DDR).
Expected Results: Should be going very smooth, like the first Mozilla 1.0,
Mozilla 1.1 Alpha release and the two builds after that release (20020612 and
20020613) with around 20-40% cpu use.
You can also go to dhtmlcentral.com for example and try some demo's, there or
the demo's that are included on the official Mozilla 1.0 release page
Reporter | ||
Updated•22 years ago
|
Reporter | ||
Updated•22 years ago
|
Comment 1•22 years ago
|
||
Checkins on the 13th and 14th:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=MozillaTinderboxAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=06%2F13%2F2002+00%3A00%3A00&maxdate=06%2F14%2F2002+23%3A59%3A59&cvsroot=%2Fcvsroot
Checkins on the 12th:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=MozillaTinderboxAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=06%2F12%2F2002+00%3A00%3A00&maxdate=06%2F12%2F2002+23%3A59%3A59&cvsroot=%2Fcvsroot
Comment 2•22 years ago
|
||
Layout-related checkins in there:
1) karnaze's fix for bug 148338
2) karnaze's fix for bug 148245
3) karnaze's fix for bug 148399
4) kin's fixes for bug 141900
Item #4 is he one I find most likely to be responsible...
Updated•22 years ago
|
Comment 3•22 years ago
|
||
That would timely correlate with the regression reported at bug 153083
Reporter | ||
Comment 4•22 years ago
|
||
Sorry for the spam, but i D/Loaded the most recent Mac build (2002062608) and
indeed my first finding was correct, this bug does NOT affect the performance on
the Mac, i would like to see some more people who can confirm this.
However Linux IS affected (also with the latest build available).
Linux: Bad (after build 20020613)
Win (all?): Bad (after build 20020613)
Mac: Fine, no regression
These are my testresults.
Comment 5•22 years ago
|
||
jrgm, if you have time can you look into this; maybe a selective backout and
test of some of the more likely regression candidates from the 13th?
Blocks: 1.1b
If this were due to my fix for bug 141900, it would have to be the patch for
nsBlockFrame.cpp and nsBox.cpp (bug 141900 attachment 87306 [details] [diff] [review]).
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla1.1beta
I just backed out attachment 87306 [details] [diff] [review] from my local tree and scrolling is still
"chunky" or slow. I'd be surprised if attachment 87306 [details] [diff] [review] made things slower since
it made scrolling significantly faster for things like:
http://www.wikipedia.com/wiki/September_11,_2001_Terrorist_Attack/New_York_Times_stories&action=edit
The rest of my changes for bug 141900 are for making selection support async
scrolling, but that's only activated when typing/editing in text widgets. All of
that code can be disabled by simply commenting out or jumping over the the line:
editorFlags |= nsIPlaintextEditor::eEditorUseAsyncUpdatesMask;
in nsGfxTextControlFrame2.cpp. I also tested scrolling with this line commented
out and things were still slow with the sample test case.
Comment 9•22 years ago
|
||
On Linux builds the regression was between 2002-06-13-04-trunk and
2002-06-13-21-trunk.
Assignee | ||
Comment 10•22 years ago
|
||
Ok, so it looks like it is related to the editor using:
editorFlags |= nsIPlaintextEditor::eEditorUseAsyncUpdatesMask;
Apparently we are creating the nsGfxTextControlFrame twice, and when I debugged
the first time, I only jumped over the setting of the async flag on the first call.
I'll need to investigate what the problem is here, and what's so special about
this test case. Note I don't see this slow down in normal text widgets.
Assignee | ||
Comment 11•22 years ago
|
||
So looking into this, it turns out that the scrollbar in question is not a
native mozilla/gfx scrollbar at all ... so everyone can relax. :-) This explains
why I don't see the slow down in other pages or text widgets with lots of text.
Dragging the scrollbar on the test page is triggering some JS that sets the
value of a hidden text widget on the page. It used to be that every time you set
the value of a text widget, we would tell the view manager to flush any pending
updates it had, immediately ... this would cause the view manager to
synchronously paint all pending update requests it had, my theory is that this
also had the side effect of painting update requests that were outside of the
text widget ... thus causing the scrollbars to redraw at their new position.
Now that we use async updates, we don't flush when the value is set. Instead,
the view manager posts a PLEvent, which gets processed at a later time. When the
PLEvent is processed, it merely invalidates the regions on the widget,
generating native OS Paint events, which means that the actual paint won't
happen again until we return to the event queue. Even if we return to the event
queue, the priority at which the paint events will be processed is OS dependent,
so you might not see any painting done until you start moving the mouse around.
In short, I think that realtime feel of the scrollbars was just a sideffect of
the fact that the text widgets used to force view manager flushes.
It would be a good experiment for the owner of the page to remove the code that
sets the value on the text widget in a branch or Moz1.0 build which doesn't
exhibit the problem, and see if they get the same "sluggish" looking movement of
their scrollbar. I'd do it myself, but I can't get the test case to run locally
on my machine without giving me lots-o-DynObj-errors.
Assignee | ||
Comment 12•22 years ago
|
||
Ugh, I meant "you might not see any painting done until you stop moving the
mouse around" above.
Comment 13•22 years ago
|
||
With an old Chimera build and my debug build with the patches in 141900 applied,
I see a big improvement when going to the above url and scrolling (improvement
in the amount of time it takes to scroll by holding down the mouse button). I'd
guess it's about half the time (4 seconds vs 7 seconds) but it's also a debug
build so the 4 seconds might be less if I had an optimized build to compare to).
Again--this is Macintosh platform so the above doesn't really apply since it
wasn't a problem on Macintosh builds to begin with.
Reporter | ||
Comment 14•22 years ago
|
||
Ok Kin i'll set 2 examples up today one with the flush and one without, BTW you
can easily overcome the errors by deleting the layers that come after the first
layer.
However I Noticed degrading performance on the one with the force view manager
flushes.
Reporter | ||
Comment 15•22 years ago
|
||
Ok new testcase:
http://www.drunk.nl/lab/bugzilla/
Ha you were right! the version without the Mozilla optimization is performing
better than the one with... incredible but true. (Linux 2002062521)
The chunky behaviour is caused by Line 170 in the scrollbar.js and scrollbar2.js
if you comment that line out it works fine in the builds after the 13th but it
will suck in the builds before that date. (up to Moz 1.0)?
Can anyone confirm what i'm seeing?
Assignee | ||
Comment 16•22 years ago
|
||
Confirming martijn's findings.
I saved the original test case (http://www.drunk.nl/lab) locally and removed the
2nd layer as martijn suggested. This allowed the file to load ok in the browser.
I then launched my MOZILLA_1_0_BRANCH based browser, which does not have any
fixes from bug 141900, and noticed how zippy it was, when I dragged the slider
frame around ... it tracked my mouse movements in realtime and scrolled the
content area smoothly.
I then edited the scrollbar.js file, and commented out the following line:
if(b.NS6) document.getElementById("ns6hlpinp").value=value;
which is what sets the hidden textfield's value. I reloaded the file in the same
browser, and confirmed my suspicions. The slider frame moved in the same
choppy/chunky manner that the TRUNK builds, which have my fixes for bug 141900,
exhibit.
So now what? Do we have a bug on slow painting performance during dhtml
execution? Should I make this bug depend on it?
Reporter | ||
Comment 17•22 years ago
|
||
maybe this one? http://bugzilla.mozilla.org/show_bug.cgi?id=124178 and this one?
http://bugzilla.mozilla.org/show_bug.cgi?id=129115
[ot]
the Audi page seems to work quite good on my machine.
[/ot]
Comment 18•22 years ago
|
||
Here are some perf painting related bugs:
bug 69010
bug 142635
.. but dcone will have more insight into this.
Also interesting is bug 153083.
Updated•22 years ago
|
Comment 19•22 years ago
|
||
marking bug 154524 dependant on this as it seems to be affected in much the same way
Updated•22 years ago
|
Summary: Slider is extremely slow (pre Moz 1.0 performance) → [regression] Slider is extremely slow (pre Moz 1.0 performance)
Comment 20•22 years ago
|
||
Nominating for nsbeta1 as this a regression of DHTML performnace.
Keywords: nsbeta1
Whiteboard: [adt3]
Comment 21•22 years ago
|
||
nsbeta1+
Comment 22•22 years ago
|
||
http://www.drunk.nl/lab/bugzilla/ scrolls smoothly when the patch in bug 163528
is applied.
Reporter | ||
Comment 23•22 years ago
|
||
Wich one Kevin? I got 2 pages up there one with the Mozilla optimization (Fast
in Mozilla up to 1.0) And one without optimization that is fast in Mozilla 1.1
Comment 24•22 years ago
|
||
Both pages scroll smoothly with the patch in bug 163528 applied
Comment 25•22 years ago
|
||
This was fixed by my checkin for bug 163528.
Resolving as fixed.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 26•22 years ago
|
||
using build 2002091908 on Win2k (sp2sr1)
ahhh....
Is something supposed to happen on those pages? By "those" I mean
http://www.drunk.nl/lab/bugzilla/
Because I'm not seeing anything happening. When I load the pages in IE, I see an area with a scrollbar that I can scroll. In mozilla, I see a blank page except for the link in the upper left hand corner. So, I guess it doesn't scroll well for me since I have nothing to scroll.
Jake
Comment 27•22 years ago
|
||
Same problem as comment 26, the page doesnt render in Mozilla. Build 20020920,
Windows 98.
Comment 28•22 years ago
|
||
I just pulled a 2002091808 build which pre-dates my checkin for bug 163528 and
it does not render the page on WinXP so the rendering issue is un-related to my
check in.
My local tree works fine so the regression must have happened after 9/12/2002.
Comment 29•22 years ago
|
||
FYI:
Bug 170040 [ Elements now shown when body has overflow:hidden; ] was filed for
this problem.
Comment 30•22 years ago
|
||
Kevin,
Would you mind checking to see if the following works in your build pre-dating
checkins for bug 163528 and/or bug 164931?
http://www.mozilla.org/newlayout/xml/debugdemos/tocdemo/rights.xml
The issue I'm seeing is that if you click on the "contents" button which opens
the TOC nav on the side and try to collapse/uncollapse the items in the tree by
clicking on the down/up arrows, nothing happens. It used to work. I get no
javascript errors in the javascript console so I surmise it may be related to
painting or events both of which were changed by you recently in the mentioned
bugs.
BTW, I'm using build 2002091908
Jake
Comment 31•22 years ago
|
||
I just tested build (20002091608) which pre-dates the checkins for bug 163528
and bug 164931 and
http://www.mozilla.org/newlayout/xml/debugdemos/tocdemo/rights.xml does *not*
work. So those checkins did not cause this regression.
Comment 32•22 years ago
|
||
the bug which prevents http://www.drunk.nl/lab/bugzilla/ from rendering was the
result of a checkin between 9/16 and 9/17.
Comment 33•22 years ago
|
||
the http://www.drunk.nl/lab/bugzilla/ regression was caused by the checkin for
bug 154568. When I apply this patch to my local tree it stops rendering. If I
remove the patch it renders and scrolls smoothly.
Comment 34•22 years ago
|
||
What checkin? There are no patches on this bug. Did you mean a different bug
number?
Comment 35•22 years ago
|
||
Oops. I meant to the say the checkin for bug 168294 caused the regression on
http://www.drunk.nl/lab/bugzilla/ in the previous comment.
Comment 36•22 years ago
|
||
It looks like the entire contents of that page are in an absolutely positioned
div with 0 size and 'overflow: hidden', which suggests that nothing should be
displayed.
In http://www.drunk.nl/lab/bugzilla we clearly should be hiding everything in
'testDiv'. Anything else would not only be wrong, it would be counter-intuitive.
We can discuss compatibility issues arising from authors using overflow:hidden
on BODY in bug 170040.
Reporter | ||
Comment 38•22 years ago
|
||
I think you are making the wrong conclusions here, the div wich actually holds
the content is generated trough javascript, i put a new version online wich
illustrates this perfectly (http://www.drunk.nl/lab/bugzilla/index2.php), the
thing is that something is broken in the latest builds, because my version
(2002091014) works perfectly with my testcase
can you convert that to a static page which shows the problem you're concerned
about?
Never mind.
http://www.drunk.nl/lab/bugzilla/index2.php is another example of a BODY with
overlow:hidden where the BODY is very small so almost everything is clipped.
See bug 170040 for discussion.
You need to log in
before you can comment on or make changes to this bug.
Description
•