1304244 - stylo: Consider grouping inherited and reset style struct pointers into intermediate structs

Status: RESOLVED WONTFIX

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

Description

[Bobby Holley (:bholley)]

Whenever servo cascades a node, it clones all the inherited struct Arcs from its parent, and all the reset struct Arcs from the initial computed values. We could reduce this refcount traffic by having ComputedValues hold just two Arcs, one to an intermediate struct to hold the inherited Arc pointers, and another to an intermediate struct to hold the reset Arc pointers.

This would save us a bunch of atomic refcounting at the expense of some additional allocations and pointer-chasing during layout. It would also let the rule tree hold onto the inherited values but not the reset values.

I don't really know the relative cost of atomic refcount operations in comparison to the other work we'd be doing here. We could certainly measure, though Patrick might have some intuition on whether this is likely to be a win or not. Patrick?

Comment 1

[Emilio Cobos Álvarez (:emilio)]

An atomic refcount bump is a relaxed increment, so I don't expect this to be a major performance improvement, over all taking into account the extra indirection we'd be adding.

Comment 2

[Patrick Walton (:pcwalton)]

Atomic operations tend to be cheap on x86 and slow on ARMv7 and below because the "dmb ish" instruction is expensive. I haven't tested ARMv8, which has better barrier instructions.

Comment 3

[Bobby Holley (:bholley)]

I've gained better intuition about this stuff since I filed the bug, and I'm pretty sure that the extra L2 cache misses during layout from the indirection would outweigh the reduction in atomic ops.