1345025 - stylo: Use Servo for querySelector{,All}, along with stuff like Element::Matches, and Element::Closest

Status: RESOLVED FIXED

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

Description

[Bobby Holley (:bholley)]

I think this doesn't actually block shipping because we can probably continue to use the old style system (it doesn't require any rule processors or style sets). However, we'll need to do it eventually, and we'd certainly get a performance boost from doing those operations in parallel.

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

It seems some tests may depend on that these functions take the same code path as CSS selector matching, so those test may falsely pass because they go to the Gecko code path.

I saw at least one fullscreen test relies on Element.matches to check the availability of pseudo-classes, and fail to reveal bug 1374901. (I found that bug because there happens to be another test which relies on the real selector matching path. But that test isn't meant to check that at all.)

I suspect we should probably do this to avoid false passes like what I've found above.

Comment 3

[Boris Zbarsky [:bzbarsky]]

I suspect this is more work than it seems, in that doing it without a performance regression will be nontrivial.  Just something to watch out for.

Comment 4

[Bobby Holley (:bholley)]

(In reply to Boris Zbarsky [:bz] (if a patch has no decent message, automatic r-) from comment #3)
> I suspect this is more work than it seems, in that doing it without a
> performance regression will be nontrivial.  Just something to watch out for.

Yeah, IMO a potential increase in test coverage accuracy isn't enough to justify putting this in-scope for now. We can certainly re-evaluate in July/August if we're in good enough shape to have time to look at this.

Comment 5

[Bobby Holley (:bholley)]

Per discussion today, when we do this, we probably want to have a conditional bloom filter, which we maintain only if there's a parent/ancestor combinator in the selector. This would likely reduce reduce our 10x deficit with the competition in the (rather unrealistic) microbenchmark in [1].

[1] https://webkit.org/blog-files/css-jit-introduction/html5-single-page-microbenchmark.html

Comment 6

[Emilio Cobos Álvarez (:emilio)]

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #5)
> Per discussion today, when we do this, we probably want to have a
> conditional bloom filter, which we maintain only if there's a
> parent/ancestor combinator in the selector. This would likely reduce reduce
> our 10x deficit with the competition in the (rather unrealistic)
> microbenchmark in [1].
> 
> [1]
> https://webkit.org/blog-files/css-jit-introduction/html5-single-page-
> microbenchmark.html

Or use the left-to-right matching, I guess, which doesn't even need a bloom filter?

Comment 7

[Xidorn Quan [:xidorn] UTC+11]

It seems inDOMUtils::SelectorMatchesElement is already using Servo.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Bug 1410624 takes care of the last ones.

Comment 9

[Chris Peterson [:cpeterson]]

status-firefox57=wontfix unless someone thinks this bug should block 57

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Bug 1410624 landed.