Summary: | Use the Selector Code Generator for matching in SelectorQuery | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Benjamin Poulain <benjamin> | ||||||||||
Component: | New Bugs | Assignee: | Benjamin Poulain <benjamin> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | allan.jensen, bfulgham, commit-queue, dbates, eflews.bot, esprehn+autocc, galpeter, gtk-ews, gyuyoung.kim, gyuyoung.kim, kangil.han, koivisto, ltilve+ews, ossy, peavo, philn, pnormand, rniwa, xan.lopez | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Benjamin Poulain
2013-12-23 15:17:11 PST
Created attachment 219936 [details]
Patch
Comment on attachment 219936 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=219936&action=review > Source/WebCore/dom/SelectorQuery.cpp:329 > +#if ENABLE(CSS_SELECTOR_JIT) > + void* compiledSelectorChecker = selectorData.compiledSelectorCodeRef.code().executableAddress(); > + if (!compiledSelectorChecker && selectorData.compilationStatus == SelectorCompilationStatus::NotCompiled) { > + JSC::VM* vm = rootNode.document().scriptExecutionContext()->vm(); > + selectorData.compilationStatus = SelectorCompiler::compileSelector(selectorData.selector, vm, selectorData.compiledSelectorCodeRef); > + } > + > + if (compiledSelectorChecker) { > + if (selectorData.compilationStatus == SelectorCompilationStatus::SimpleSelectorChecker) { > + SelectorCompiler::SimpleSelectorChecker selectorChecker = SelectorCompiler::simpleSelectorCheckerFunction(compiledSelectorChecker, selectorData.compilationStatus); > + executeCompiledSimpleSelectorChecker<SelectorQueryTrait>(rootNode, selectorChecker, output); > + } else { > + ASSERT(selectorData.compilationStatus == SelectorCompilationStatus::SelectorCheckerWithCheckingContext); > + SelectorCompiler::SelectorCheckerWithCheckingContext selectorChecker = SelectorCompiler::selectorCheckerFunctionWithCheckingContext(compiledSelectorChecker, selectorData.compilationStatus); > + > + SelectorCompiler::CheckingContext context; > + context.elementStyle = nullptr; > + context.resolvingMode = SelectorChecker::QueryingRules; > + executeCompiledSelectorCheckerWithContext<SelectorQueryTrait>(rootNode, selectorChecker, context, output); > + } > + return; > + } > +#endif // ENABLE(CSS_SELECTOR_JIT) This function is getting really big now. Would it make sense to extract this as a separate inline function instead? > Source/WebCore/dom/SelectorQuery.h:63 > + mutable SelectorCompilationStatus compilationStatus; > + mutable JSC::MacroAssemblerCodeRef compiledSelectorCodeRef; It seems like we could encapsulate these two variables into one struct in SelectorCompiler.h > LayoutTests/fast/selectors/querySelector-long-adjacent-backtracking.html:4 > +<script src="../../resources/js-test-pre.js"></script> We can put this in the body. > LayoutTests/fast/selectors/querySelector-long-child-backtracking.html:4 > +<script src="../../resources/js-test-pre.js"></script> Ditto. > LayoutTests/fast/selectors/querySelector-mixed-child-adjacent-backtracking.html:4 > +<script src="../../resources/js-test-pre.js"></script> Ditto. > LayoutTests/fast/selectors/querySelector-multiple-simple-child-backtracking.html:4 > +<script src="../../resources/js-test-pre.js"></script> Ditto. > LayoutTests/fast/selectors/querySelector-simple-adjacent-backtracking.html:3 > +<head> Ditto. > LayoutTests/fast/selectors/querySelector-simple-child-backtracking.html:4 > +<script src="../../resources/js-test-pre.js"></script> Ditto. Comment on attachment 219936 [details] Patch Attachment 219936 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/49998311 Comment on attachment 219936 [details] Patch Attachment 219936 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/48148598 Comment on attachment 219936 [details] Patch Attachment 219936 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/51288044 Created attachment 220032 [details]
Patch
Comment on attachment 220032 [details] Patch Attachment 220032 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/6529312899465216 Comment on attachment 220032 [details] Patch Attachment 220032 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/4545682790678528 Comment on attachment 220032 [details] Patch Attachment 220032 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/5407948478087168 Comment on attachment 220032 [details]
Patch
Looks good!
Created attachment 220216 [details]
Patch
Let's see if GTK and EFL build now...
Comment on attachment 220216 [details] Patch Attachment 220216 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/6374351821602816 Comment on attachment 220216 [details] Patch Attachment 220216 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/4966976938049536 Comment on attachment 220216 [details] Patch Attachment 220216 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/6346239650037760 Comment on attachment 220216 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220216&action=review > Source/WebCore/dom/SelectorQuery.h:32 > +#include <JavaScriptCore/MacroAssemblerCodeRef.h> This kind of include is supported only by Mac, because forwarding headers aren't generated for WebCore. Comment on attachment 220216 [details]
Patch
It seems that Source/JavaScriptCore/assembler/MacroAssemblerCodeRef.h is not referenced in Source/JavaScriptCore/GNUmakefile.list.am. Can you please add it? Not sure it would fix the build though.
(In reply to comment #15) > (From update of attachment 220216 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=220216&action=review > > > Source/WebCore/dom/SelectorQuery.h:32 > > +#include <JavaScriptCore/MacroAssemblerCodeRef.h> > > This kind of include is supported only by Mac, because > forwarding headers aren't generated for WebCore. EFL port already includes "JavaScriptCore/assembler". So, if you modify to include the "MacroAssemblerCodeRef.h" as below, it works fine. #include "MacroAssemblerCodeRef.h" Created attachment 220301 [details]
Patch
Committed r161839: <http://trac.webkit.org/changeset/161839> (In reply to comment #19) > Committed r161839: <http://trac.webkit.org/changeset/161839> It broke the WinCairo build: 1>c:\projects\buildslave\win-cairo-release\build\webkitbuild\release_wincairo\include\private\javascriptcore\X86Assembler.h(2348): warning C4309: 'argument' : truncation of constant value (..\dom\DOMAllInOne.cpp) 1>c:\projects\buildslave\win-cairo-release\build\source\webcore\dom\SelectorQuery.h(31): fatal error C1083: Cannot open include file: 'SelectorCompiler.h': No such file or directory (..\dom\DOMAllInOne.cpp) 1>Done Building Project "C:\Projects\BuildSlave\win-cairo-release\build\Source\WebCore\WebCore.vcxproj\WebCore.vcxproj" (Build target(s)) -- FAILED. |