Closed Bug 19388 Opened 25 years ago Closed 25 years ago

Demo 13 DHTML test is slow

Categories

(Core :: Layout, defect, P3)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: belg4mit, Assigned: kmcclusk)

References

()

Details

(Keywords: perf)

It causes M11 to eat 90% of the CPU And mozilla ceases to heed any user interaction (notably anything involving document changes (forward, back, new URL)
Assignee: vidur → beard
Summary: DHTML test is vicious → [perf]DHTML test is vicious
Passing this on to beard to do analysis and cc:ing Troy. This isn't a DOM bug - both the compositor and layout/style need tuning.
On Windows what I see happening is that Javascript fires a timer every 10ms, which causes all sorts of layout changes. As a result of the layout changes, a whole lot of events related to the layout changes are sent to the operating system's event queue (WM_WINDOWPOSCHANGING, WM_PAINT, WM_ERASEBACKGROUND, etc). It might be that so many events are sent to the operating systems event queue and so much processing is done as a result of these events that some of the lower priority events in the queue (eg. timer events) are never processed (they are starved). If this is true, then one way to fix the problem would be to change the way Javascript implements the setTimout() function. Currently when the Javascript timer fires, it does not use the operating systems event queue for the timeout event (it seems). If it was changed to do so, then since WM_TIMER events are processed at a low priority in the event queue, the user interface events in the OS event queue would never be stared as they are now. All the user interface events would be processed before any timer events, providing a responsive user interface. Note: This analysis was based on the behavior I observe of the operating system's event queue, and not of any analysis of how Mozilla's code works, so I might be wrong.
*** Bug 16567 has been marked as a duplicate of this bug. ***
Component: DOM Level 0 → Layout
OS: Windows NT → All
Hardware: PC → All
Summary: [perf]DHTML test is vicious → [perf]DHTML test is vicious / starved UI&net
It may be worth noting that test13 is particularly aggressive but similar problems are observable at any point where Mozilla is simply updating a lot of graphics rapidly. I can more-or-less kill the UI responsiveness just by having four or five Mozilla browser windows open each trying to load a URL -- it seems that the simple matter of updating the spinner and progressbar for each window is enough to cause a problem (even after the Bug 13131 fix). In these situations the network also seems to stall -- this may be particular to Mozilla on GTK where the network buffers are emptied on the UI thread (is this still true?) Ironically this means that in the course of casual browsing I can reach the position where pages are not able to finish loading because too many progress-bars are spinning, and too many progress-bars are spinning because pages do not finish loading. (Try it!) Changed platform/os to all/all, componant to Layout.
Further to my comments, I think the timer implemention should be extended so that you can specify a priority for the timer notification. Timers used in the UI would be given a fairly high priority, while timers used in the Javascript DOM would be given a low priority to prevent other events in the event queue from being starved if a high frequency timer is created by a Javascript program.
Status: NEW → ASSIGNED
This test runs just fine on MacOS, the event handling isn't starved at all. It's clearly a Windows/linux problem. Hey, for once we're doing something better on the Mac OS!
Assignee: beard → kmcclusk
Status: ASSIGNED → NEW
If event starvation caused by the DOM timers is the cause of the problems, then I expect that changing the event loop (in widget/src/windows/nsAppShell.cpp) from: | | if (::PeekMessage(&msg, NULL, 0, WM_USER-1, PM_REMOVE) || | ::PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) { to: | | if (::PeekMessage(&msg, NULL, 0, WM_TIMER-1, PM_REMOVE) || | ::PeekMessage(&msg, NULL, WM_TIMER+1, UINT_MAX, PM_REMOVE || | ::PeekMessage(&msg, NULL, WM_TIMER, WM_TIMER, PM_REMOVE) { may eliviate some of the starvation problems. I don't have a machine setup with the Mozilla code in front of me to test I this actually works. Could someone (Kevin, Rod?) give this a try, and check it in if it works? In the long term, the timer class should be extended to allow different priority timers. Timers below a certain priority would be put on a queue when they complete - not to be handled immediately, but deferred until the event queue is empty. Javascript DOM timers would be created at this low priority to prevent user interface starvation.
Woops, some typos crept in. Should have been: | if (::PeekMessage(&msg, NULL, 0, WM_TIMER-1, PM_REMOVE) || | ::PeekMessage(&msg, NULL, WM_TIMER+1, UINT_MAX, PM_REMOVE) || | ::PeekMessage(&msg, NULL, WM_TIMER, WM_TIMER, PM_REMOVE)) {
An improved version - make sure [WM_USER, UINT_MAX] range is kept at second lowest priority. | if (::PeekMessage(&msg, NULL, 0, WM_TIMER-1, PM_REMOVE) || | ::PeekMessage(&msg, NULL, WM_TIMER+1, WM_USER-1, PM_REMOVE) || | ::PeekMessage(&msg, NULL, WM_USER, UINT_MAX, PM_REMOVE) || | ::PeekMessage(&msg, NULL, WM_TIMER, WM_TIMER, PM_REMOVE)) {
Status: NEW → ASSIGNED
Target Milestone: M13
Depends on: 22979
Forget the last suggested patch. See bug #22979 for a better solution.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → DUPLICATE
*** This bug has been marked as a duplicate of 22979 ***
Status: RESOLVED → VERIFIED
Dup. Verified.
Blocks: 24206
Severity: critical → normal
Status: VERIFIED → REOPENED
Keywords: perf
Summary: [perf]DHTML test is vicious / starved UI&net → Demo 13 DHTML test is slow
Even with my new timer code in the tree, this demo is still much slow than I remember it being some time ago. It would be good for someone to quantify execution of this demo and make appropriate changes in layout/rendering code to improve performance.
Resolution: DUPLICATE → ---
Clearing DUPLICATE resolution due to reopen.
Target Milestone: M13 → M14
Moving to M14
is this a dup of 4807? (or vice versa)
Moving to M15
Target Milestone: M14 → M15
*** Bug 22661 has been marked as a duplicate of this bug. ***
We need to add logic similar to what was done in the 4.x code base to dynamically back off on update rate when rendering a single frame takes a long time.
*** Bug 25667 has been marked as a duplicate of this bug. ***
Bulk moving to M16
Target Milestone: M15 → M16
Target Milestone: M16 → M17
Moving to M18
Target Milestone: M17 → M18
With the lastest build 5/17/2000 on WINNT demo13 runs much faster. I suspect the reason it is faster is because Gfx-scrollbars no longer cause style system resolution when their thumb changes. I'm marking this fixed. There is a separate bug for slow performance of DEMO13 on Linux. (bug 4807)
Status: REOPENED → RESOLVED
Closed: 25 years ago25 years ago
Resolution: --- → FIXED
Tested it with 2000-060809. Marking VERIFIED.
Status: RESOLVED → VERIFIED
No longer blocks: 24206
You need to log in before you can comment on or make changes to this bug.