Changeset 143845 in webkit

Timestamp:

Feb 23, 2013 10:37:50 AM (11 years ago)

Author:

abarth@webkit.org

Message:

Threaded HTML parser should pass fast/parser/parser-yield-timing.html
​https://bugs.webkit.org/show_bug.cgi?id=110647

Reviewed by Eric Seidel.

Previously, the threaded HTML parser would run for an arbitrary amount
of time without yielding after speculation succeeded. This might be the
cause of the good DOMContentLoaded numbers.

Note: This patch also demonstrates that the ParseHTML_max numbers
aren't correct currently because they're measuring the interior of this
loop instead of all the time spent in the loop. We should move the
instrumentation in a followup patch.

• html/parser/HTMLDocumentParser.cpp:

(WebCore::HTMLDocumentParser::resumeParsingAfterYield):
(WebCore::HTMLDocumentParser::pumpPendingSpeculations):
(WebCore):
(WebCore::HTMLDocumentParser::resumeParsingAfterScriptExecution):

• html/parser/HTMLDocumentParser.h:

(HTMLDocumentParser):

• html/parser/HTMLParserScheduler.cpp:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/parser/HTMLDocumentParser.cpp (modified) (4 diffs)
• html/parser/HTMLDocumentParser.h (modified) (1 diff)
• html/parser/HTMLParserScheduler.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-02-23  Adam Barth  <abarth@webkit.org>
+
+        Threaded HTML parser should pass fast/parser/parser-yield-timing.html
+        https://bugs.webkit.org/show_bug.cgi?id=110647
+
+        Reviewed by Eric Seidel.
+
+        Previously, the threaded HTML parser would run for an arbitrary amount
+        of time without yielding after speculation succeeded. This might be the
+        cause of the good DOMContentLoaded numbers.
+
+        Note: This patch also demonstrates that the ParseHTML_max numbers
+        aren't correct currently because they're measuring the interior of this
+        loop instead of all the time spent in the loop. We should move the
+        instrumentation in a followup patch.
+
+         * html/parser/HTMLDocumentParser.cpp:
+        (WebCore::HTMLDocumentParser::resumeParsingAfterYield):
+        (WebCore::HTMLDocumentParser::pumpPendingSpeculations):
+        (WebCore):
+        (WebCore::HTMLDocumentParser::resumeParsingAfterScriptExecution):
+        * html/parser/HTMLDocumentParser.h:
+        (HTMLDocumentParser):
+        * html/parser/HTMLParserScheduler.cpp:
+
 2013-02-23  Martin Robinson  <mrobinson@igalia.com>
 

trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp

@@ -241 +241 @@
 void HTMLDocumentParser::resumeParsingAfterYield()
 {
-    ASSERT(!m_haveBackgroundParser);
-
     // pumpTokenizer can cause this parser to be detached from the Document,
     // but we need to ensure it isn't deleted yet.
     RefPtr<HTMLDocumentParser> protect(this);
+
+#if ENABLE(THREADED_HTML_PARSER)
+    if (m_haveBackgroundParser) {
+        pumpPendingSpeculations();
+        return;
+    }
+#endif
 
     // We should never be here unless we can pump immediately.  Call pumpTokenizer()
@@ -356 +361 @@
 void HTMLDocumentParser::processParsedChunkFromBackgroundParser(PassOwnPtr<ParsedChunk> chunk)
 {
+    // ASSERT that this object is both attached to the Document and protected.
+    ASSERT(refCount() >= 2);
     ASSERT(shouldUseThreading());
 
@@ -393 +400 @@
 
     checkForSpeculationFailure();
+}
+
+void HTMLDocumentParser::pumpPendingSpeculations()
+{
+    // FIXME: Share this constant with the parser scheduler.
+    const double parserTimeLimit = 0.500;
+
+    // ASSERT that this object is both attached to the Document and protected.
+    ASSERT(refCount() >= 2);
+
+    // FIXME: Pass in current input length.
+    InspectorInstrumentationCookie cookie = InspectorInstrumentation::willWriteHTML(document(), 0, lineNumber().zeroBasedInt());
+
+    double startTime = currentTime();
+
+    while (!m_speculations.isEmpty()) {
+        processParsedChunkFromBackgroundParser(m_speculations.takeFirst());
+
+        if (isWaitingForScripts() || isStopped())
+            break;
+
+        if (currentTime() - startTime > parserTimeLimit && !m_speculations.isEmpty()) {
+            m_parserScheduler->scheduleForResume();
+            break;
+        }
+    }
+
+    InspectorInstrumentation::didWriteHTML(cookie, lineNumber().zeroBasedInt());
 }
 
@@ -796 +831 @@
         // but we need to ensure it isn't deleted yet.
         RefPtr<HTMLDocumentParser> protect(this);
-
-        // FIXME: Pass in current input length.
-        InspectorInstrumentationCookie cookie = InspectorInstrumentation::willWriteHTML(document(), 0, lineNumber().zeroBasedInt());
-
-        while (!m_speculations.isEmpty()) {
-            processParsedChunkFromBackgroundParser(m_speculations.takeFirst());
-            if (isWaitingForScripts() || isStopped())
-                break;
-        }
-
-        InspectorInstrumentation::didWriteHTML(cookie, lineNumber().zeroBasedInt());
-
+        pumpPendingSpeculations();
         return;
     }

trunk/Source/WebCore/html/parser/HTMLDocumentParser.h

@@ -143 +143 @@
     void didFailSpeculation(PassOwnPtr<HTMLToken>, PassOwnPtr<HTMLTokenizer>);
     void processParsedChunkFromBackgroundParser(PassOwnPtr<ParsedChunk>);
+    void pumpPendingSpeculations();
 #endif
 

trunk/Source/WebCore/html/parser/HTMLParserScheduler.cpp

@@ -38 +38 @@
 
 // defaultParserTimeLimit is the seconds the parser will run in one write() call
-// before yielding.  Inline <script> execution can cause it to excede the limit.
+// before yielding. Inline <script> execution can cause it to exceed the limit.
 // FIXME: We would like this value to be 0.2.
 static const double defaultParserTimeLimit = 0.500;