67235 - need a general mechanism for marking incremental reflows as constrained

Status: RESOLVED INVALID

(Core :: Layout, defect, P3)

Description

[buster]

broken off from bug 58148.

During the NS6 push, I added an embarrassing piece of code called
nsBlockFrame::IsIncrementalDamageConstrained().

The name was intended to hint at the general problem:  to be efficient, we need
to identify incremental reflows whose changes are constrained to a particular
child.  If we know that an incremental reflow has no impact outside it's local
scope, then we can skip all kinds of brute-force code that basically says: "an
incremantal change has happened, and I don't know what has changed, so reflow
all my children (or recompute all sorts of data, or whatever.)  A trivial
example of this is typing into a light=weight text field.  Typing causes an
incremental reflow, but there's no way that the result of that reflow can change
the dimensions of the text control itself.  (Oh, sure, you could contrive a case
where an OnKeyDown event handler changed the style or an attribute of the text
field, but we'll capture that as a separate styleChanged reflow anyway, so
that's not relevant here.)

Today, nsBlockFrame::IsIncrementalDamageConstrained() simply checks to see if
the target of the incremental reflow is inside a text control or not.  It should
be much more general.  In particular, we should keep some state in
nsHTMLReflowState (probably), most likely just a bit.  The target frame itself
could set the bit, or any container along the way.  For example, in the case of
 typing in a text field, it would be the text field frame itself that knew it's
dimensions hadn't changed.

Comment 1

[buster]

reducing the number of reflows has all kinds of great perf benefits, so I'm
nominating this for moz1.0.

Comment 2

[Marc Attinasi]

Sounds great! I'm on it.

Comment 3

[Kevin McCluskey (gone)]

*** Bug 67495 has been marked as a duplicate of this bug. ***

Comment 4

[Markus Hübner]

buster, what is the target of this bug now?

Comment 5

[Ronald van Kuijk]

Does solving this 'bug' improve DHTML performance or is that a 'change style' issue?

Comment 6

[Marc Attinasi]

The real benefits of fixing this bug are unknown, and the scope of the change is
large (probably impacts all of reflow).

Until we can quantitatively show a significant improvement for important classes
of performance problems, I'd strongly urge that this bug be left alone, at least
until we have more resources to devote to experimental and potentially risky
changes.

Comment 7

[Markus Hübner]

Did things change in view of fixing bug 129115 ?

Comment 8

[Chris Waterson]

Yes, this is invalid. A frames that can constrain damage (i.e., reflow within
the frame is known not to affect any ancestor or sibling frames) should become a
`reflow root'. A reflow should be dispatched from the frame.