Bug 126185

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:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	eflews.bot: commit-queue-
Patch	none
Patch	none

Benjamin Poulain

Reported 2013-12-23 15:17:11 PST

Use the Selector Code Generator for matching in SelectorQuery

Attachments
Patch (25.57 KB, patch) 2013-12-23 15:21 PST, Benjamin Poulain	no flags	Details Formatted Diff Diff
Patch (25.48 KB, patch) 2013-12-26 06:43 PST, Benjamin Poulain	eflews.bot: commit-queue-	Details Formatted Diff Diff
Patch (25.48 KB, patch) 2014-01-02 03:42 PST, Benjamin Poulain	no flags	Details Formatted Diff Diff
Patch (25.87 KB, patch) 2014-01-03 06:59 PST, Benjamin Poulain	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Benjamin Poulain

Comment 1 2013-12-23 15:21:02 PST

Created attachment 219936 [details] Patch

Ryosuke Niwa

Comment 2 2013-12-23 16:05:29 PST

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.

EFL EWS Bot

Comment 3 2013-12-23 16:27:59 PST

Comment on attachment 219936 [details] Patch Attachment 219936 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/49998311

kov's GTK+ EWS bot

Comment 4 2013-12-23 18:16:17 PST

Comment on attachment 219936 [details] Patch Attachment 219936 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/48148598

EFL EWS Bot

Comment 5 2013-12-23 19:35:49 PST

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

Benjamin Poulain

Comment 6 2013-12-26 06:43:52 PST

Created attachment 220032 [details] Patch

EFL EWS Bot

Comment 7 2013-12-26 06:50:03 PST

Comment on attachment 220032 [details] Patch Attachment 220032 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/6529312899465216

EFL EWS Bot

Comment 8 2013-12-26 06:51:28 PST

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

kov's GTK+ EWS bot

Comment 9 2013-12-26 06:52:29 PST

Comment on attachment 220032 [details] Patch Attachment 220032 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/5407948478087168

Antti Koivisto

Comment 10 2013-12-26 12:19:51 PST

Comment on attachment 220032 [details] Patch Looks good!

Benjamin Poulain

Comment 11 2014-01-02 03:42:59 PST

Created attachment 220216 [details] Patch Let's see if GTK and EFL build now...

EFL EWS Bot

Comment 12 2014-01-02 04:02:26 PST

Comment on attachment 220216 [details] Patch Attachment 220216 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/6374351821602816

kov's GTK+ EWS bot

Comment 13 2014-01-02 04:04:27 PST

Comment on attachment 220216 [details] Patch Attachment 220216 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/4966976938049536

EFL EWS Bot

Comment 14 2014-01-02 04:05:55 PST

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

Csaba Osztrogonác

Comment 15 2014-01-02 08:48:26 PST

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.

Philippe Normand

Comment 16 2014-01-02 09:18:12 PST

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.

Gyuyoung Kim

Comment 17 2014-01-02 17:09:13 PST

(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"

Benjamin Poulain

Comment 18 2014-01-03 06:59:27 PST

Created attachment 220301 [details] Patch

Benjamin Poulain

Comment 19 2014-01-12 15:01:30 PST

Committed r161839: <http://trac.webkit.org/changeset/161839>

Csaba Osztrogonác

Comment 20 2014-01-13 03:23:05 PST

(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.

Note You need to log in before you can comment on or make changes to this bug.