If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Stop heap-allocating every CellData separately

NEW
Unassigned

Status

()

Core
Layout: Tables
11 years ago
4 years ago

People

(Reporter: bz, Unassigned)

Tracking

(Depends on: 1 bug, Blocks: 1 bug, 4 keywords)

Trunk
x86
Linux
footprint, helpwanted, perf, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

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
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...  :(
Blocks: 313295
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*.
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...

Comment 15

11 years ago
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-

Comment 17

10 years ago
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.

Updated

9 years ago
Keywords: testcase
Depends on: 862624
You need to log in before you can comment on or make changes to this bug.