On Fri, Nov 11, 2016 at 03:22:16AM -0800, Manish Goregaokar wrote: > A possible solution is to have a opaquish DOM backpointer to > corresponding rule objects in the style tree and have it call some > update function to update the DOM when mutating. This isn't a great > solution IMO.
Note that this is already what is done for a Node's computed style (and
the rest of the style and layout data) in Servo, and it's already
causing some problems, like unnecessary extra reflows and overhead to
check a node's computed value for a given property using
GetComputedStyle.
As a side note, given we're thinking in how to structure these crates,
it would be nice to find a solution for this too, since the extra
reflows aren't cheap, and a layout thread query in general seems a lot
of overhead for something that should be really cheap.
That's probably a slightly harder problem though, because it involves
knowing layout data structures (as in: Servo's layout crate). I've
thought about enforcing the first member of the heap-allocated node data
to be a ComputedStyle, but I sincerely would prefer to avoid as many
transmutes as possible, one of them is too many...
> The one that we seem to prefer is maintaining the style tree and the
> CSSOM tree in parallel, with references from the CSSOM tree to the
> style tree, but not vice versa. Only the DOM mutates the style tree.
This seems like the most reasonable approach to me too. Merging style
and DOM will not only be probably a blocker for Stylo, but also make the
style system much more unsafe (having lots more of JS reflectors in
random threads).
> This could possible be enforced by wrapping all interior mutability in
> the style tree with a type that requires a zero-sized token to be
> passed in to the borrow_mut/write method; a token which can't be
> created normally but can be obtained via a method on dom::Reflectable
> (within arms reach for most DOM code). Stylo code can FFI call into
> functions using a similar mechanism.
Or probably asserting that we're not in a Servo Layout thread when we
`borrow_mut`? Not perfect, but...
Also, note that the layout thread right now is the one that constructs
and manages the Stylist, and that stylesheet web fonts are processed
right now async with script (see the AddStylesheet layout message), and
it reads the font face rules, so potentially you want a stronger
synchronization there.
> Now, we could just put the Vec in an Arc. I'm not sure if we want to
> with the extra indirection, but it's a simple solution. An alternative
> is to recognize that the `Vec<CSSRule>` is always going to be stored
> within larger Arc'd objects, and just use those, by storing an
>
> enum RuleListOwner {
> Sheet(Arc<Stylesheet>),
> Media(Arc<MediaRule>),
> // ..
> }
>
> This is great for the Rust code -- easy to abstract over and no extra perf
> cost.
Can you move the same CSSRuleList to a different owner? If you can, this
wouldn't work, right?
> Personally I think just sticking the vec into an Arc is fine and won't
> impact us much. But I don't know.
Agreed, the extra Arc, over all given where it's used doesn't seem such
a show-stopper to me. We have Arc<RwLocks<>> all over the place, and I
don't think this two places are hotter than the ones on top of, let's
say, property declaration blocks.
I assume this will also need it's own RwLock in order to mutate the
list, right? If that's the case, probably something like
Arc<RwLock<CSSRuleList>>, where CSSRuleList(Vec<CSSRule>) is probably
nicer.
-- Emilio
signature.asc
Description: PGP signature
_______________________________________________ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
