stylo: Parallelize stylist::update

NEW
Assigned to

Status

()

Core
CSS Parsing and Computation
P1
normal
23 days ago
19 days ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

In my measurements, we spend about the same amount of time inserting elements into the rulehashes as we do into the dependency set. The SelectorMaps are disjoint, and so we could do this work in parallel.

We can even iterate the rules in opposite order, to potentially reduce cache contention on the refcount bumps (not sure if this is an issue or not, but doesn't hurt).

We could also potentially parallelize across the different rule hashes we use for the different cascade levels.

This touches all the stylist::update stuff, so I'm going to wait until bz finishes touching it first.
Note that @-moz-document support adding in bug 1355408 doesn't have thread-safe evaluation in its current form. Patches there have got r+ but haven't landed.

We can make it thread-safe via making the rule evaluated during parsing, but I thought that could be a low-priority optimization followup. If parallelizing stylist::update is high priority, we probably need to fix that first, either in that bug before its landing, or in another bug depending on it.
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #1)
> Note that @-moz-document support adding in bug 1355408 doesn't have
> thread-safe evaluation in its current form. Patches there have got r+ but
> haven't landed.
> 
> We can make it thread-safe via making the rule evaluated during parsing, but
> I thought that could be a low-priority optimization followup. If
> parallelizing stylist::update is high priority, we probably need to fix that
> first, either in that bug before its landing, or in another bug depending on
> it.

Does this happen at the same time as media query evaluation? In general it would be good to avoid doing the media query evaluation on each thread, which would mean doing it beforehand on the main thread somehow. If all that's the case, then we wouldn't need to worry about -moz-document OMT.

Doing the above would probably require collect()ing into a vec though, which might be a bit of overhead. That said, bz and emilio were discussion media query optimizations and caching, which might mean caching the resulting rules anyway? NI emilio on the planned setup there.
Flags: needinfo?(emilio+bugs)
Yeah, media-query evaluation is not thread-safe either AFAIK (probably can be done without too much effort). I'm not sure you can't do without evaluating the media queries on update(), what you'd do is avoiding to call update() itself when media query results haven't changed AFAIK.

That said, note that rule lists, both for stylesheets, media queries, and -moz-document are Arc'd, so collecting lists of rules that apply may be as simple as collecting a Vec<Arc<RuleList>>, which is probably ok if the wins from parallelizing it are enough.
Flags: needinfo?(emilio+bugs)
Does that mean that each stylesheet will have N+1 effective rule lists, where N is the nu,ber of media queries + number of moz-document rules? If so that's great news. The refcount traffic for that would be fine (whereas the refcount traffic for Arc<Rule> might not be).
Yes, I think so, at most N + 1 effective rulelists.
Ok great - then I think we should be able to just .collect() here, and don't need any thread-safety from @-moz-document handling.
You need to log in before you can comment on or make changes to this bug.