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...
Created attachment 247783 [details] [diff] [review] 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... :(
Created attachment 250929 [details] [diff] [review] 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*.
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&.
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
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.
4 years ago