51022 - [MF]form control frames are badly bloated

Status: RESOLVED WORKSFORME

(Core :: Layout: Form Controls, defect, P3)

Description

[buster]

many of the form control frames are badly bloated.  with very little work, we 
could shrink these down considerably.

for example, nsGfxListControlFrame contains 10 member variables of type PRBool.  
At 4 bytes each, this is 40 bytes per instance that can easily be squeezed into 
a 2-byte bit field.  This would save us 38 bytes per instance.  If you use 
inline getter and setters on the bit field, the performance cost is negligible.

Also, 32-bit values are used for the indexes.  Do we really ever expect a list 
box to contain 2^32 elements?  Would 16 bit values be sufficient?  If so, we 
could save an addition 16 bytes per instance.

Also, it looks like mPresContext could easily be removed by simply adding the 
pres context as a parameter to methods that need it.  I think it's always 
available to the callers.  That saves another 4 bytes, plus an addref/release 
and potential source of leak.

I suggest rod and/or peter spend a day or two knocking off the easy bloat kills 
in these classes.  Peter has already done this kind of work in some layout 
classes, it might be a good use of his time with a little help from rod.

Some of the classes in the html/forms/src directory are no longer used.  Be sure 
to only focus on the classes we're going to ship with Sea Monkey.

Other things I notice:
1) nsFormFrame contains nsVoidArray mRadioGroups.  Is mRadioGroups always used, 
or only if the form really contains radio buttons?  If the latter, maybe it 
should be a pointer, to avoid extra bloat in forms without radio buttons.

2) nsFormControlFrame holds onto the pres context.  Can that be removed by 
simply passing it around in needed methods?

Comment 1

[rods (gone)]

Yes, I have been waiting for this one, but we may be switching to XBL form 
controls in 6.1 so I haven't invested a lot effort in cleaning it up like it 
really needs (this and nsComboboxControlframe). I wouldn't nominate it for 
nsbeta3, too many changes. 

Also, if I remember correctly there are times when I need a mPresContext and it 
isn't available, but I will make sure.

Comment 2

[buster]

since the switch to XBL is definately post-6.0, and the amount of work to make 
large improvements is very low (a day at most), and very low risk (just 
switching how boolean values are stored, for example, not changing anything 
about the semantics of the variables' use), I'd suggest spending a day on this 
before FCS.  It doesn't have to be Rod, though he should own the problem.  Peter 
is a potential resource.

Bloat is a real problem in the 6.0 timeframe.  I think it's worth persuing 
low-risk, low-cost fixes.

Comment 3

[buster]

Just want to be clear:  I don't think it's required to do everything on the list 
in this bug for 6.0.  But I think it's worth a day to do what we can, and 
changing the booleans is a very easy task.  If that's all that happened, that 
would solve 80% of the problem.

Comment 4

[ckritzer (gone)]

Buster, would you flag this bug when this work you're talking about is 
completed?  Thanks dude... - ckritzer

Comment 5

[bsharma]

Updating QA contact.

Comment 6

[rods (gone)]

I wouldn't say badly bloated, but they could use a little trimming down

Comment 7

[Kevin McCluskey (gone)]

Set milestone to mozilla0.9

Comment 8

[Vladimir Ermakov]

QA Contact Update

Comment 9

[Kevin McCluskey (gone)]

Setting milestone to future. The memory savings is minimal.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

comment 0 uses bitfields now. I don't think this bug as-is is particularly actionable.