If we wait until nodes are deleted to clear the ServoNodeData, then we will miss a last chance to GC the Servo RuleTree, which happens when the Stylist is dropped, and we leak the RuleNodes. The simplest thing would be to iterate over the document and clear all the ServoNodeDatas when the ServoStyleSet is being shut down, but before it drops the RawServoStyleSet. Timing on my machine, iteration over the single page HTML spec without doing the actual ServoNodeData dropping takes ~100ms, which is about 0.3 us per node we iterate over (there being ~340,000 nodes in the document).

Comment on attachment 8811586 [details] GC the RuleTree when the Stylist is dropped. https://reviewboard.mozilla.org/r/93658/#review93750 ::: servo/components/style/selector_matching.rs:102 (Diff revision 1) > non_common_style_affecting_attributes_selectors: Vec<Selector<TheSelectorImpl>>, > } > > +impl Drop for Stylist { > + fn drop(&mut self) { > + unsafe { self.rule_tree.gc(); } Can you add a comment here with why is it needed?

One strange thing: at the last minute, I moved the ClearAllStyleData call from BeginShutdown to Shutdown, but I realize now that the leak persists because of that. Moving it back fixes it. Don't know why...

Comment on attachment 8811587 [details] Mark the Gecko main thread as a Servo layout thread. https://reviewboard.mozilla.org/r/93660/#review93906

Comment on attachment 8811585 [details] Bug 1318238 - Clear all ServoNodeData during style set shutdown. https://reviewboard.mozilla.org/r/93656/#review93912 Hm, so that traversal overhead is significant - about 10% of a full-document restyle IIUC. I think we should probably do the thing you suggested where we transfer ownership of the style set to the document. However, I don't want to get in the way of fixing the leaks. Can you file this as a followup blocking stylo-perf and we'll get to it later? ::: layout/base/ServoRestyleManager.cpp:94 (Diff revision 1) > + aContent->ClearServoData(); > + aContent->SetIsDirtyForServo(); Can you also clear the dirty descendants bit here? ::: layout/style/ServoStyleSet.cpp:42 (Diff revision 1) > + // It's important to do this before mRawSet is released, since that will cause > + // a RuleTree GC, which needs to happen after we have dropped all of the > + // document's strong references to RuleNodes. Add a comment here about perf.

Oh, and let's call it "ClearServoDataFromSubtree" instead of "ClearAllStyleData".

Pushed by cmccormack@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d8bea653cdb7 Clear all ServoNodeData during style set shutdown. r=bholley