Title: [164202] trunk/Source/WebCore
Revision
164202
Author
[email protected]
Date
2014-02-16 19:14:48 -0800 (Sun, 16 Feb 2014)

Log Message

Split compilation state between querySelector and CSS matching
https://bugs.webkit.org/show_bug.cgi?id=128869

Reviewed by Antti Koivisto.

Cleanup after recent changes:
-SelectorCompiler now has a SelectorContext defining if the code is compiled for QuerySelector
 or for ElementRuleCollector.
-Generalize m_selectorCannotMatchAnything by making it part of the FunctionType. FunctionType now
 fully represent the type of code generation and we don't rely implicitly on m_selectorFragments being
 empty.

* css/ElementRuleCollector.cpp:
(WebCore::ElementRuleCollector::ruleMatches):

* cssjit/SelectorCompiler.cpp:
(WebCore::SelectorCompiler::compileSelector):
(WebCore::SelectorCompiler::SelectorCodeGenerator::SelectorCodeGenerator):
There is no tree marking for QuerySelector, so we can generate a simple selector
for the sibling selectors.

(WebCore::SelectorCompiler::SelectorCodeGenerator::compile):
The code is split to make it simpler. The classic code generation has been moved
to generateSelectorChecker().
The decision on what to generate is reduced to a simple switch-case.

(WebCore::SelectorCompiler::SelectorCodeGenerator::generateSelectorChecker):
The code that was previously in compile().

(WebCore::SelectorCompiler::SelectorCodeGenerator::markParentElementIfResolvingStyle):
We should not generate tree marking on querySelector traversal. Since the constructor
now generate a SimpleChecker, it would also be incorrect to attempt to access the checkingContext
on the stack.
We can just skip the marking entierly.

* cssjit/SelectorCompiler.h:
* dom/SelectorQuery.cpp:
(WebCore::SelectorDataList::execute):
With the changes of SelectorContext, we can no longer generate a complex checker for querySelector.
This code may come back in the future but at the moment it is useless.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (164201 => 164202)


--- trunk/Source/WebCore/ChangeLog	2014-02-17 03:11:41 UTC (rev 164201)
+++ trunk/Source/WebCore/ChangeLog	2014-02-17 03:14:48 UTC (rev 164202)
@@ -1,3 +1,46 @@
+2014-02-16  Benjamin Poulain  <[email protected]>
+
+        Split compilation state between querySelector and CSS matching
+        https://bugs.webkit.org/show_bug.cgi?id=128869
+
+        Reviewed by Antti Koivisto.
+
+        Cleanup after recent changes:
+        -SelectorCompiler now has a SelectorContext defining if the code is compiled for QuerySelector
+         or for ElementRuleCollector.
+        -Generalize m_selectorCannotMatchAnything by making it part of the FunctionType. FunctionType now
+         fully represent the type of code generation and we don't rely implicitly on m_selectorFragments being
+         empty.
+
+        * css/ElementRuleCollector.cpp:
+        (WebCore::ElementRuleCollector::ruleMatches):
+
+        * cssjit/SelectorCompiler.cpp:
+        (WebCore::SelectorCompiler::compileSelector):
+        (WebCore::SelectorCompiler::SelectorCodeGenerator::SelectorCodeGenerator):
+        There is no tree marking for QuerySelector, so we can generate a simple selector
+        for the sibling selectors.
+
+        (WebCore::SelectorCompiler::SelectorCodeGenerator::compile):
+        The code is split to make it simpler. The classic code generation has been moved
+        to generateSelectorChecker().
+        The decision on what to generate is reduced to a simple switch-case.
+
+        (WebCore::SelectorCompiler::SelectorCodeGenerator::generateSelectorChecker):
+        The code that was previously in compile().
+
+        (WebCore::SelectorCompiler::SelectorCodeGenerator::markParentElementIfResolvingStyle):
+        We should not generate tree marking on querySelector traversal. Since the constructor
+        now generate a SimpleChecker, it would also be incorrect to attempt to access the checkingContext
+        on the stack.
+        We can just skip the marking entierly.
+
+        * cssjit/SelectorCompiler.h:
+        * dom/SelectorQuery.cpp:
+        (WebCore::SelectorDataList::execute):
+        With the changes of SelectorContext, we can no longer generate a complex checker for querySelector.
+        This code may come back in the future but at the moment it is useless.
+
 2014-02-16  Benjamin Poulain  <[email protected]>
 
         The FTP view is squished to the left

Modified: trunk/Source/WebCore/css/ElementRuleCollector.cpp (164201 => 164202)


--- trunk/Source/WebCore/css/ElementRuleCollector.cpp	2014-02-17 03:11:41 UTC (rev 164201)
+++ trunk/Source/WebCore/css/ElementRuleCollector.cpp	2014-02-17 03:14:48 UTC (rev 164202)
@@ -295,7 +295,7 @@
         JSC::VM* vm = m_element.document().scriptExecutionContext()->vm();
         SelectorCompilationStatus compilationStatus;
         JSC::MacroAssemblerCodeRef compiledSelectorCodeRef;
-        compilationStatus = SelectorCompiler::compileSelector(ruleData.selector(), vm, compiledSelectorCodeRef);
+        compilationStatus = SelectorCompiler::compileSelector(ruleData.selector(), vm, SelectorCompiler::SelectorContext::RuleCollector, compiledSelectorCodeRef);
 
         ruleData.setCompiledSelector(compilationStatus, compiledSelectorCodeRef);
         compiledSelectorChecker = ruleData.compiledSelectorCodeRef().code().executableAddress();

Modified: trunk/Source/WebCore/cssjit/SelectorCompiler.cpp (164201 => 164202)


--- trunk/Source/WebCore/cssjit/SelectorCompiler.cpp	2014-02-17 03:11:41 UTC (rev 164201)
+++ trunk/Source/WebCore/cssjit/SelectorCompiler.cpp	2014-02-17 03:14:48 UTC (rev 164202)
@@ -86,6 +86,7 @@
 enum class FunctionType {
     SimpleSelectorChecker,
     SelectorCheckerWithCheckingContext,
+    CannotMatchAnything,
     CannotCompile
 };
 
@@ -118,7 +119,7 @@
 
 class SelectorCodeGenerator {
 public:
-    SelectorCodeGenerator(const CSSSelector*);
+    SelectorCodeGenerator(const CSSSelector*, SelectorContext);
     SelectorCompilationStatus compile(JSC::VM*, JSC::MacroAssemblerCodeRef&);
 
 private:
@@ -129,6 +130,7 @@
 #endif
 
     void computeBacktrackingInformation();
+    void generateSelectorChecker();
 
     // Element relations tree walker.
     void generateWalkToParentElement(Assembler::JumpList& failureCases, Assembler::RegisterID targetRegister);
@@ -163,9 +165,9 @@
     StackAllocator m_stackAllocator;
     Vector<std::pair<Assembler::Call, JSC::FunctionPtr>> m_functionCalls;
 
+    SelectorContext m_selectorContext;
     FunctionType m_functionType;
     SelectorFragmentList m_selectorFragments;
-    bool m_selectorCannotMatchAnything;
 
     StackAllocator::StackReference m_checkingContextStackReference;
 
@@ -183,11 +185,11 @@
 #endif
 };
 
-SelectorCompilationStatus compileSelector(const CSSSelector* lastSelector, JSC::VM* vm, JSC::MacroAssemblerCodeRef& codeRef)
+SelectorCompilationStatus compileSelector(const CSSSelector* lastSelector, JSC::VM* vm, SelectorContext selectorContext, JSC::MacroAssemblerCodeRef& codeRef)
 {
     if (!vm->canUseJIT())
         return SelectorCompilationStatus::CannotCompile;
-    SelectorCodeGenerator codeGenerator(lastSelector);
+    SelectorCodeGenerator codeGenerator(lastSelector, selectorContext);
     return codeGenerator.compile(vm, codeRef);
 }
 
@@ -287,10 +289,10 @@
     return FunctionType::CannotCompile;
 }
 
-inline SelectorCodeGenerator::SelectorCodeGenerator(const CSSSelector* rootSelector)
+inline SelectorCodeGenerator::SelectorCodeGenerator(const CSSSelector* rootSelector, SelectorContext selectorContext)
     : m_stackAllocator(m_assembler)
+    , m_selectorContext(selectorContext)
     , m_functionType(FunctionType::SimpleSelectorChecker)
-    , m_selectorCannotMatchAnything(false)
 #if CSS_SELECTOR_JIT_DEBUGGING
     , m_originalSelector(rootSelector)
 #endif
@@ -310,8 +312,10 @@
         case CSSSelector::Id: {
             const AtomicString& id = selector->value();
             if (fragment.id) {
-                if (id != *fragment.id)
-                    goto InconsistentSelector;
+                if (id != *fragment.id) {
+                    m_functionType = FunctionType::CannotMatchAnything;
+                    return;
+                }
             } else
                 fragment.id = &(selector->value());
             break;
@@ -321,8 +325,8 @@
             break;
         case CSSSelector::PseudoClass:
             m_functionType = mostRestrictiveFunctionType(m_functionType, addPseudoType(selector->pseudoType(), fragment));
-            if (m_functionType == FunctionType::CannotCompile)
-                goto CannotHandleSelector;
+            if (m_functionType == FunctionType::CannotCompile || m_functionType == FunctionType::CannotMatchAnything)
+                return;
             break;
         case CSSSelector::Set:
             fragment.attributes.append(selector);
@@ -346,8 +350,12 @@
         if (relation == CSSSelector::ShadowDescendant && !selector->isLastInTagHistory())
             goto CannotHandleSelector;
 
-        if (relation == CSSSelector::DirectAdjacent || relation == CSSSelector::IndirectAdjacent)
-            m_functionType = std::max(m_functionType, FunctionType::SelectorCheckerWithCheckingContext);
+        if (relation == CSSSelector::DirectAdjacent || relation == CSSSelector::IndirectAdjacent) {
+            FunctionType relationFunctionType = FunctionType::SelectorCheckerWithCheckingContext;
+            if (m_selectorContext == SelectorContext::QuerySelector)
+                relationFunctionType = FunctionType::SimpleSelectorChecker;
+            m_functionType = std::max(m_functionType, relationFunctionType);
+        }
 
         fragment.relationToLeftFragment = fragmentRelationForSelectorRelation(relation);
         fragment.relationToRightFragment = relationToPreviousFragment;
@@ -360,11 +368,8 @@
     computeBacktrackingInformation();
 
     return;
-InconsistentSelector:
-    m_functionType = FunctionType::SimpleSelectorChecker;
-    m_selectorCannotMatchAnything = true;
 CannotHandleSelector:
-    m_selectorFragments.clear();
+    m_functionType = FunctionType::CannotCompile;
 }
 
 static inline unsigned minimumRegisterRequirements(const SelectorFragmentList& selectorFragments)
@@ -401,102 +406,17 @@
 
 inline SelectorCompilationStatus SelectorCodeGenerator::compile(JSC::VM* vm, JSC::MacroAssemblerCodeRef& codeRef)
 {
-    if (m_selectorFragments.isEmpty() && !m_selectorCannotMatchAnything)
-        return SelectorCompilationStatus::CannotCompile;
-
-    bool reservedCalleeSavedRegisters = false;
-    unsigned availableRegisterCount = m_registerAllocator.availableRegisterCount();
-    unsigned minimumRegisterCountForAttributes = minimumRegisterRequirements(m_selectorFragments);
-    if (availableRegisterCount < minimumRegisterCountForAttributes) {
-        reservedCalleeSavedRegisters = true;
-        m_registerAllocator.reserveCalleeSavedRegisters(m_stackAllocator, minimumRegisterCountForAttributes - availableRegisterCount);
-    }
-
-    m_registerAllocator.allocateRegister(elementAddressRegister);
-
-    if (m_functionType == FunctionType::SelectorCheckerWithCheckingContext)
-        m_checkingContextStackReference = m_stackAllocator.push(checkingContextRegister);
-
-    Assembler::JumpList failureCases;
-
-    for (unsigned i = 0; i < m_selectorFragments.size(); ++i) {
-        const SelectorFragment& fragment = m_selectorFragments[i];
-        switch (fragment.relationToRightFragment) {
-        case FragmentRelation::Rightmost:
-            generateElementMatching(failureCases, fragment);
-            break;
-        case FragmentRelation::Descendant:
-            generateAncestorTreeWalker(failureCases, fragment);
-            break;
-        case FragmentRelation::Child:
-            generateParentElementTreeWalker(failureCases, fragment);
-            break;
-        case FragmentRelation::DirectAdjacent:
-            generateDirectAdjacentTreeWalker(failureCases, fragment);
-            break;
-        case FragmentRelation::IndirectAdjacent:
-            generateIndirectAdjacentTreeWalker(failureCases, fragment);
-            break;
-        }
-        generateBacktrackingTailsIfNeeded(fragment);
-    }
-
-    m_registerAllocator.deallocateRegister(elementAddressRegister);
-
-    if (m_functionType == FunctionType::SimpleSelectorChecker && m_selectorCannotMatchAnything) {
+    switch (m_functionType) {
+    case FunctionType::SimpleSelectorChecker:
+    case FunctionType::SelectorCheckerWithCheckingContext:
+        generateSelectorChecker();
+        break;
+    case FunctionType::CannotMatchAnything:
         m_assembler.move(Assembler::TrustedImm32(0), returnRegister);
         m_assembler.ret();
-    } else if (m_functionType == FunctionType::SimpleSelectorChecker) {
-        // Success.
-        m_assembler.move(Assembler::TrustedImm32(1), returnRegister);
-        if (!reservedCalleeSavedRegisters)
-            m_assembler.ret();
-
-        // Failure.
-        if (!failureCases.empty()) {
-            Assembler::Jump skipFailureCase;
-            if (reservedCalleeSavedRegisters)
-                skipFailureCase = m_assembler.jump();
-
-            failureCases.link(&m_assembler);
-            m_assembler.move(Assembler::TrustedImm32(0), returnRegister);
-
-            if (!reservedCalleeSavedRegisters)
-                m_assembler.ret();
-            else
-                skipFailureCase.link(&m_assembler);
-        }
-        if (reservedCalleeSavedRegisters) {
-            m_registerAllocator.restoreCalleeSavedRegisters(m_stackAllocator);
-            m_assembler.ret();
-        }
-    } else {
-        ASSERT(m_functionType == FunctionType::SelectorCheckerWithCheckingContext);
-        ASSERT(!m_selectorCannotMatchAnything);
-
-        // Success.
-        m_assembler.move(Assembler::TrustedImm32(1), returnRegister);
-
-        StackAllocator successStack = m_stackAllocator;
-        StackAllocator failureStack = m_stackAllocator;
-
-        LocalRegister checkingContextRegister(m_registerAllocator);
-        successStack.pop(m_checkingContextStackReference, checkingContextRegister);
-
-        // Failure.
-        if (!failureCases.empty()) {
-            Assembler::Jump skipFailureCase = m_assembler.jump();
-
-            failureCases.link(&m_assembler);
-            failureStack.discard();
-            m_assembler.move(Assembler::TrustedImm32(0), returnRegister);
-
-            skipFailureCase.link(&m_assembler);
-        }
-
-        m_stackAllocator.merge(std::move(successStack), std::move(failureStack));
-        m_registerAllocator.restoreCalleeSavedRegisters(m_stackAllocator);
-        m_assembler.ret();
+        break;
+    case FunctionType::CannotCompile:
+        return SelectorCompilationStatus::CannotCompile;
     }
 
     JSC::LinkBuffer linkBuffer(*vm, &m_assembler, CSS_CODE_ID);
@@ -509,7 +429,7 @@
     codeRef = FINALIZE_CODE(linkBuffer, ("CSS Selector JIT"));
 #endif
 
-    if (m_functionType == FunctionType::SimpleSelectorChecker)
+    if (m_functionType == FunctionType::SimpleSelectorChecker || m_functionType == FunctionType::CannotMatchAnything)
         return SelectorCompilationStatus::SimpleSelectorChecker;
     return SelectorCompilationStatus::SelectorCheckerWithCheckingContext;
 }
@@ -662,6 +582,100 @@
     }
 }
 
+void SelectorCodeGenerator::generateSelectorChecker()
+{
+    bool reservedCalleeSavedRegisters = false;
+    unsigned availableRegisterCount = m_registerAllocator.availableRegisterCount();
+    unsigned minimumRegisterCountForAttributes = minimumRegisterRequirements(m_selectorFragments);
+    if (availableRegisterCount < minimumRegisterCountForAttributes) {
+        reservedCalleeSavedRegisters = true;
+        m_registerAllocator.reserveCalleeSavedRegisters(m_stackAllocator, minimumRegisterCountForAttributes - availableRegisterCount);
+    }
+
+    m_registerAllocator.allocateRegister(elementAddressRegister);
+
+    if (m_functionType == FunctionType::SelectorCheckerWithCheckingContext)
+        m_checkingContextStackReference = m_stackAllocator.push(checkingContextRegister);
+
+    Assembler::JumpList failureCases;
+
+    for (unsigned i = 0; i < m_selectorFragments.size(); ++i) {
+        const SelectorFragment& fragment = m_selectorFragments[i];
+        switch (fragment.relationToRightFragment) {
+        case FragmentRelation::Rightmost:
+            generateElementMatching(failureCases, fragment);
+            break;
+        case FragmentRelation::Descendant:
+            generateAncestorTreeWalker(failureCases, fragment);
+            break;
+        case FragmentRelation::Child:
+            generateParentElementTreeWalker(failureCases, fragment);
+            break;
+        case FragmentRelation::DirectAdjacent:
+            generateDirectAdjacentTreeWalker(failureCases, fragment);
+            break;
+        case FragmentRelation::IndirectAdjacent:
+            generateIndirectAdjacentTreeWalker(failureCases, fragment);
+            break;
+        }
+        generateBacktrackingTailsIfNeeded(fragment);
+    }
+
+    m_registerAllocator.deallocateRegister(elementAddressRegister);
+
+    if (m_functionType == FunctionType::SimpleSelectorChecker) {
+        // Success.
+        m_assembler.move(Assembler::TrustedImm32(1), returnRegister);
+        if (!reservedCalleeSavedRegisters)
+            m_assembler.ret();
+
+        // Failure.
+        if (!failureCases.empty()) {
+            Assembler::Jump skipFailureCase;
+            if (reservedCalleeSavedRegisters)
+                skipFailureCase = m_assembler.jump();
+
+            failureCases.link(&m_assembler);
+            m_assembler.move(Assembler::TrustedImm32(0), returnRegister);
+
+            if (!reservedCalleeSavedRegisters)
+                m_assembler.ret();
+            else
+                skipFailureCase.link(&m_assembler);
+        }
+        if (reservedCalleeSavedRegisters) {
+            m_registerAllocator.restoreCalleeSavedRegisters(m_stackAllocator);
+            m_assembler.ret();
+        }
+    } else {
+        ASSERT(m_functionType == FunctionType::SelectorCheckerWithCheckingContext);
+
+        // Success.
+        m_assembler.move(Assembler::TrustedImm32(1), returnRegister);
+
+        StackAllocator successStack = m_stackAllocator;
+        StackAllocator failureStack = m_stackAllocator;
+
+        LocalRegister checkingContextRegister(m_registerAllocator);
+        successStack.pop(m_checkingContextStackReference, checkingContextRegister);
+
+        // Failure.
+        if (!failureCases.empty()) {
+            Assembler::Jump skipFailureCase = m_assembler.jump();
+
+            failureCases.link(&m_assembler);
+            failureStack.discard();
+            m_assembler.move(Assembler::TrustedImm32(0), returnRegister);
+
+            skipFailureCase.link(&m_assembler);
+        }
+
+        m_stackAllocator.merge(std::move(successStack), std::move(failureStack));
+        m_registerAllocator.restoreCalleeSavedRegisters(m_stackAllocator);
+        m_assembler.ret();
+    }
+}
+
 static inline Assembler::Jump testIsElementFlagOnNode(Assembler::ResultCondition condition, Assembler& assembler, Assembler::RegisterID nodeAddress)
 {
     return assembler.branchTest32(condition, Assembler::Address(nodeAddress, Node::nodeFlagsMemoryOffset()), Assembler::TrustedImm32(Node::flagIsElement()));
@@ -778,6 +792,9 @@
 
 void SelectorCodeGenerator::markParentElementIfResolvingStyle(JSC::FunctionPtr markingFunction)
 {
+    if (m_selectorContext == SelectorContext::QuerySelector)
+        return;
+
     //     if (checkingContext.resolvingMode == ResolvingStyle) {
     //         Element* parent = element->parentNode();
     //         markingFunction(parent);

Modified: trunk/Source/WebCore/cssjit/SelectorCompiler.h (164201 => 164202)


--- trunk/Source/WebCore/cssjit/SelectorCompiler.h	2014-02-17 03:11:41 UTC (rev 164201)
+++ trunk/Source/WebCore/cssjit/SelectorCompiler.h	2014-02-17 03:14:48 UTC (rev 164202)
@@ -72,9 +72,17 @@
     RenderStyle* elementStyle;
 };
 
+enum class SelectorContext {
+    // Rule Collector needs a resolvingMode and can modify the tree as it matches.
+    RuleCollector,
+
+    // Query Selector does not modify the tree and never match :visited.
+    QuerySelector
+};
+
 typedef unsigned (*SimpleSelectorChecker)(Element*);
 typedef unsigned (*SelectorCheckerWithCheckingContext)(Element*, const CheckingContext*);
-SelectorCompilationStatus compileSelector(const CSSSelector*, JSC::VM*, JSC::MacroAssemblerCodeRef& outputCodeRef);
+SelectorCompilationStatus compileSelector(const CSSSelector*, JSC::VM*, SelectorContext, JSC::MacroAssemblerCodeRef& outputCodeRef);
 
 inline SimpleSelectorChecker simpleSelectorCheckerFunction(void* executableAddress, SelectorCompilationStatus compilationStatus)
 {

Modified: trunk/Source/WebCore/dom/SelectorQuery.cpp (164201 => 164202)


--- trunk/Source/WebCore/dom/SelectorQuery.cpp	2014-02-17 03:11:41 UTC (rev 164201)
+++ trunk/Source/WebCore/dom/SelectorQuery.cpp	2014-02-17 03:14:48 UTC (rev 164202)
@@ -308,23 +308,14 @@
             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);
+                selectorData.compilationStatus = SelectorCompiler::compileSelector(selectorData.selector, vm, SelectorCompiler::SelectorContext::QuerySelector, selectorData.compiledSelectorCodeRef);
+                RELEASE_ASSERT(selectorData.compilationStatus != SelectorCompilationStatus::SelectorCheckerWithCheckingContext);
                 compiledSelectorChecker = selectorData.compiledSelectorCodeRef.code().executableAddress();
             }
 
             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);
-                }
+                SelectorCompiler::SimpleSelectorChecker selectorChecker = SelectorCompiler::simpleSelectorCheckerFunction(compiledSelectorChecker, selectorData.compilationStatus);
+                executeCompiledSimpleSelectorChecker<SelectorQueryTrait>(rootNode, selectorChecker, output);
                 return;
             }
 #endif // ENABLE(CSS_SELECTOR_JIT)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to