{sink}Limit number of incremental reflows

VERIFIED FIXED in M17

Status

()

Core
Layout
P2
major
VERIFIED FIXED
19 years ago
10 years ago

People

(Reporter: Michael Lowe, Assigned: Chris Waterson)

Tracking

({perf})

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

(Reporter)

Description

19 years ago
A good point was made on MozillaZine.   On a high speed connection, doing an
incremental reflow each time data arrives is probably not the best approach.
Instead, it is probably best to use a timer and only do reflows when the data
arrives if it is X milliseconds since the last reflow was done.   Any content
held back because of this should be added and a reflow performed at the end
of the next interval of X milliseconds since the last reflow, or at the
completion of the page loading.

Comment 1

19 years ago
Adding a preference to change the X milliseconds between reflows should be
added to Mozilla for the pleasure of experienced users :)

But yes, incremental reflow based on a certain number of downloaded bytes
between reflows is a pain for people with high speed connections since it makes
pages load much longer than they were before this was this incremental reflow
was put in Mozilla. One based on time would be much more efficient.
(Reporter)

Comment 2

19 years ago
This was posted to the newsgroups:


       From: Scott Furman <fur@netscape.com>

Is there any benefit to performing content append notifications at intervals
defined by the passage of a fixed amount of time rather than by the
processing of a single buffer passed from netlib ?  In that way, the faster the
link, the more buffers of data are aggregated into a single content
append notification.  As a result, less incremental layout/display takes place
when the netlib delivers high data rates.  The basic shape of the
code might look like this:


     For each buffer of data from netlib:

        1.Update the content model
        2.If a content append timeout is already pending, return to caller
        3.timeoutFiringTime = MAX(now, lastFiringTime + INTERVAL)
        4.If no timeout is already pending, launch a content append notification
timeout that will fire at timeoutFiringTime.

     When a content append notification timeout fires:

        1.lastFiringTime = now
        2.Deliver a content append notification for all content added since the
last notification, including content added between the time
          that the timeout was launched and the time it fired

     When OnStopRequest is received:

        1.If no timeout is pending, return success
        2.Destroy the pending timeout
        3.Deliver a content append notification, as if the timeout had fired

Incidentally, this thought was triggered by the way in which Navigator (and
Mozilla) implement progressive JPEG decoding.  The (expensive)
JPEG decode/display process is performed at regular time intervals rather than
after processing every netlib buffer of data.  Using this
technique, on very fast network links, no progressive image display takes place,
because at the time of the first image decode, all the data has
arrived and only the final JPEG image is displayed.  On slow links, the
identical file is initially displayed as a fuzzy image and the image
resolution is progressively improved as more data arrives.  The slower the link,
the more progressive the display.

-Scott

Comment 3

19 years ago
*** Bug 17444 has been marked as a duplicate of this bug. ***

Updated

19 years ago
Status: NEW → ASSIGNED
Summary: [PERF]Limit number of incremental reflows → {sink}[PERF]Limit number of incremental reflows
(Reporter)

Comment 4

19 years ago
It would be good to add another heuristic as well, so that you time how long
each incremental reflow takes and decrease the frequency of reflows as the
reflow time increases.   This way when pages with very big tables download, as
more of the table arrives you do reflows less and less frequently since it takes
longer to do each reflow.   The minimum time between reflows would still be set
in a preference.
Ideally (sorry), we'd do more reflows, not fewer, for big complicated tables, in
order to service UI events more often.  If a big reflow takes too long away from
the event loop, the UI starves.  But for layout to return early rather than take
too long (say, > 100ms) away from the event loop, you'd need something troy has
talked about before: interruptible layout.

The recent fixes to make layout incremental given content notifications at the
end of network buffers is not the same as making layout interruptible, is it?
You may need more buffering and states in your state machine to be able to stop
a reflow rather than "take too long", it seems to me.  Troy, what's the word?

/be
(Reporter)

Comment 6

19 years ago
With the way things are at the moment, even downloading content over a slow
modem line starves the UI for very long periods (try www.slashdot.org).
Someone mentioned that OS events will be given higher priority in the event
loop, which might help this problem.   Is this code in yet?
From my point of view, this RFE is basically to ease the end user experience, so
the content doesn't shift around too much.

I think the UI starving issues probably have resolutions other than this RFE.
If there's more to reflow, more CPU time will be used.  I don't see this is bad
if it doesn't starve the UI.

Comment 8

19 years ago
I don't think we're going to attempt to have interruptable layout, Brendan. Not
for beta, probably not for this version. There's plenty of tuning that can be
done to the way we do notifications and event queue prioritization. The checkin
this week was just a step along the way.

Troy, did the PL_Event change make it into apprunner?

Comment 9

19 years ago
The PL event change is XP. The Windows specific change to message pumping to
give priority to system events is in apprunner

As far as interruptible layout goes, there's no evidence to suggest that it is
the solution. What we haven't done (me included) is spend enough time to
understand what issues related to the sink change ere causing the UI to be
unresponsive

Comment 10

19 years ago
There are a lot of interrelated issues here:
1. the sink changes were intended to solve a certain category of problem, and I
think they have. That problem was the case where all the document's content was
inside a tag that was a child of the BODY. For example, everything in a CENTER
tag. The old sink only knew how to generate append notifications on the BODY and
so we only generated one append notification once the entire document was loaded

2. we're generating way too many notifications today. I looked at www.espn.com
and saw that we processed 142 reflow commands. That suggests (although I didn't
count) about that many append notifications

Having that fine a granularity will increase the overall document load time (we
all know that), and that's an obvious problem, but it can also hurt the UI
responsiveness problem as well.

The kind of problems I expect we're seeing (this is all speculatiion) are:
1. the overall incremental reflow process isn't tuned yet and so that constant
overhead is too high and needs to be reduced. An example of this is the way
WillReflow/DidReflow notifications are handled.

This is a known issue that I will be looking at, but it's a very large change
and it will take time

2. we don't coalesce reflow commands today. That means 142 (in the example
above) incremental reflows of the tree. That is going to suck

3. we don't yield while processing reflow commands. I mentioned this in the
previous discussion when I listed the top 5 problems causing UI responsiveness.

This means that if multiple reflow commands get generated each time the parser
calls the sink, then we'll enter the pres shell and pump all of the
pending reflow commands before returning

We need to treat reflow command processing as a scheduled activity just like
everything else.

That means we would go into the frame construction code and generate frames
(that's a potentially slow operation as well), that would cause reflow commands
to be generated, we would indicate we have reflow processing to do, and when
we're idle we do the processing

Anyone can make this change, and we don't need me to do this.

The tricky part is coordinating it all. We really want the reflow commands
processed _before_ we return back to the processor; otherwise, we won't reflow
any nothing willbe displayed

Maybe a nested event loop in the pres shell will work okay. A better scheme
would probably be to have a XP event processing mechansism with a prioprity
queue or some such mechanism

4. tables are not all that optimized for incremental reflow. They're getting
better, but it's a very big job.

That means if we generate a lot of append notifications (e.g., one per table
row) then it's going to both take a lot longer for total load time and be very
unresponsiveness

The total load time increase is obvious, but the unresponsiveness is a real
problem as well. If you imagine a really stupid table incremental reflow
algorithm (our's isn't this bad, but it's not optimal) that simple invalidates
the current data and forces a pass-1 reflow of the entire table as each new row
is added by the time you get to the last row in the table you're basically doing
almost as much work as you would have done had you buffered it up and reflowed
the entire table in one shot

If the UI was unresponsive when loading the table before Vidur's changes it will
be just as unresponsive now. Only worse, because adding the N-1 row is nearly as
slow and so with the N-2 row, ... For very large tables it's a real problem

Comment 11

19 years ago
To summarize, we need to do the following:

- fix the sink to cut down the number of append notifications
- tune overall event processing (socket notifications vs. system events)
- schedule reflow command processing (or yield while processing reflow commands)
- reduce incremental reflow overhead
- coalesce reflow commands
- improve table incremental reflow performance

On a separate note, on long documents we don't seem to be able to scroll while
the documeng is loading (at least on Windows). We should be able to because the
animation is going fine.

It may just be the fact that we need to tune the overall event processing code
(item #2 above), or it may be a different problem all together
Vidur, don't get me wrong -- interruptible layout doesn't look necessary to
me yet either, and it goes beyond anything Netscape ever shipped (a complicated
table could starve the UI in Nav1.0-4.x).  Thought I'd ask, however, since troy
and I had exchanged mail about it a while ago, and cuz michael.lowe described a
situation where fewer reflows would mean more time spent away from the event
loop (since we don't nest event loops from layout code).

Adding sfraser, who has measured why the Mac is much less responsive on certain
pages since the content notification changes went in, for the reason troy gives.
Also adding fur, whose timer-based progressive-jpeg idea seems to be not getting
considered directly -- is it inapplicable?

/be
(Reporter)

Comment 13

19 years ago
Brenden: From the users perspective I don't mind if reflows occur fairly
frequently (say 1 per second) until I get at least something on the page I can
start reading.   As more of a long document downloads though I don't want the
page shifting around as much, so I would like some heuristic that dampens the
frequency of page reflows as the download progresses (say 1 every five seconds
for the next 5 page fulls, then one every 30 seconds, etc).

Comment 14

19 years ago
*** Bug 17700 has been marked as a duplicate of this bug. ***

Updated

19 years ago
Severity: normal → critical
Priority: P3 → P1
Summary: {sink}[PERF]Limit number of incremental reflows → [PDT] [DEMO] {sink}[PERF]Limit number of incremental reflows

Comment 15

19 years ago
Bug 17700 is a [PDT] [DEMO] bug, and a duplicate of this one.  So I'm updating
this bug to be [PDT] [DEMO], and P1, Critical.

thx,
kevin

Comment 16

19 years ago
harishd plans to check in a fix today that should get rid of a bunch of extra
notifications that are happening with pages that have tables. I'm trying to plan
when to work on the timer-based notifications.

Comment 17

19 years ago
[in response to vidur's note on bug 17700]
on a fairly fast windows machine, the total load time is 1.4 seconds running a
debug build.  that's loading off the net, not local file.  from a local file,
the document loads almost instantly. so I don't think text controls are a
problem any more.

Updated

19 years ago
Summary: [PDT] [DEMO] {sink}[PERF]Limit number of incremental reflows → [DEMO] {sink}[PERF]Limit number of incremental reflows

Comment 18

19 years ago
Is this fixed now?  Was the fix checked in?

Comment 19

19 years ago
*** Bug 17905 has been marked as a duplicate of this bug. ***

Comment 20

19 years ago
Thank you.

You have fixed the speed problem I was facing (which I first filed though bug
17700, a dup of this bug).  The speed is back to its M10 days, for a page being
created for the Gecko demo.

I'm not sure if the scope of this bug extends beyond the speed problem I had,
but the speed problem is fixed.

Thank you.

-kevin
Well if there's no speed reason to reduce this anymore, it might have less
priority, but the ability for the user to reduce them might still remain.
(Reporter)

Updated

19 years ago
Severity: critical → major
Priority: P1 → P2

Comment 22

19 years ago
I was told that what I posted ( bug 17171 ) was pretty much a duplicate of this
bug, so I'll post my info here.    I did a little benchmark from my system
comparing IE5, Netscape 4.7, and Mozilla 1999110313
I have a PII-450 running Win98 on a Dual ISDN connection.

Sample page is:   http://slashdot.org/articles/99/11/03/1644219.shtml
(this is a dynamic page, so the results could possibly change from mine)

Time Until Page Interaction possible:
     Netscape 4.7 - 1 sec
     IE 5         - 1 sec
     Mozilla      - 40 sec

Time until entire page loaded:
     Netscape 4.7 - 5 sec
     IE 5         - 5 sec
     Mozilla      - 40 sec

As you can see, compared to other browsers, it's quite un-usable!

-WD

Comment 23

19 years ago
The slashdot page is definitely unusable, though my sense is that it's more
because of table layout inefficiencies than something controllable via the
granularity of reflows. I'm hoping that Troy can do some quantify analysis on
this page.

Comment 24

19 years ago
Just wanna know what's going on here.
I'd like to see a user defined time for incremental reflows (advanced
preferences) but reading through this bug page I see there are bigger problems
than I thought.

Updated

19 years ago
Target Milestone: M11

Comment 25

19 years ago
it would be a very good thing to fix this for m11
(Reporter)

Comment 26

19 years ago
Another site performing badly is http://www.lemonde.fr

Comment 27

19 years ago
As one person sayed it in the course of this discussion incremental reflow is
a gain from the user point of view when new content is added within the visible
area of the document.

But once the new content being added is no longer added to the visible part area
of the document I think we do not need incremental reflow very much.

That's why IMHO decision for reflows must be based also on visual feedback and
not only time considerations.  One of the purpose of this new architecture is
to improve the *apparent* rendering speed as I understand it.

Updated

19 years ago
Target Milestone: M11 → M12

Comment 28

19 years ago
Moving this off the M11 radar. The slashdot page, while it could still use
improvement, is actually better than it was in M10. Also, a forthcoming checkin
from Nisheeth should ameliorate some of the responsiveness issues (though that
will also need more work).

Comment 29

19 years ago
*** Bug 17552 has been marked as a duplicate of this bug. ***

Comment 30

19 years ago
I just checked in my changes that shave 2-4 seconds off the reflow time on this
page and make the UI a little more responsive.

Comment 31

19 years ago
I've noticed that M11 builds are much slower than M10 for displaying
http://my.netscape.com
But I have noticed an improvement since yesterday, which may reflect
nisheeth's work.

Here is some stop watch data that I gathered:
- on a LAN
- Pentium II/300MHZ laptop with 160MB RAM, Win98
- sidebar closed
- hit reload and measured time after initial launch

Test Run on a Pentium II/300MHZ laptop with 160MB RAM, Win98

                Test Run on 10/28/1999

              M10       M11 (1999102808) M11/M10
              3.8       11.5
              2.1        9.1
              2.5       10.6
              3.7       11.6
              2.2       10.8
 Average (sec)2.9       10.7               375%

               Test Run on 11/9/1999
              M10       M11 (199110908)    M11/M10
              2.3        7.0
              2.7        7.3
              3.7        7.3
              2.0        7.5
              2.8        7.4
 Average (sec)2.7        7.3                271%

Updated

19 years ago
Summary: [DEMO] {sink}[PERF]Limit number of incremental reflows → {sink}[PERF]Limit number of incremental reflows

Comment 32

19 years ago
Taking off [DEMO] radar.

Updated

19 years ago
Blocks: 18471
*** Bug 18921 has been marked as a duplicate of this bug. ***

Updated

18 years ago
Blocks: 17369

Updated

18 years ago
Whiteboard: [PDT+]

Comment 34

18 years ago
Putting on PDT+ radar.

Updated

18 years ago
Whiteboard: [PDT+] → [PDT-]

Comment 35

18 years ago
Changing to PDT- per a conversation with sfraser...not needed for dogfood.

Updated

18 years ago
Whiteboard: [PDT-] → [PDT+]

Comment 36

18 years ago
Hey, wait, I didn't say that _this_ could be PDT-. This should be PDT+++. Putting
+ back.

Comment 37

18 years ago
All right. I know nothing about the layout engine, so please take this comment
with a grain of salt. But if I understand this correctly, we're trying to find a
way of juggling (a) stuff appearing for the user to read and (b) overall speed
of loading.

So, here's a proposal. Do an incremental reflow whenever any of the following
situations occur:
* the connection has been closed (for whatever reason);
* (2 seconds + x) has passed since the last incremental reflow;
* a complete element of any of the types {DIV, TD, TH, P, BLOCKQUOTE, ADDRESS,
  FORM ... I've probably missed some, but you get the idea} has arrived, AND at
  least (x) has passed since the last incremental reflow.

x = t * M
t = the time which the previous incremental reflow of this page took to complete
    (so it's zero if this is the first rendering)
M = some constant (a value of about 6, probably, but this could be worked out in
    testing labs)

This way,
* we don't have to second-guess the speed of the computer, since we've
  stipulated that Mozilla should never spend more than 1/M of its time
  reflowing;
* we don't have to second-guess the speed of the connection, since we don't
  start a reflow just to display a small additional amount (unless it's been at
  least (2 seconds + x) since the last reflow);
* we don't need Yet Another Pref for setting the reflow interval, because
  Mozilla works the optimal interval out for itself as the page loads.

Updated

18 years ago
Whiteboard: [PDT+] → [PDT-]

Comment 38

18 years ago
Per discussion at PDT meeting: Changing to PDT-   Note that performance has
degraded notably in the past few weeks, and Hamerly has created a PDT+ bug that
reflects this need to fix the performance regression.  This "incremental
reflow" bug takes a stab at the cause of the regression, but really doesn't have
any ammo to demonstrate that it is the cause of, or potentially the solution to,
the regression problem.
Unless folks can show that this is **THE** performance bug (factor of 10 speed
regression in some cases), this should not be a focal PDT+ bug.  It will surely
be nice to get... but it is not stopping folks from using the browser (re:
dogfood).  In contrast, the other bug, with the factor of 10 regression, *is* a
dogfood bug.

Hope that explanation helps,

Jim

Comment 39

18 years ago
This is still an important problem, for (at least) two reasons:
1. The solution requires some degree of rearchitecture, which is better done
sooner than later.
2. The problem is bad enought that 1) it was one of the first things that people
mentioned when they tried M11, and 2) it is bad enough to prevent me from typing
in *another* application while a large page is loading, on Mac.

Comment 40

18 years ago
Here's another painfully slow URL
http://help.netscape.com/forms/bug-client.html
jar, what's the other bug's number?

/be
(Reporter)

Comment 42

18 years ago
It seems to me that the recent changes to the content sink were responsible for
the 10x performance degredation, so the additional content sink changes proposed
in this bug have a very good chance of bringing the browser back up to speed.
Are there any other proposed changes besides this bug that have the potential to
improve speed by 10x ?

Comment 43

18 years ago
What bug is jar referring to?  Bugzilla doesn't know about any bugs
with hamerly@netscape.com as reporter/QA/CC.  A review of the current
PDT+ bug summaries doesn't turn up anything that matches the description
given.

Comment 44

18 years ago
I agree with Michael's comment that more than likely the sink changes are the
cause of the 10x slowdown as well.

The other possibility would be the WipeContainingBlock() code that Kipp recently
added

Comment 45

18 years ago
*** Bug 19920 has been marked as a duplicate of this bug. ***

Comment 46

18 years ago
The reason I've thrown my votes at this bug is that I *hate* to have things move
around my screen. Once text shows up that I can read, I want it to stay put.
This is the only thing I really dislike about IE. Performance/speed is nice, but
a pleasant browsing experience is too.
skh2: A request to prevent any reflow on loading was filed as bug #17322 and
knocked back.  If this turns bug turns out to have a corresponding pref I guess
it could kind of be used for that.
(Reporter)

Updated

18 years ago
Whiteboard: [PDT-] → [PDT+]
(Reporter)

Comment 48

18 years ago
Changing back to PDT+ since the bug filed by Hamerly was found to be a duplicate
of this one.

Updated

18 years ago
Blocks: 20203

Updated

18 years ago
Whiteboard: [PDT+] → [PDT-]

Comment 49

18 years ago
Michael.lowe: Please don't change PDT status on bugs in bugzilla.
This bug was made PDT- by jar@netscape.com because (he said)
hamerly@netscape.com had filed another PDT+ bug that captured the performance
regression symptom, which must be fixed for Netscape dogfood (PDT+).  That bug
was not found by querying right after jar's update, and later, michael.lowe
seemed to find it and said it was a DUP of this one.  Meanwhile, I asked jar
what that other bug's number was, but got no answer.

Will someone please say what the other bug number is?  If it's not on file, or
was closed as a DUP, then this bug 17325 should remain PDT+.  So, what's the
other bug number?

BTW, mozilla.org doesn't want bugzilla status whiteboard wars, so as a matter of
courtesy, people in the Mozilla community shouldn't be changing one anothers'
whiteboard keywords.  This goes as much for netscape.com PDT managers not
removing users' DOGFOOD attributions as it does for michael.lowe not fiddling
with PDT+/-.  But I sympathize with michael, because the way this bug was marked
PDT- without the "other" bug being cited, and with the unsupported claim that
the performance regression was not (necessarily) caused by the problem this bug
describes (begging us to prove a negative), was a poor showing for the PDT+/-
label and its netscape.com designators -- in my humble opinion.

/be
(Reporter)

Comment 51

18 years ago
In my defence: the bug filed by Hamerly was Bug #19920, which was said by
jar@netscape.com to be a [PDT+] bug.   Troy marked that [PDT+] bug as a
duplicate of this bug, so I moved this bug back up to [PDT+] - it only seemed
logical.   Sorry if it seemed to be interfering.
Blocks: 17309

Comment 52

18 years ago
*** Bug 19491 has been marked as a duplicate of this bug. ***

Comment 53

18 years ago
http://www.voodooextreme.com is an excellent test page.
(Reporter)

Comment 54

18 years ago
Subject: checkin: timer-based notification in content sink
Date: 4 Dec 99 01:48:16 GMT
From: vidur@netscape.com (Vidur Apparao)
Organization: Another Netscape Collabra Server User
To: mozilla-layout@mozilla.org
Newsgroups: netscape.public.mozilla.layout

I just checked in a set of changes that allow for better tuning of
notifications done in the HTMLContentSink. For now, they are turned off
by default; they can be turned on via prefs.

A few weeks ago I checked in changes that caused the content sink to
more aggressively notify layout as it built content. This resulted in a
greater number of notification calls (at least one at the end of every
buffer of content sent to the parser by the networking library). The
good part of this change was quicker intial display of pages, especially
those with all of their content within a single body child (a table or
CENTER, for example). The bad part was less efficient layout since we
weren't batching as much new content as we were before.

One of the suggestions that came out of this change was to do
notifications based on a timer rather than relying on network chunking.
The current set of changes enable that. If the boolean pref
content.notify.ontimer is set to true, we now wait a specific interval
before sending notifications. The interval can be specified as a
microsecond value using the integer pref content.notify.interval (the
default if timer-based notification is turned on is 1 second). Finally,
a back-off count can be provided that controls how many intervals we
wait before just batching all of the remaining content. This value
allows the first part of the content to be displayed more incrementally,
while cutting down on the number of notifications (and hence increasing
the efficiency of layout) for the rest (presumably undisplayed) content.
A backoff count of 1 means we only do a timer-based notification once,
before chunking the rest of the content. A larger backoff count
increases the number of timer-based notifications.

This is one means by which we can tune the efficiency of layout.
However, this isn't meant to be a panacea of any sort. In fact,
experimenting with this change has emphasized to me the need for better
incremental layout (especially in tables). We may decide to turn on
timer-based notification for M12 or wait till more analysis and tuning
are possible. In either case, feel free to experiment with the prefs and
let me know if you see any problems.

--Vidur

Comment 55

18 years ago
An alternate tuning possibility is one that we used for progessive JPEG
decoding in the image library.  You could have two timeout interval parameters:
One that specifies the time from arrival of first parser data to the first
content notification for a document and one for subsequent content notification
intervals, the former being set to a lower value than the second.  In this way,
the initial display of a page is faster than the updates due to subsequent
appended content.

Updated

18 years ago
Target Milestone: M12 → M13

Comment 56

18 years ago
Moving out to m13 -- have a nice day.

Comment 57

18 years ago
I checked in a change to enable timer-based notification by default - the
interval and backoff values can be tuned later. I recognize that there is scope
for a lot more tuning in the content sink, but I'm tempted to close out this bug
for now. My hope is that slightly better incremental table layout (soon to be
checked in by karnaze) will help load times. Following that, I'll start
exploring other notification schemes.

Note that the voodooextreme page loading problems are exacerbated by the number
of SCRIPT elements in the page. I'm also going to explore better notification
mechanisms surrounding SCRIPTs (currently we force a notification before and
during). The voodooextreme page is discussed in bug 20485.

Again, unless anyone feels strongly, I will close out this bug at the end of
today.
(Reporter)

Comment 58

18 years ago
Could we morph this bug, or at least open another, as a meta bug for tracking
content sink performance work?

Comment 59

18 years ago
*** Bug 18426 has been marked as a duplicate of this bug. ***

Comment 60

18 years ago
So now all we need is some code which calculates the
interval-before-notifications-resume on the fly, based on how long the last
reflow took to complete. That way it'll work optimally no matter how slow/fast
the computer/connection is, without user intervention in setting the pref.
You might use the last few intervals, with digital filtering a la fur's
timer-based mozilla-classic compositing code near
http://lxr.mozilla.org/classic/source/lib/liblayer/src/cl_comp.c#1628

/be
(Reporter)

Comment 62

18 years ago
*** Bug 18948 has been marked as a duplicate of this bug. ***

Updated

18 years ago
Target Milestone: M13 → M15

Comment 63

18 years ago
This is more a tracking bug for now, so I'm moving it out of the current
milestone.

Updated

18 years ago
Keywords: perf
Summary: {sink}[PERF]Limit number of incremental reflows → {sink}Limit number of incremental reflows

Comment 64

18 years ago
Moving "perf" to keyword field.  This is the are we use now :-)

Comment 65

18 years ago
Removing PDT- now that dogfood nomination has been withdrawn.
Whiteboard: [PDT-]

Comment 66

18 years ago
*** Bug 25170 has been marked as a duplicate of this bug. ***

Comment 67

18 years ago
Troy landed some changes yesterday (fixed bug 27056), which were IMO a little 
bit to drastic. http://www.voodooextreme.com for example now lays out in one big 
chunk. In previous builds, each news-article triggered a reflow, which was 
quite cool.
cmattar -- did Troy's changes make layout faster or slower on your machine? 
Speed to document completion is 10^6 times more important than "coolness" of 
visual display right now and it's what we're optimizing for. We appreciate the 
feedback on the aesthetic appeal of the user experience, but what we're focusing 
on right now is increased performance, even at the expense of losing page layout 
visual effects that may seem cool to some. Thanks for your understanding, and 
any timing statistics you can provide by stopwatch would also be helpful!

Comment 69

18 years ago
What cmattr meant was that the news articles do not display (all 200 of them or 
whatever) until the entire page has finished loading.  So you sit there with the 
title and side bar for about a minute while the page loads and THEN it shows all 
the news articles.  One would think that incremental reflow might reflow a 
couple of times during the loading.

Comment 70

18 years ago
Yes, it would be nice if the table cell was incremental in that case. However, 
it's not all that common for people to put 300K of markup inside of a table 
cell.

Comment 71

18 years ago
1)Sorry for being that unspecific, but IMO table incremental reflow was really a 
great improvement over NN4.7 and IE.  I'm using Win98 on a Celeron 366@550 with 
128MB RAM. I've a ISDN-connection (8kb/s).
(Please note that these aren't 100% accurate, as the Status bar didn't update 
after the document was done, probably due to some animated gifs displaying. I 
had to rely on the time during which data transfer was displayed by windows).

M13 - 68seconds (TR incremental reflow enabled)
build 200021808 - 66seconds (TR incremental reflow disabled)
NN4.7 - 88seconds (note that there was a lot of HD-activity due to the 
disk-caching)

There were some improvements since M13, so the 2 seconds difference would 
probably be gone by now.
My machine is probably fast enough to handle the reflows. I'll try to dig out 
some stats for slower machines...

2) I agree, there aren't that many pages which have this problem. But hasn't 
there been a timer for the sink? If this would fire a incremental reflow at 
least every 10 seconds, it wouldn't be too much of a perfomance problem, but 
large pages would greatly benefit fromt this, esp. on slower connections (Here 
in Europe, many people still have modems with 4kb/s; don't know about the rest 
of the world, though)
(Reporter)

Comment 72

18 years ago
The time it takes to complete the loading of a page is usually *not important*, 
except for comparisons against other browsers in computer software reviews.   
What is more important is to have what the user wants to view on the screen 
quickly (ie. loading pages incrementally), even if that increases the total time 
to load the page.   For users like myself who are regularly limited to a slow 
modem connection, this *is* especially important.   Maybe if the Netscape 
developers were forced to use a modem for their browsing instead of the high 
speed connection they certainly have, their criteria for measuring page loading 
performance would be different (than the time to completely load a page).   To 
me the incremental page loading in Mozilla was the best improvment over NS4.x, 
but it has noticeably gone downhill recently (due to sync stylesheet loading, 
and other "improvements"?).    I can only hope that it will be fixed again soon, 
even if it has to become a global preference (ie. a preference like 'display 
complete page quickly or display page incrementally?').
(Reporter)

Comment 73

18 years ago
Maybe one performance criteria the Netscape engineers could use is the time it 
takes from the start of loading one of the Slashdot comments pages (eg. 
http://slashdot.org/articles/00/02/18/1313206.shtml) till the time the first X 
comments (eg. 20) are displayed on screen.   Such performance criteria are just 
as, or more important than total page load time.

Comment 74

18 years ago
Now that we fixed the Win98 performance problem things may be better and we can 
revisit the issue post-beta1

The problem we saw was that only really slow machines, e.g., as 90MHZ Pentium in 
the lab, even simple pages like www.excite.com were loading too slowly. Before 
the change it took 30 seconds to load www.excite.com. After we made the change 
to wait for complete table rows the page loaded in 6 seconds

I think your suggestion of using a timeout of say 10 seconds at which point we 
flush even if we're in the middle of a row is a good idea, and we'll look at 
doing that. You're right there's no point in going an extended period of time 
with nothing being displayed

Comment 75

18 years ago
If the problem is now differentiating between fast and slow machines (and I knew 

it would come to that eventually), I have to point at the algorithm I posted here 

earlier for calculating how soon the next reflow should be, based on how long the 

previous reflow took. No user should have to put up with having to fiddle such a 

setting manually, and my suggestion is a way of calculating it on the fly.

Comment 76

18 years ago
*** Bug 29912 has been marked as a duplicate of this bug. ***

Comment 77

18 years ago
Hints for jst@netscape.com, if he tries to implement the last suggestion about 
not waiting beyond a certain interval inside table rows:
1) The test in nsHTMLContentSink::IsTimeToNotify() checks the state of 
mInMonolithicContainer (set if we're inside a TR or a SELECT). If we are, it 
should further check how long it's been since the last flush.
2) We also check mInMonolithicContainer in nsHTMLContentSink::WillInterrupt. The 
code there should just set up a different delay timeout rather than none at all.
Moving forward to M16.
Target Milestone: M15 → M16

Comment 79

18 years ago
non-essential for m16
Target Milestone: M16 → M17
*SPAM* - adding mostfreq keyword to bugs with loads of DUPEs. Please aid this 
effort by adding this keyword to any bugs with more than 15 DUPEs.

Gerv
Keywords: mostfreq

Updated

18 years ago
No longer blocks: 18471

Updated

18 years ago
No longer blocks: 20203

Comment 81

18 years ago
*** Bug 45617 has been marked as a duplicate of this bug. ***

Updated

18 years ago
Depends on: 30942
more performance goodness; nominating for nsbeta3
Whiteboard: nsbeta3
I agree with dmose.  This *must* be nsbeta3+.  Performance over a modem is
unacceptably slow because of this bug (at least I assume it's still covered by
this bug, since the problem seems to be caused by the current solution to this
bug):  when loading a long document, one sees two updates at the beginning of
the load, and then *nothing* until the entire page is loaded, which is often
minutes.
Adding nsbeta3 keyword to bugs which already have nsbeta3 status markings so 
the queries don't get all screwed up.
Keywords: nsbeta3
Removing incorrect 'nsbeta3' from status whiteboard and reassigning to myself.
Assignee: vidur → jst
Status: ASSIGNED → NEW
Whiteboard: nsbeta3

Comment 86

18 years ago
Re-assigning to waterson for triage according to the priorities of the layout 
group.
Assignee: jst → waterson
(Assignee)

Comment 87

18 years ago
I'm closing this bug. It's meant too many different things, and it's time for it
to die. The timer work that was initially suggested has been done, even if there
are improvements that can still be made.

If there are issues with modem performance on specific pages, please file a new bug.

If somebody wants to track work related to tweaking the formulae used to reflow,
please file a new bug.
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 88

18 years ago
Marking veified per last comments.
Status: RESOLVED → VERIFIED
So this issue is now dealt with in bug 50634? Any other open bugs about this?

Comment 90

18 years ago
Filed bug 51202, for dynamically calculating the reflow interval depending on the 
speed of the connection and on the speed of the previous reflow.

Updated

17 years ago
No longer depends on: 30942
You need to log in before you can comment on or make changes to this bug.