We need to manage to either make them faster, or call them less... I'd hope the first :)

Emilio says this will also be an issue for Shadow DOM.

Comment on attachment 8953846 [details] Bug 1422653: Compute whether XBL is involved in ChildIterator lazily. https://reviewboard.mozilla.org/r/223036/#review229750 ::: dom/base/ChildIterator.h:155 (Diff revision 1) > , mXBLInvolved(aOther.mXBLInvolved) > , mOriginalContent(aOther.mOriginalContent) > {} > > - bool XBLInvolved() { return mXBLInvolved; } > + bool XBLInvolved() { > + if (!mXBLInvolved) { nit: I'd prefer s/!mXBLInvolved/mXBLInvolved.isNothing()/ for clarity. ::: dom/base/ChildIterator.h:184 (Diff revision 1) > + // This is lazily computed when asked for it. > + Maybe<bool> mXBLInvolved; > > const nsIContent* mOriginalContent; > }; I dislike Maybe<T> members in general (because they usually double the memory requirement for the member due to alignment) and Maybe<bool> in particular since "if (mSomeBooleanSoundingName)" is *very easily* misread. It's a footgun IMO. I guess it's fine here, if we always use the accessor method to test the value. Can we enforce that by making the member private? Please move the member last to avoid alignment spill.

Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/283b2a4cb219 Compute whether XBL is involved in ChildIterator lazily. r=mats

Comment on attachment 8953846 [details] Bug 1422653: Compute whether XBL is involved in ChildIterator lazily. https://reviewboard.mozilla.org/r/223036/#review229750 > nit: I'd prefer s/!mXBLInvolved/mXBLInvolved.isNothing()/ for clarity. Done. > I dislike Maybe<T> members in general (because they usually double the memory requirement for the member due to alignment) and Maybe<bool> in particular since "if (mSomeBooleanSoundingName)" is *very easily* misread. It's a footgun IMO. > > I guess it's fine here, if we always use the accessor method to test the value. > Can we enforce that by making the member private? > Please move the member last to avoid alignment spill. Yeah, fair. `Maybe<bool>` in this case isn't really worse than two bools. I could use bitflags to pack it a bit more but it seemed unnecessary trouble, given this is a stack type anyway. Made that and the function private before landing. Thanks for the review!

Backout by ebalazs@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9268243e4040 Backed out changeset 283b2a4cb219 for bustage in /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:6840:26 on a CLOSED TREE

Comment on attachment 8954684 [details] Bug 1422653: Make FindSiblingInternal take the iterator by ref to please the static analysis. https://reviewboard.mozilla.org/r/223790/#review229998

hg error in cmd: hg rebase -s 84429c176d221ffa1c60b68543cbd71e6b0c9f8f -d 13eacb5892bb: abort: unknown revision '13eacb5892bb'!

Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/df7075a430f4 Make FindSiblingInternal take the iterator by ref to please the static analysis on the next patch. r=mats https://hg.mozilla.org/integration/autoland/rev/522aa8e25bd6 Compute whether XBL is involved in ChildIterator lazily. r=mats

https://hg.mozilla.org/mozilla-central/rev/df7075a430f4 https://hg.mozilla.org/mozilla-central/rev/522aa8e25bd6