Open Bug 357729 Opened 13 years ago Updated 7 years ago

Stop heap-allocating every CellData separately

Categories

(Core :: Layout: Tables, defect)

x86
Linux
defect
Not set

Tracking

()

People

(Reporter: bzbarsky, Unassigned)

References

(Blocks 1 open bug)

Details

(4 keywords)

Attachments

(2 files)

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...
Blocks: 348805
Keywords: footprint, perf
Blocks: 355940
Attached patch PatchSplinter Review
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...  :(
Blocks: 313295
Attached patch Updated to tipSplinter Review
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*.
Attachment #250929 - Flags: review?(bernd_mozilla)
Just realized bernd is not cced on here...
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&.
I kind of think that I'd like to fix bug 352461 before working more on this....
Depends on: 352461
And I've been thinking about this some more and I really don't like change #2 all that much...
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...
Deferring review, thats easy for me, somehow the Mozilla training finally pays.
Comment on attachment 250929 [details] [diff] [review]
Updated to tip

This is bitrotted anyway
Attachment #250929 - Flags: review?(bernd_mozilla) → review-
Boris what shall happen to this?
I'll try to fix it sometime, if no one gets to it first, but it might be a few months.
Keywords: testcase
Depends on: 862624
You need to log in before you can comment on or make changes to this bug.