Use the Selector Code Generator for resolving style
Created attachment 219952 [details] Patch
Comment on attachment 219952 [details] Patch Attachment 219952 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/50458029
Comment on attachment 219952 [details] Patch Attachment 219952 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/45698122
Comment on attachment 219952 [details] Patch Attachment 219952 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/51868073
Comment on attachment 219952 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=219952&action=review > Source/WebCore/css/ElementRuleCollector.cpp:301 > inline bool ElementRuleCollector::ruleMatches(const RuleData& ruleData, PseudoId& dynamicPseudo) > { > const StyleResolver::State& state = m_state; > +#if ENABLE(CSS_SELECTOR_JIT) > + void* compiledSelectorChecker = ruleData.compiledSelectorCodeRef.code().executableAddress(); > + if (!compiledSelectorChecker && ruleData.compilationStatus == SelectorCompilationStatus::NotCompiled) { > + JSC::VM* vm = document().scriptExecutionContext()->vm(); > + ruleData.compilationStatus = SelectorCompiler::compileSelector(ruleData.selector(), vm, ruleData.compiledSelectorCodeRef); > + compiledSelectorChecker = ruleData.compiledSelectorCodeRef.code().executableAddress(); > + } This compiles too eagerly. If you look below in ruleData.hasFastCheckableSelector() branch you see there are number of cases we resolve without calling into SelectorCheckerFastPath at all. Those cases will use more memory and may get slower by activating the JIT. We should probably not compile selectors that are in this category. Does the infrastructure support compiling off the main thread? I think optimally we would kick off the compiling during RuleData construction and use the compiled versions when ready. > Source/WebCore/css/ElementRuleCollector.cpp:312 > + if (compiledSelectorChecker) { > + if (ruleData.compilationStatus == SelectorCompilationStatus::SimpleSelectorChecker) { > + if (m_pseudoStyleRequest.pseudoId != NOPSEUDO) > + return false; > + SelectorCompiler::SimpleSelectorChecker selectorChecker = SelectorCompiler::simpleSelectorCheckerFunction(compiledSelectorChecker, ruleData.compilationStatus); > + return selectorChecker(state.element()); > + } > + ASSERT(ruleData.compilationStatus == SelectorCompilationStatus::SelectorCheckerWithCheckingContext); > + > + if (m_pseudoStyleRequest.pseudoId != NOPSEUDO) > + return false; NOPSEUDO tests can move before the compilationStatus check and combined. > Source/WebCore/css/RuleSet.h:84 > +#if ENABLE(CSS_SELECTOR_JIT) > + mutable SelectorCompilationStatus compilationStatus; > + mutable JSC::MacroAssemblerCodeRef compiledSelectorCodeRef; > +#endif // ENABLE(CSS_SELECTOR_JIT) These fields should be optimized for memory at some point (I think we can manage with 64bits or less for all this). We can have large numbers of selectors and so RuleDatas. We also shouldn't generate code for simplest selectors that are resolved by hash look-ups and don't need overhead of even having these fields.
Created attachment 220991 [details] Patch
Comment on attachment 220991 [details] Patch Attachment 220991 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/6029259655086080
Comment on attachment 220991 [details] Patch Attachment 220991 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/6717223087374336
Comment on attachment 220991 [details] Patch Attachment 220991 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/6353650481889280
Created attachment 221004 [details] Patch
Comment on attachment 221004 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221004&action=review > Source/WebCore/css/ElementRuleCollector.cpp:312 > + JSC::VM* vm = document().scriptExecutionContext()->vm(); We should be using a reference instead in all these places (probably should be done in a separate patch though). > Source/WebCore/css/ElementRuleCollector.cpp:321 > + SelectorCompiler::SimpleSelectorChecker selectorChecker = SelectorCompiler::simpleSelectorCheckerFunction(compiledSelectorChecker, ruleData.compilationStatus); Do we really need this temporary variable? Also, isn't this the case where auto will be useful? Since this is a function pointer into a JIT'ed code, type doesn't give us any extra information anyway. > Source/WebCore/css/RuleSet.h:84 > +#if ENABLE(CSS_SELECTOR_JIT) > + mutable SelectorCompilationStatus compilationStatus; > + mutable JSC::MacroAssemblerCodeRef compiledSelectorCodeRef; > +#endif // ENABLE(CSS_SELECTOR_JIT) > + These should really be private and have accessors instead.
Committed r161917: <http://trac.webkit.org/changeset/161917>