1311473 - stylo: Investigate the performance impact of AtomicRefCell

Status: RESOLVED WORKSFORME

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

Description

[Bobby Holley (:bholley)]

This one is like bug 1311469, but more straightforward.

AtomicRefCell is a structure I added that wraps parking_lot::RwLock with RefCell semantics. This means that it's basically a runtime safety check (i.e. the only time it ever does something observable is when it panics).

Cribbing from bug 1311469, AtomicRefCell borrows are cheap, but not free. In my testing, a mutable borrow/release pair is about 10ns, and an immutable one is 20. For comparison, an Arc borrow/release pair is about 13ns.

So we should measure if it the overhead is noticeable. If it is, we should either optimize the callers, or consider compiling out AtomicRefCell in stylo release builds.

Comment 1

[Bobby Holley (:bholley)]

A few things here:
* Since this bug was filed, I rewrote AtomicRefCell from scratch to be twice as fast (it's now at [1]).
* The runtime checking of AtomicRefCell buys us a lot of safety.
* In my measurements this far, traversal overhead only been a couple of percent of styling time (the rest is in compute_style). So this probably isn't a very large optimization target for the Servo traversal.
* It's possible that we may see overhead when Gecko needs to pull things out of the AtomicRefCell (i.e. during frame construction). We might consider an unsafe borrow if that stuff shows up in profiles.
* Bug 1335941 amortizes the overhead of synchronization for stylesheets to be negligible.

I think we can close this bug as-filed.

[1] https://crates.io/crates/atomic_refcell