Comment on attachment 8956504 [details] Bug 1443553: Devirtualize nsIDocument::AddObserver / RemoveObserver. https://reviewboard.mozilla.org/r/225420/#review231494

Comment on attachment 8956505 [details] Bug 1443553: Devirtualize ContentStateChanged / DocumentStatesChanged / StyleRule*. https://reviewboard.mozilla.org/r/225422/#review231496

Comment on attachment 8956506 [details] Bug 1443553: Devirtualize BeginUpdate, FlushPendingNotifications, CreatorParserOrNull. https://reviewboard.mozilla.org/r/225424/#review231498 Could you re-check that variables are defined only in one class. ::: dom/base/nsDocument.h:1040 (Diff revision 1) > // Array of owning references to all children > nsAttrAndChildArray mChildren; > > // Pointer to our parser if we're currently in the process of being > // parsed into. > nsCOMPtr<nsIParser> mParser; there is mParser here... ::: dom/base/nsDocument.h:1045 (Diff revision 1) > nsCOMPtr<nsIParser> mParser; > > // Weak reference to our sink for in case we no longer have a parser. This > // will allow us to flush out any pending stuff from the sink even if > // EndLoad() has already happened. > nsWeakPtr mWeakSink; So you have mWeakSink here... ::: dom/base/nsIDocument.h:1691 (Diff revision 1) > > // Observation hooks used to propagate notifications to document observers. > // BeginUpdate must be called before any batch of modifications of the > // content model or of style data, EndUpdate must be called afterward. > // To make this easy and painless, use the mozAutoDocUpdate helper class. > - virtual void BeginUpdate(nsUpdateType aUpdateType) = 0; > + void BeginUpdate(nsUpdateType aUpdateType); I guess things go wrong rather soon if one tries to override BeginUpdate in HTMLDocument or somewhere. ::: dom/base/nsIDocument.h:3786 (Diff revision 1) > // List of ancestor outerWindowIDs that correspond to the ancestor principals. > nsTArray<uint64_t> mAncestorOuterWindowIDs; > > + // Pointer to our parser if we're currently in the process of being > + // parsed into. > + nsCOMPtr<nsIParser> mParser; ...and there is mParser here. ::: dom/base/nsIDocument.h:3793 (Diff revision 1) > + nsrefcnt mStackRefCnt; > + > + // Weak reference to our sink for in case we no longer have a parser. This > + // will allow us to flush out any pending stuff from the sink even if > + // EndLoad() has already happened. > + nsWeakPtr mWeakSink; ...and there is mWeakSink here.

(In reply to Olli Pettay [:smaug] (unusual timezone [JST] for a week) from comment #6) > ::: dom/base/nsIDocument.h:1691 > (Diff revision 1) > > > > // Observation hooks used to propagate notifications to document observers. > > // BeginUpdate must be called before any batch of modifications of the > > // content model or of style data, EndUpdate must be called afterward. > > // To make this easy and painless, use the mozAutoDocUpdate helper class. > > - virtual void BeginUpdate(nsUpdateType aUpdateType) = 0; > > + void BeginUpdate(nsUpdateType aUpdateType); > > I guess things go wrong rather soon if one tries to override BeginUpdate in > HTMLDocument or somewhere. If you want I can revert the BeginUpdate changes. > ::: dom/base/nsIDocument.h:3786 > (Diff revision 1) > > // List of ancestor outerWindowIDs that correspond to the ancestor principals. > > nsTArray<uint64_t> mAncestorOuterWindowIDs; > > > > + // Pointer to our parser if we're currently in the process of being > > + // parsed into. > > + nsCOMPtr<nsIParser> mParser; > > ...and there is mParser here. > > ::: dom/base/nsIDocument.h:3793 > (Diff revision 1) > > + nsrefcnt mStackRefCnt; > > + > > + // Weak reference to our sink for in case we no longer have a parser. This > > + // will allow us to flush out any pending stuff from the sink even if > > + // EndLoad() has already happened. > > + nsWeakPtr mWeakSink; > > ...and there is mWeakSink here. Blerg, I had this fixed locally, sorry, apparently mozreview failed to push the changes before.

(In reply to Emilio Cobos Álvarez [:emilio] from comment #10) > > > > I guess things go wrong rather soon if one tries to override BeginUpdate in > > HTMLDocument or somewhere. > > If you want I can revert the BeginUpdate changes. > nah. If someone needs virtual BeginUpdate, that can be added then. (I have a wip patch somewhere trying to de-virtualize EndUpdate, but since it didn't help with performance after all, I didn't finish it.)

Comment on attachment 8956506 [details] Bug 1443553: Devirtualize BeginUpdate, FlushPendingNotifications, CreatorParserOrNull. https://reviewboard.mozilla.org/r/225424/#review231506

Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/9efb360ef2bc Devirtualize nsIDocument::AddObserver / RemoveObserver. r=smaug https://hg.mozilla.org/integration/autoland/rev/0b4aac4a6f1c Devirtualize ContentStateChanged / DocumentStatesChanged / StyleRule*. r=smaug https://hg.mozilla.org/integration/autoland/rev/ec884c383255 Devirtualize BeginUpdate, FlushPendingNotifications, CreatorParserOrNull. r=smaug

https://hg.mozilla.org/mozilla-central/rev/9efb360ef2bc https://hg.mozilla.org/mozilla-central/rev/0b4aac4a6f1c https://hg.mozilla.org/mozilla-central/rev/ec884c383255