We're running out of node bits

RESOLVED FIXED in mozilla34

Status

()

defect
RESOLVED FIXED
5 years ago
4 months ago

People

(Reporter: bzbarsky, Assigned: Ms2ger)

Tracking

(Depends on 1 bug)

unspecified
mozilla34
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

We have two bool bits left, and only one non-bool bit.  That's not counting the event state, which we have 18 free bits in, if we want to try using them somehow, of course.

Looking at our existing bits, we can nix ParentIsContent and replace its one use with:

  nsIContent* GetParent() const {
    return mParent && mParent->IsContent() ? mParent : nullptr;
  }

That does mean reading stuff from the parent, which may mean more cache misse, but the caller was probably going to anyway.  Worth doing that?

Any other bits we can obviously kill off?

Or should I just not be worrying about this until the next poor sod tries to add flags and gets compile errors?
Killing XBL would free up several ...

Is directionality important enough to chew up two of them?  

NODE_ATTACH_BINDING_ON_POSTCREATE appears to be unused.  That's almost too good to be true.
> Killing XBL would free up several ...

Yes, but that's not a short-term project.

> Is directionality important enough to chew up two of them?  

Directionality uses two non-boolean flags and 6 boolean flags.  I couldn't find obvious ways to nix some of them.  :(

> NODE_ATTACH_BINDING_ON_POSTCREATE appears to be unused.

Oho!  It is in fact unused.  It went away in bug 883892.  We used to set the flag in PreCreate (because we had to check for XBL there to decide whether we can do slimwrappers) and check it in PostCreate (to avoid redoing all that checking work), but now Element::WrapObject just does all the work involved, so there's no need to smuggle state along.
Posted patch Patch v1Splinter Review
Attachment #8388060 - Flags: review?(khuey)
Comment on attachment 8388060 [details] [diff] [review]
Patch v1

I would prefer that bz reviewed this, mostly because I'm not sure if there's anywhere else that depends on the numbers involved here.
Attachment #8388060 - Flags: review?(khuey) → review?(bzbarsky)
Comment on attachment 8388060 [details] [diff] [review]
Patch v1

This should be good.  r=me
Attachment #8388060 - Flags: review?(bzbarsky) → review+
Depends on: 986730
Depends on: 988842
No longer depends on: 986730
ms2ger, do you plan to land that?
Flags: needinfo?(Ms2ger)
Can we wait to land it until I'm ready to land bug 996796?
Blocks: 996796
Sure.  You're the main consumer for this patch right now!
I forgot about this... Let me know when/if I should land it.
Flags: needinfo?(Ms2ger)
https://hg.mozilla.org/mozilla-central/rev/267e2477f763
Assignee: nobody → Ms2ger
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.