Open Bug 2021304 Opened 5 months ago Updated 3 months ago

Clearing CascadeData / SelectorMap causes significant main-thread jank

Categories

(Core :: CSS Parsing and Computation, defect, P3)

defect

Tracking

()

People

(Reporter: jstutte, Unassigned)

References

Details

(Whiteboard: [bhr:style::stylist::CascadeData::clear_cascade_data])

BHR (Background Hang Reporter) data from nightly (2026-02-28) shows that
CascadeData::clear_cascade_data (via Stylist::flush /
Servo_StyleSet_FlushStyleSheets) contributes approximately 55 seconds
of main-thread hang time (weighted by usage hours) in content processes.

The call chain is:

servo_arc::Arc<T>::drop
 -> SmallVec<A>::drop
  -> SelectorMap<T>::clear
   -> GenericElementAndPseudoRules<T>::clear
    -> CascadeData::clear_cascade_data
     -> Stylist::flush
      -> Servo_StyleSet_FlushStyleSheets
       -> ServoStyleSet::UpdateStylist()

This also appears in ShutDownKill crash data (nightly, last 7 days):

  • SelectorMap<T>::clear as top frame: 38 kills
  • servo_arc::Arc::drop in same path: 7 kills

The hang comes from dropping large numbers of Arc-wrapped selector
entries when clearing cascade data. Each entry requires an atomic
refcount decrement and potential deallocation.

For context, this is part of a broader pattern: servo_arc::Arc::drop
accounts for ~1,123s of total content-process hang time across all
callers. The biggest contributors are:

  • RestyleManager::ProcessPostTraversal dropping old ComputedValues (~250s, 22%)
  • RestyleManager::ClearServoDataFromSubtree (~150s, 13%)
  • Stylist::flush / CascadeData::clear (~55s, 5%) ← this bug
  • CSS parsing DeclarationBlock_Release (~30s, 3%)
  • Frame destruction (~19s, 2%)

Relevant code:

Related: bug 1406996 tracks crash reports in the same code path (Rust
HashMap operations), but the crashes are now very low volume (1 in 90
days). This bug is about the jank/hang impact.

Whiteboard: [bhr:SelectorMap<T>::clear]
Whiteboard: [bhr:SelectorMap<T>::clear] → [bhr:style::stylist::CascadeData::clear_cascade_data]
See Also: → 1809115

I don't think this is particularly actionable? I mean we could not clear it on shutdown or something, or defer it / make it more async I guess.

But those maps are just massive.

Severity: -- → S3
Priority: -- → P3

(In reply to Emilio Cobos Álvarez [:emilio] from comment #1)

I don't think this is particularly actionable? I mean we could not clear it on shutdown or something, or defer it / make it more async I guess.

I am not particularly concerned about shutdown hangs in this case, but more about jank. Defer and slice it into smaller runnables, if possible, may be a strategy that helps also during normal operations, but is probably not an easy thing to do.

But those maps are just massive.

Yeah, there is some smell here of something going out of normal. There might also be some more or less quadratic, nested loop across different functions and how they interact? Without having a profile it is a bit hard to tell what to look at. Well, we have a very sparse sampling through BHR, maybe one can try to see a pattern there.

See Also: → 2037326

Investigation update.

The BHR signal that motivated this bug — CascadeData::clear_cascade_data accumulating ~1.5M weighted ms·hr / 1839 hangs in nightly 2026-04-30 child BHR (12,639 install-hours) — comes essentially entirely from the document-stylesheet path: Servo_StyleSet_FlushStyleSheetsStylist::flushDocumentCascadeData::rebuildself.{user,author}.rebuild. ~0% comes through the shadow-DOM author-styles path (AuthorStyles::flushStylist::rebuild_author_dataCascadeDataCacheEntry::rebuild).

The two paths are structurally different. The cache-entry path constructs a fresh CascadeData per rebuild (via old.clone() or Self::new()), so its clear_cascade_data call had clone-then-clear redundancy that was straightforward to elide — that's now bug 2037326. But self.user / self.author are persistent populated CascadeData fields owned directly by DocumentCascadeData (servo/components/style/stylist.rs:433-434), so the clear there is doing genuine ownership-release on the prior cascade — no clone-then-clear redundancy to skip.

Reducing how often the CascadeInvalid/FullyInvalid path is taken at all would obviously be best (so more flushes land on DataValidity::Valid, which already skips clear at servo/components/style/stylist.rs:3444) and is a long ongoing optimization path; bug 1434145 is the meta-tracker for flush-frequency concerns in this area and is the natural home for that line of work. If we knew more about the bad behaving cases in terms of real sites we could maybe target those efforts better?

There might also be some more or less quadratic, nested loop across different functions and how they interact?

Nothing obvious jumped out to Claude (except bug 2037326).

Defer and slice it into smaller runnables, if possible, may be a strategy that helps also during normal operations, but is probably not an easy thing to do.

Indeed, what could specifically target the BHR signal here would be moving the per-rule-Arc refcount-down work that dominates the clear off the input-blocking critical section. The clear walks the rule-bearing containers and runs Drop per entry; the cumulative atomic-decrement cost surfaces as main-thread time even when total CPU is unchanged. Deferring that work to idle (via IdleTaskRunner with a per-slice budget) would pull it out of the input-latency window — total CPU stays the same, BHR sample distribution should shift. This may be less trivial as it sounds on the Rust→C++ side: lock ordering, shutdown drain, maybe even back-pressure if churn outpaces the idle drain. Still worth a try on this bug, possibly.

You need to log in before you can comment on or make changes to this bug.