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)

x86
All
defect

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
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...
Status: UNCONFIRMED → NEW
Depends on: 21762
Ever confirmed: true
That would timely correlate with the regression reported at bug 153083
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.
OS: Windows XP → All
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
->kin first.
Assignee: attinasi → kin
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.
On Linux builds the regression was between 2002-06-13-04-trunk and 2002-06-13-21-trunk.
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.
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.
Ugh, I meant "you might not see any painting done until you stop moving the mouse around" above.
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.
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.
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?
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?
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]
Here are some perf painting related bugs: bug 69010 bug 142635 .. but dcone will have more insight into this. Also interesting is bug 153083.
Blocks: 21762
No longer depends on: 21762
Blocks: 154524
marking bug 154524 dependant on this as it seems to be affected in much the same way
No longer blocks: 154524
No longer blocks: 1.1b
Depends on: 157144
Summary: Slider is extremely slow (pre Moz 1.0 performance) → [regression] Slider is extremely slow (pre Moz 1.0 performance)
Nominating for nsbeta1 as this a regression of DHTML performnace.
Keywords: nsbeta1
Whiteboard: [adt3]
Keywords: testcase
nsbeta1+
Keywords: nsbeta1nsbeta1+
Summary: [regression] Slider is extremely slow (pre Moz 1.0 performance) → [DHTML][regression] Slider is extremely slow (pre Moz 1.0 performance)
Target Milestone: mozilla1.1beta → mozilla1.2beta
http://www.drunk.nl/lab/bugzilla/ scrolls smoothly when the patch in bug 163528 is applied.
Depends on: 163528
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
Both pages scroll smoothly with the patch in bug 163528 applied
This was fixed by my checkin for bug 163528. Resolving as fixed.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
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
Same problem as comment 26, the page doesnt render in Mozilla. Build 20020920, Windows 98.
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.
FYI: Bug 170040 [ Elements now shown when body has overflow:hidden; ] was filed for this problem.
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
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.
the bug which prevents http://www.drunk.nl/lab/bugzilla/ from rendering was the result of a checkin between 9/16 and 9/17.
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.
What checkin? There are no patches on this bug. Did you mean a different bug number?
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.
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.
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.
verified on branch
Status: RESOLVED → VERIFIED
No longer blocks: 21762
Blocks: 21762
You need to log in before you can comment on or make changes to this bug.