Closed
Bug 19388
Opened 25 years ago
Closed 25 years ago
Demo 13 DHTML test is slow
Categories
(Core :: Layout, defect, P3)
Core
Layout
Tracking
()
VERIFIED
FIXED
M18
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)
Updated•25 years ago
|
Assignee: vidur → beard
Summary: DHTML test is vicious → [perf]DHTML test is vicious
Comment 1•25 years ago
|
||
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.
Comment 2•25 years ago
|
||
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.
Updated•25 years ago
|
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
Comment 4•25 years ago
|
||
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.
Comment 5•25 years ago
|
||
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.
Updated•25 years ago
|
Status: NEW → ASSIGNED
Comment 6•25 years ago
|
||
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!
Updated•25 years ago
|
Assignee: beard → kmcclusk
Status: ASSIGNED → NEW
Comment 7•25 years ago
|
||
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.
Comment 8•25 years ago
|
||
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)) {
Comment 9•25 years ago
|
||
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)) {
Assignee | ||
Updated•25 years ago
|
Status: NEW → ASSIGNED
Target Milestone: M13
Comment 10•25 years ago
|
||
Forget the last suggested patch. See bug #22979 for a better solution.
Assignee | ||
Updated•25 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → DUPLICATE
Assignee | ||
Comment 11•25 years ago
|
||
*** This bug has been marked as a duplicate of 22979 ***
Updated•25 years ago
|
Status: RESOLVED → VERIFIED
Comment 12•25 years ago
|
||
Dup. Verified.
Updated•25 years ago
|
Severity: critical → normal
Status: VERIFIED → REOPENED
Keywords: perf
Summary: [perf]DHTML test is vicious / starved UI&net → Demo 13 DHTML test is slow
Comment 13•25 years ago
|
||
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.
Comment 14•25 years ago
|
||
Clearing DUPLICATE resolution due to reopen.
Assignee | ||
Updated•25 years ago
|
Target Milestone: M13 → M14
Assignee | ||
Comment 15•25 years ago
|
||
Moving to M14
Comment 16•25 years ago
|
||
is this a dup of 4807? (or vice versa)
Assignee | ||
Comment 18•25 years ago
|
||
*** Bug 22661 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 19•25 years ago
|
||
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.
Comment 20•25 years ago
|
||
*** Bug 25667 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 21•25 years ago
|
||
Bulk moving to M16
Assignee | ||
Updated•25 years ago
|
Target Milestone: M15 → M16
Assignee | ||
Comment 23•25 years ago
|
||
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 ago → 25 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•