357729 - Stop heap-allocating every CellData separately

Status: NEW

(Core :: Layout: Tables, defect)

Description

[Boris Zbarsky [:bzbarsky]]

See discussion in bug 348805.  This could save a good bit of memory on table-intensive pages, even with the switch to the presshell allocator that I plan to make in bug 356335.

Unfortunately, the problem is how to do this while hiding the different storage from most cellmap consumers...

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Need to do a self-review -- merging in bug 356335 was painful.  bernd, I think you should have just reviewed this patch to start with; it's almost as big as it was before I landed bug 356335...  :(

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated to tip

This basically looks good to me.  If there are tests I should run, what are they?

Summary of changes:

1)  Introduce a "base" cellmap class and make templated cellmaps inherit from
    it.
2)  Put some checks for mBCInfo and casts in nsTableCellMap so I don't have to
    templatize that class.  Hopefully this is not a performance issue...
3)  Macro-ize a lot of the nsTableCellMap code, since I was going to have to
    change it anyway.  This basically fixes bug 355940.
4)  Some const-ness and type-safety fixes; in particular, the new code is a lot
    better at enforcing that the return value of GetDataAt on a const cellmap is
    a const CellData* or BCCellData*.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Just realized bernd is not cced on here...

Comment 8

[Boris Zbarsky [:bzbarsky]]

Oh, one other important change:

5)  All places where we had null celldata we should now have dead celldata,
    except GetDataAt when it's asked for an out-of-bounds location.  As a
    followup, I do think we should make GetDataAt return an nsCellData&.

Comment 11

[Boris Zbarsky [:bzbarsky]]

I kind of think that I'd like to fix bug 352461 before working more on this....

Comment 12

[Boris Zbarsky [:bzbarsky]]

And I've been thinking about this some more and I really don't like change #2 all that much...

Comment 14

[Boris Zbarsky [:bzbarsky]]

So the point is, don't review this quite yet.  ;)  I'll do some more thinking about a way to implement this that doesn't duplicate quite so much code...

Comment 15

[Bernd]

Deferring review, thats easy for me, somehow the Mozilla training finally pays.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 250929 [details] [diff] [review]
Updated to tip

This is bitrotted anyway

Comment 17

[Bernd]

Boris what shall happen to this?

Comment 18

[Boris Zbarsky [:bzbarsky]]

I'll try to fix it sometime, if no one gets to it first, but it might be a few months.