60403 - layout reflow states use too much stack space

Status: NEW

(Core :: Layout, defect, P2)

Description

[buster]

Reflow states are created on the stack, one per child frame.  Each reflow state
is over 150 bytes. So the total amount of stack space used by a reflow state is
more than (150 bytes * max_depth_of_frame_tree), where the max depth for a
"normal" page is typically double digits, but for some documents can get into
the hundreds.  The difference between one reflow state and it's child are
typically very few.

We should be able to be clever about how we create reflow states.  We should be
able to create a class that only contains the differences between one reflow
state and another, for example.  Maybe we create a class that only holds the
info that is unique to it, and walks up the reflow state tree to find other info.

Alternately, we could create a "core" reflow state and an "auxiliary" reflow
state.  The core is always created, and it contains the major attributes.  The
"auxiliary" reflow state is created whenever any attribute within it is
different than it's parent.  Otherwise, it just points up the hierarchy to the
one that contains the common shared info.

Comment 1

[buster]

moving to mozilla0.9

Comment 2

[buster]

moving to mozilla0.9

Comment 3

[buster]

Some thoughts:
1. the cached style struct pointers might be a waste of space.  We get style
structs from frames all the time.  we should look in the profile and see if
there is any significant time savings to keeping these cached pointers around,
and if not get rid of them (24 bytes per context)
2. there is no real attempt to pack bits together. there are 2 bools,
nsCSSFrameType which is currently 32 bits but could easily be just 8 bits (or
less), and nsReflowReason which is currently 32 bits but could easily be 8 bits
(or less).  mReflowDepth need not be 32 bits.

Comment 4

[karnaze (gone)]

Moving to mozilla1.0

Comment 5

[Kevin McCluskey (gone)]

Build reassigning Buster's bugs to Marc.

Comment 6

[Marc Attinasi]

Until we know how much bloat the reflow state contributes, and how much would be
saved by making changes, I'd suggest we put this on hold and NOT try to fix ot
for Mozilla 1.0  - it is too unclear if there is any significant benefit to
this, and we are short on resources at it is.

BTW: reflow state is allocated on the stack, during reflow only.  The bloat it
contributes is transitory and will not cause fragmentation of the heap either[1].

(1) there are some cases where we use the heap to allocate reflow states, but it
is very rare.

Comment 7

[Markus Hübner]

Any info in the meantime how much bloat the reflow state contributes (resp. 
how much we could save)?

Comment 8

[Markus Hübner]

Reassigning to [e-mail] until someone is willing to take this.

Comment 9

[Boris Zbarsky [:bzbarsky]]

How about getting the component right too, huh?

Comment 10

[Boris Zbarsky [:bzbarsky]]

How about getting the component right too, huh?

Comment 11

[Brendan Eich [:brendan]]

Why do we need reflow state and all the code required to manage it?  Wouldn't 
brute force approach that kept state in the content and frame trees work as 
well, or nearly so, in terms of dynamic memory -- and much better in terms of 
code footprint and simplicity, eliminating bugs?

/be

Comment 12

[Boris Zbarsky [:bzbarsky]]

Unfortunately, that could be fairly costly in terms of static footprint... We 
have a _lot_ of frames; making them all bigger by a few hundred (or even a few 
dozen) bytes each could be very painful...

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We could just move reflow states to a linked list on the heap (with recycling so
we don't thrash the heap during reflow).