Changeset 145277 in webkit

Timestamp:

Mar 8, 2013 3:39:25 PM (11 years ago)

Author:

eric@webkit.org

Message:

Free up background parser's checkpoints when speculation succeeds
​https://bugs.webkit.org/show_bug.cgi?id=110547

Reviewed by Adam Barth.

This should be a memory (and possible perf) win while parsing
pages, as we will no longer hold multiple copies of every
source byte during the whole parse.

Many LayoutTests exercise this code path, and I've manually (debugger and printf)
that we're hitting this code, but we'll have to wait for the memory/perf bots
to tell us if this shows up as a win.

We only bother to message the parser at the end of a speculation chain, so as not
to send too many messages to the background parser.

• html/parser/BackgroundHTMLInputStream.cpp:

(WebCore::BackgroundHTMLInputStream::BackgroundHTMLInputStream):
(WebCore::BackgroundHTMLInputStream::invalidateCheckpointsUpThrough):
(WebCore):
(WebCore::BackgroundHTMLInputStream::rewindTo):

• html/parser/BackgroundHTMLInputStream.h:

(BackgroundHTMLInputStream):
(Checkpoint):
(WebCore::BackgroundHTMLInputStream::Checkpoint::isNull):
(WebCore::BackgroundHTMLInputStream::Checkpoint::clear):

• html/parser/BackgroundHTMLParser.cpp:

(WebCore::BackgroundHTMLParser::passedCheckpoint):
(WebCore):

• html/parser/BackgroundHTMLParser.h:

(BackgroundHTMLParser):

• html/parser/HTMLDocumentParser.cpp:

(WebCore::HTMLDocumentParser::pumpPendingSpeculations):

Location:

trunk/Source/WebCore

Files:

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

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-03-08  Eric Seidel  <eric@webkit.org>
+
+        Free up background parser's checkpoints when speculation succeeds
+        https://bugs.webkit.org/show_bug.cgi?id=110547
+
+        Reviewed by Adam Barth.
+
+        This should be a memory (and possible perf) win while parsing
+        pages, as we will no longer hold multiple copies of every
+        source byte during the whole parse.
+
+        Many LayoutTests exercise this code path, and I've manually (debugger and printf)
+        that we're hitting this code, but we'll have to wait for the memory/perf bots
+        to tell us if this shows up as a win.
+
+        We only bother to message the parser at the end of a speculation chain, so as not
+        to send too many messages to the background parser.
+
+        * html/parser/BackgroundHTMLInputStream.cpp:
+        (WebCore::BackgroundHTMLInputStream::BackgroundHTMLInputStream):
+        (WebCore::BackgroundHTMLInputStream::invalidateCheckpointsUpThrough):
+        (WebCore):
+        (WebCore::BackgroundHTMLInputStream::rewindTo):
+        * html/parser/BackgroundHTMLInputStream.h:
+        (BackgroundHTMLInputStream):
+        (Checkpoint):
+        (WebCore::BackgroundHTMLInputStream::Checkpoint::isNull):
+        (WebCore::BackgroundHTMLInputStream::Checkpoint::clear):
+        * html/parser/BackgroundHTMLParser.cpp:
+        (WebCore::BackgroundHTMLParser::passedCheckpoint):
+        (WebCore):
+        * html/parser/BackgroundHTMLParser.h:
+        (BackgroundHTMLParser):
+        * html/parser/HTMLDocumentParser.cpp:
+        (WebCore::HTMLDocumentParser::pumpPendingSpeculations):
+
 2013-03-08  Chandra Shekar Vallala  <brk376@motorola.com>
 

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

@@ -33 +33 @@
 
 BackgroundHTMLInputStream::BackgroundHTMLInputStream()
+    : m_firstValidCheckpointIndex(0)
+    , m_firstValidSegmentIndex(0)
 {
 }
@@ -54 +56 @@
 }
 
+void BackgroundHTMLInputStream::invalidateCheckpointsUpThrough(HTMLInputCheckpoint checkpointIndex)
+{
+    ASSERT(checkpointIndex < m_checkpoints.size());
+    ASSERT(checkpointIndex >= m_firstValidCheckpointIndex);
+    const Checkpoint& checkpoint = m_checkpoints[checkpointIndex];
+    ASSERT(m_firstValidSegmentIndex <= checkpoint.numberOfSegmentsAlreadyAppended);
+    for (size_t i = m_firstValidSegmentIndex; i < checkpoint.numberOfSegmentsAlreadyAppended; ++i)
+        m_segments[i] = String();
+    m_firstValidSegmentIndex = checkpoint.numberOfSegmentsAlreadyAppended;
+
+    for (size_t i = m_firstValidCheckpointIndex; i <= checkpointIndex; ++i)
+        m_checkpoints[i].clear();
+    m_firstValidCheckpointIndex = checkpointIndex + 1;
+}
+
 void BackgroundHTMLInputStream::rewindTo(HTMLInputCheckpoint checkpointIndex, const String& unparsedInput)
 {
     ASSERT(checkpointIndex < m_checkpoints.size()); // If this ASSERT fires, checkpointIndex is invalid.
     const Checkpoint& checkpoint = m_checkpoints[checkpointIndex];
+    ASSERT(!checkpoint.isNull());
 
     bool isClosed = m_current.isClosed();
@@ -63 +81 @@
     m_current = checkpoint.input;
 
-    for (size_t i = checkpoint.numberOfSegmentsAlreadyAppended; i < m_segments.size(); ++i)
+    for (size_t i = checkpoint.numberOfSegmentsAlreadyAppended; i < m_segments.size(); ++i) {
+        ASSERT(!m_segments[i].isNull());
         m_current.append(SegmentedString(m_segments[i]));
+    }
 
     if (!unparsedInput.isEmpty())
@@ -76 +96 @@
     m_segments.clear();
     m_checkpoints.clear();
+    m_firstValidCheckpointIndex = 0;
+    m_firstValidSegmentIndex = 0;
 }
 

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

@@ -51 +51 @@
     HTMLInputCheckpoint createCheckpoint();
     void rewindTo(HTMLInputCheckpoint, const String& unparsedInput);
+    void invalidateCheckpointsUpThrough(HTMLInputCheckpoint);
 
 private:
@@ -58 +59 @@
         SegmentedString input;
         size_t numberOfSegmentsAlreadyAppended;
+
+#ifndef NDEBUG
+        bool isNull() const { return input.isEmpty() && !numberOfSegmentsAlreadyAppended; }
+#endif
+        void clear() { input.clear(); numberOfSegmentsAlreadyAppended = 0; }
     };
 
@@ -63 +69 @@
     Vector<String> m_segments;
     Vector<Checkpoint> m_checkpoints;
+
+    // Note: These indicies may === vector.size(), in which case there are no valid checkpoints/segments at this time.
+    size_t m_firstValidCheckpointIndex;
+    size_t m_firstValidSegmentIndex;
 };
 

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

@@ -168 +168 @@
 }
 
+void BackgroundHTMLParser::passedCheckpoint(HTMLInputCheckpoint inputCheckpoint)
+{
+    // Note, we should not have to worry about the index being invalid
+    // as messages from the main thread will be processed in FIFO order.
+    m_input.invalidateCheckpointsUpThrough(inputCheckpoint);
+}
+
 void BackgroundHTMLParser::finish()
 {

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

@@ -75 +75 @@
     void append(const String&);
     void resumeFrom(PassOwnPtr<Checkpoint>);
+    void passedCheckpoint(HTMLInputCheckpoint);
     void finish();
     void stop();

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

@@ -458 +458 @@
 
     double startTime = currentTime();
+    HTMLInputCheckpoint lastCheckpointPassed = 0;
 
     while (!m_speculations.isEmpty()) {
-        processParsedChunkFromBackgroundParser(m_speculations.takeFirst());
-
-        if (isWaitingForScripts() || isStopped())
+        OwnPtr<ParsedChunk> chunk = m_speculations.takeFirst();
+        lastCheckpointPassed = chunk->inputCheckpoint;
+        processParsedChunkFromBackgroundParser(chunk.release());
+
+        // Processing a chunk can call document.write, causing us to invalidate any remaining speculations.
+        if (m_speculations.isEmpty() || isStopped()) {
+            // We're aborting these speculations, so don't tell the parser we've passed a checkpoint (its already cleared its checkpoints).
+            lastCheckpointPassed = 0;
             break;
-
-        if (currentTime() - startTime > parserTimeLimit && !m_speculations.isEmpty()) {
+        }
+
+        if (isWaitingForScripts())
+            break;
+
+        if (currentTime() - startTime > parserTimeLimit) {
             m_parserScheduler->scheduleForResume();
             break;
         }
     }
+
+    if (lastCheckpointPassed)
+        HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::passedCheckpoint, m_backgroundParser, lastCheckpointPassed));
 
     InspectorInstrumentation::didWriteHTML(cookie, lineNumber().zeroBasedInt());