Status

()

Core
XUL
P3
normal
18 years ago
10 years ago

People

(Reporter: buster, Assigned: Eric Vaughan)

Tracking

({helpwanted})

Trunk
Future
x86
Windows NT
helpwanted
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta3-])

(Reporter)

Description

18 years ago
I spent a few minutes scanning for easy bloat kills in the layout directory.  In 
XUL, there are a few classes that can be shrunk a bit with very little effort.

1) nsTitledButtonFrame has 6 member variables of type PRBool, consuming 24 
bytes.  These could easily be held in a single byte bitfield, with inline 
getters and setters.  We do this all over layout in frame classes.

2) generally, whereever a class has a member of type PRBool, it should be 
changed to PRPackedBool if there are just one or two of them, and a bitfield 
should be used if there are more than that.  nsBoxSize, nsComputedBoxSize, 
nsSliderFrame, nsScrollBoxFrame, nsMenuPopupFrame, nsBoxToBlockAdaptor, etc...

3) nsMenuFrame contains these 10 bytes
  PRPackedBool mIsMenu; // Whether or not we can even have children or not.
  PRPackedBool mMenuOpen;
  PRPackedBool mCreateHandlerSucceeded;  // Did the create handler succeed?
  PRPackedBool mChecked;              // are we checked?
  nsMenuType mType;
all of which could be collapsed into a single byte bitfield.

4) several classes cache a pointer to the pres context.  in other layout 
classes, I've noticed this pointer can often be removed if API's are expanded to 
include passing the pres context around.  it's generally available, your mileage 
may vary...


My opinion is that it's worth 1 day of work from somebody in xp toolkit to knock 
off the easy kills and see how much overall footprint improvement we get.  
PeterL has started doing some of this work in layout, maybe he could be 
convinced to do a once-over on these classes as well.

Comment 1

18 years ago
Titledbutton is obsolete and can be removed from the tree.

Comment 2

18 years ago
Sending up to trudelle.
Assignee: hyatt → trudelle

Comment 3

18 years ago
UP???  Very funny Dave!
If you think these are worth the effort, risk and and the tradeoff between space
and access time, then let's talk about who can do it.  I'd also like to get a
swag at how much bloat this really amounts to.
(Reporter)

Comment 4

18 years ago
I think the risk is extremely low.

The savings for 1-3 depends on how many classes really use PRBools 
inefficiently, how many instances of these classes are allocated, and whether 
these instances are heap or stack allocated.  From my quick look, I would say 
there are 10-20 classes that could be cleaned up, with an average savings of 
10-20 bytes per class.  Most of these are heap allocated, so the savings is 
somewhat important in terms of improving total bloat, working set size, and 
performance (memory fragmentation, etc.) As to how many of these classes are 
actually allocated, that depends on your application, and whether allocation for 
them persists or not.  For example, if we cache the frames used to construct a 
dialog after first instance, the savings will matter more than if we rebuild the 
frames each time.

I think the amount of time this would take is trivial, probably a day including 
testing and getting it checked in.  I've been suggesting Peter L. as a resource 
for this kind of work, though I haven't heard from him or his manager about 
whether that's a good use of his time or not.

Hope this helps.

Comment 5

18 years ago
Thanks Steve, that does help, although I'd still like to hear Hyatt's SWAG of
total bloat caused by these, and the time/space tradeoff.  Also, one engineering
day is not trivial to me, especially with only 13 of them left (including a long
holidya weekend) before we're supposed to be finished - it is a precious commodity.
nominating for nsbeta3
Keywords: nsbeta3

Comment 6

18 years ago
Thanks, we will address this in the future, but currently XUL frame bloat is 
dwarfed by style contexts and strings held by content nodes.  This will be 
a small win, with questionable effect on runtime performance, and we can't 
afford the time or risk.  nsbeta3-/future
Whiteboard: [nsbeta3-]
Target Milestone: --- → Future
(Reporter)

Comment 7

18 years ago
I don't want to push too hard, but what if someone outside the netscape team did 
the work?  Would you judge it low enough risk to allow in?
Keywords: helpwanted

Comment 8

18 years ago
We encourage patches of course, but can't promise to take them sight unseen, or
on any particular schedule. These changes would only save a small amount of RAM,
in an area that is barely a blip on the bloat radar, and packing the booleans
may even introduce performance regressions. We just don't think this is the
right thing to be working on right now. If you know of someone who could improve
this, I think they'd be much more help working on something that really mattered
instead.  It doesn't make much sense to stick your finger in a hole in the dike
when water is cascading over the top.  Grab some sandbags!
Don't worry about pushing back, we want to do the right thing too.  Escalate to
PDT if you think that would be appropriate.

Updated

18 years ago
Status: NEW → ASSIGNED

Comment 9

17 years ago
->evaughan, untargetted
Assignee: trudelle → evaughan
Status: ASSIGNED → NEW
Target Milestone: Future → ---

Comment 10

17 years ago
->moz1.0
Target Milestone: --- → mozilla1.0

Comment 11

16 years ago
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)
Target Milestone: mozilla1.0 → mozilla1.0.1

Comment 12

15 years ago
retargeting
Target Milestone: mozilla1.0.1 → Future

Updated

10 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: jrgmorrison → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.