Bug 77527 - <style scoped>: Implement scoped selector matching in the fast path
Summary: <style scoped>: Implement scoped selector matching in the fast path
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Roland Steiner
URL:
Keywords:
Depends on: 77525 77528
Blocks: 49142
  Show dependency treegraph
 
Reported: 2012-02-01 01:37 PST by Roland Steiner
Modified: 2014-02-06 10:25 PST (History)
7 users (show)

See Also:


Attachments
Patch (pending user-feedback) (11.80 KB, patch)
2012-02-16 23:49 PST, Roland Steiner
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roland Steiner 2012-02-01 01:37:00 PST
Allow scoped selector matching to use the fast selector checking path.
Comment 1 Roland Steiner 2012-02-16 23:49:27 PST
Created attachment 127536 [details]
Patch (pending user-feedback)
Comment 2 Roland Steiner 2012-02-16 23:55:31 PST
Dromaeo CSS test results from Tools/Scripts/run-perf-tests, using WK Release at r107911:

baseline 'master' results:

Running Dromaeo/cssquery-dojo.html (1 of 25)
RESULT Dromaeo: cssquery-dojo= 169415.365092 ms
median= 0.0 ms, stdev= 370.108854584 ms, min= 167729.0 ms, max= 170693.0 ms

Running Dromaeo/cssquery-jquery.html (2 of 25)
RESULT Dromaeo: cssquery-jquery= 1245755.45774 ms
median= 0.0 ms, stdev= 83992.7599566 ms, min= 1133214.0 ms, max= 1363938.0 ms

Running Dromaeo/cssquery-prototype.html (3 of 25)
RESULT Dromaeo: cssquery-prototype= 197169.653808 ms
median= 0.0 ms, stdev= 335.688608645 ms, min= 195401.0 ms, max= 198543.0 ms


Results with my patch:

Running Dromaeo/cssquery-dojo.html (1 of 3)
RESULT Dromaeo: cssquery-dojo= 169620.786028 ms
median= 0.0 ms, stdev= 398.474399033 ms, min= 168027.93014 ms, max= 171185.0 ms

Running Dromaeo/cssquery-jquery.html (2 of 3)
RESULT Dromaeo: cssquery-jquery= 1158673.44356 ms
median= 0.0 ms, stdev= 79879.470916 ms, min= 1060246.37163 ms, max= 1280063.79321 ms

Running Dromaeo/cssquery-prototype.html (3 of 3)
RESULT Dromaeo: cssquery-prototype= 197052.6 ms
median= 0.0 ms, stdev= 365.454511533 ms, min= 195161.0 ms, max= 198560.0 ms


The 2nd test (Dromaeo/cssquery-jquery) does not seem very reliable, as it has quite a bit of variance and the results look "too good to be true", but there seems to be almost no change in the other 2 tests. If you want me to run additional tests, please let me know!
Comment 3 Antti Koivisto 2012-02-17 00:27:22 PST
We might not want to do this right now. Fast path is meant to optimize the most common cases and there is no <style scoped> content out yet. It will be hard to prove that the extra branchiness won't regress our general performance (note that effect may be different on ARM).

Benchmarks using querySelectorAll don't generally do nearly enough selector matching for differences to show up. Web page style resolve is where differences really matters, PerformanceTests/Parser/html5-full-render.html may be interesting. Comparing sampler traces gives the most accurate picture.
Comment 4 Roland Steiner 2012-02-17 02:01:22 PST
(In reply to comment #3)
> PerformanceTests/Parser/html5-full-render.html may be interesting. 

Ask and ye shall receive! ;) 

baseline 'master' result:

Running Parser/html5-full-render.html (1 of 1)
RESULT Parser: html5-full-render= 23679.5 ms
median= 23679.5 ms, stdev= 11.5 ms, min= 23668.0 ms, max= 23691.0 ms

with patch:

Running Parser/html5-full-render.html (1 of 1)
RESULT Parser: html5-full-render= 23820.5 ms
median= 23820.5 ms, stdev= 59.5 ms, min= 23761.0 ms, max= 23880.0 ms

a performance hit of 0.6%.

I understand your point regarding postponing this, but just to bring it up: This is just a straightforward implementation - apart from trying more "creative" implementations, another simple possibility would be to completely branch for 'scoped' matching in the fast path - that is essentially duplicating the code, one branch for non-scoped matching, one for scoped matching. This would just entail a single additional 'if' at the start of matching. Would that be a realistic approach?
Comment 5 Eric Seidel (no email) 2012-05-21 14:47:12 PDT
I don't know how to review this, or if it's still wanted.  It's a rather old patch.
Comment 6 Roland Steiner 2012-05-21 16:51:43 PDT
(In reply to comment #5)
> I don't know how to review this, or if it's still wanted.  It's a rather old patch.

We agreed to freeze implementation of this pending more user-feedback on the whole <style scoped> feature. Removing review request for now.
Comment 7 Andreas Kling 2014-02-06 10:25:12 PST
Scoped stylesheets were taken out.