|Summary:||<style scoped>: Implement scoped selector matching in the fast path|
|Product:||WebKit||Reporter:||Roland Steiner <rolandsteiner>|
|Component:||CSS||Assignee:||Roland Steiner <rolandsteiner>|
|Severity:||Normal||CC:||dglazkov, eric, kling, koivisto, macpherson, menard, syoichi|
|Version:||528+ (Nightly build)|
|Bug Depends on:||77525, 77528|
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.