105068 - merge mLazyState bits to save a word in slots

Status: RESOLVED FIXED

(Core :: XUL, defect, P3)

Description

[Chris Waterson]

The mLazyState field is used by XUL generated from RDF templates. Since these
nodes are all going to be ``heavyweight'' anyway, there is no reason to store
this value in the ``lightweight'' part of the XUL element. (Well, the topmost
node in every template also uses the lazy state, so we'd be faulting these nodes
-- but the number of these nodes is extremely small.)

Comment 1

[Chris Waterson]

Attachment: move mLazyState to nsXULElement::Slots

Comment 2

[Chris Waterson]

So, when bringing up a browser window, 14 out of 1750 nsXULElement objects get
their lazy state set: all of these are lightweights. There are 926 heavyweights.
nsXULElement is currently (before patch) 17 words, nsXULElements::Slots is 7
words. That means are memory usage is:

  1750 * 17 + 926 * 7 = 36232 words

With the patch, nsXULElement is 16 words, nsXULElements::Slots is 8 words:

  1750 * 16 + (926 + 14) * 8 = 35520 words

That's a net savings of 712 words, or about 2%. 

Hmm. Maybe we should stuff this state into the low bits of a pointer somewhere,
so we'd get

  1750 * 16 + (926 + 14) * 7 = 34580 words

to save 1652 words, or about 4.5%.

Comment 3

[Chris Waterson]

Bah, that'd be great to do, but I'll do it once I clean out all the other cruft
from the slots.

Comment 4

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 54042 [details] [diff] [review]
move mLazyState to nsXULElement::Slots

r=shaver.  How many bits are used in mLazyState, anyway?

Comment 5

[Chris Waterson]

Three. But I'd like to review their usage, too. A lot of that stuff was cooked
up on too much caffeine and may be unnecessary.

Comment 6

[David Hyatt]

sr=hyatt

Comment 7

[Chris Waterson]

Okay, I've checked in attachment 54042 [details] [diff] [review]. I'll leave this open as a reminder to
merge bits once stuff has been cleaned up a bit more.

Comment 8

[Chris Waterson]

Attachment: collapse mAttributes and mLazyState

To save another word in the mSlots, this patch collapses mLazyState and
mAttributes: mLazyState takes the low three bits, mAttributes takes the high
bits.

Comment 9

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 66021 [details] [diff] [review]
collapse mAttributes and mLazyState

I tried really hard to find something to complain about here, and came up
empty.

I guess that means r=shaver.

Comment 10

[David Hyatt]

Comment on attachment 66021 [details] [diff] [review]
collapse mAttributes and mLazyState

sr=hyatt

Comment 11

[Chris Waterson]

Fix checked in.

Comment 12

[Brendan Eich [:brendan]]

To be 64-bit safe, you want

+#define LAZYSTATE_MASK  (((JSWord)1 << LAZYSTATE_BITS) - 1)
+#define ATTRIBUTES_MASK (~LAZYSTATE_MASK)

(note the (JSWord)1 rather than 1 in the first line).

/be

Comment 13

[Chris Waterson]

Just made that change. Thanks.