1431421 - stylo: Special-case GeckoElement::has_class to be less branchy.

Status: RESOLVED FIXED

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

Description

[Emilio Cobos Álvarez (:emilio)]

See bug 1422522.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1431421: Make GeckoElement::has_class more specialized.

Review commit: https://reviewboard.mozilla.org/r/214022/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/214022/

Comment 2

[Emilio Cobos Álvarez (:emilio)]

This slightly improves the time, but it's still not great, seems like we really should get rid of the function call... I'll look into it.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Hackety hackety hack.

This is on top of the patch above. Still does _not_ make us faster than Gecko on the test-case, on my local build.

We could do this (with all the bindgen stuff to avoid hardcoding the symbol), though not ultra-enthusiastic about it.

Also, it'd be nice if you profiled it in your machine.

Times for gecko on my machine are:


    1444
    1405
    1305
    1279
    1275
    1258
    1265
    1293
    1349
    1274

For stylo, with these two patches:


    1696
    1557
    1532
    1474
    1494
    1514
    1530
    1554
    1510
    1495

Which is better than nightly, but still not faster.

Gecko profile:

  https://perfht.ml/2DLGeSf

Seems like most of the overhead is still in matches_complex_selector_internal... We may want to look into optimizations that the compiler doesn't end up doing or something.

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

How much faster does the first patch alone give us?

I'm not a fan of the second patch either, and it looks like inlining that doesn't make as much improvement as we wanted... Probably need some further investigation :/

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8943643 [details]
Bug 1431421: Make GeckoElement::has_class more specialized.

https://reviewboard.mozilla.org/r/214022/#review222242

Looks like a straightforward optimization we can take, although the function call overhead should still persist.

Comment 6

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8943716 [details] [diff] [review]
Hackety hackety hack.

As I mentioned before, this indeed isn't great, and given that it doesn't fix all the problem, we should probably continue investigating before having it in...

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2687ac50f577
Make GeckoElement::has_class more specialized. r=xidorn

Comment 8

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/2687ac50f577

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Sorry I forgot to reply to this on time.

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #4)
> How much faster does the first patch alone give us?

For reference, it gave us some. Enough to be noticeable. The second gave us much more of course, but it's gross.