RESOLVED INVALID 61004
move fastCheckSelector call into SelectorChecker in preparation for querySelector using it 
https://bugs.webkit.org/show_bug.cgi?id=61004
Summary move fastCheckSelector call into SelectorChecker in preparation for querySele...
Ojan Vafai
Reported 2011-05-17 16:27:32 PDT
move fastCheckSelector call into SelectorChecker in preparation for querySelector using it
Attachments
Patch (27.02 KB, patch)
2011-05-17 16:38 PDT, Ojan Vafai 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Ojan Vafai
Comment 1 2011-05-17 16:38:56 PDT
Created attachment 93846 [details] Patch
Ojan Vafai
Comment 2 2011-05-17 16:47:37 PDT
To make querySelector go down this codepath, we'll use a dummy RuleData object. I've prototyped this locally, and, at least in some cases, it's much faster (e.g. 4x). The downside is that regular selector matching is possibly 0.5% slower. I ran the test case in the patch under sample and aggregated the matchRules(...) calls. I did 30 iterations with a clean build and 30 iterations with this patch. It's hard to be 100% sure given the large standard deviation. Including the data below: clean: Min. Median Mean Max. Stddev 278.0 305.0 307.4 361.0 19.1 Raw data: 338,315,284,361,303,291,293,287,303,294,310,325,278,327,318,311,335,291,331,295,286,295,307,292,318,304,323,306,289,313 modified: Min. Median Mean Max. Stddev 278.0 308.0 309.5 335.0 16.8 Raw data: 316,285,321,316,300,330,297,307,305,317,310,294,278,307,289,300,312,309,335,306,301,333,291,322,280,335,296,332,327,333 I was surprised to see this cause any performance difference, so I confirmed that the disassembly for matchRulesForList is in fact different between the two builds. I'm open to suggestions for how to do this differently that might not have any performance impact or how to get a testcase with less variability.
Ojan Vafai
Comment 3 2011-05-17 16:48:11 PDT
I wanted to get this patch in separately since this is the only performance sensitive part of the patch to make querySelector use fastCheckSelector.
Ojan Vafai
Comment 4 2011-05-18 12:12:17 PDT
Comment on attachment 93846 [details] Patch Hm. I'm not sure the patch this one is trying to enable makes sense actually. I had a bug in my performance measurement. I'll remove the r? while I figure it out.
Antti Koivisto
Comment 5 2011-05-19 06:40:08 PDT
All these checkSelector functions are kinda confusing. It would be nice not to add new ones.
Note You need to log in before you can comment on or make changes to this bug.