Commit 2a21ec67 authored by jianli@chromium.org's avatar jianli@chromium.org

Revert of Revert of HTMLConstructionSite: avoid n^2 running time for large...

Revert of Revert of HTMLConstructionSite: avoid n^2 running time for large scripts. (patchset #1 id:1 of https://codereview.chromium.org/555223002/)

Reason for revert:
Not a culprit. Reverted to bring back the original patch.

Original issue's description:
> Revert of HTMLConstructionSite: avoid n^2 running time for large scripts. (patchset #5 id:80001 of https://codereview.chromium.org/494993002/)
> 
> Reason for revert:
> Speculative revert.
> 
> It may break the following blink sheriff bot:
> http://build.chromium.org/p/chromium.webkit/builders/Android%20Tests%20%28dbg%29/builds/21403
> 
> 
> 
> Original issue's description:
> > HTMLConstructionSite: avoid n^2 running time for large scripts.
> > 
> > Every time background parser sends chunk, tree is flushed.
> > 
> > If page contains very large script, then script node content is updated
> > many times. Every update is causes string concatenation.
> > 
> > Solution: do not flush pending text until it is mandatory.
> > 
> > Test: https://codereview.chromium.org/500363002
> > Test depends on: https://codereview.chromium.org/544453004/
> > 
> > BUG=410790
> > 
> > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181635
> 
> TBR=eseidel@chromium.org,kouhei@chromium.org,eustas@chromium.org
> NOTREECHECKS=true
> NOTRY=true
> BUG=410790
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181664

TBR=eseidel@chromium.org,kouhei@chromium.org,eustas@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=410790

Review URL: https://codereview.chromium.org/558743002

git-svn-id: svn://svn.chromium.org/blink/trunk@181668 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent b018267f
...@@ -231,11 +231,15 @@ static String atomizeIfAllWhitespace(const String& string, WhitespaceMode whites ...@@ -231,11 +231,15 @@ static String atomizeIfAllWhitespace(const String& string, WhitespaceMode whites
return string; return string;
} }
void HTMLConstructionSite::flushPendingText() void HTMLConstructionSite::flushPendingText(FlushMode mode)
{ {
if (m_pendingText.isEmpty()) if (m_pendingText.isEmpty())
return; return;
if (mode == FlushIfAtTextLimit
&& !shouldUseLengthLimit(*m_pendingText.parent))
return;
PendingText pendingText; PendingText pendingText;
// Hold onto the current pending text on the stack so that queueTask doesn't recurse infinitely. // Hold onto the current pending text on the stack so that queueTask doesn't recurse infinitely.
m_pendingText.swap(pendingText); m_pendingText.swap(pendingText);
...@@ -269,7 +273,7 @@ void HTMLConstructionSite::flushPendingText() ...@@ -269,7 +273,7 @@ void HTMLConstructionSite::flushPendingText()
void HTMLConstructionSite::queueTask(const HTMLConstructionSiteTask& task) void HTMLConstructionSite::queueTask(const HTMLConstructionSiteTask& task)
{ {
flushPendingText(); flushPendingText(FlushAlways);
ASSERT(m_pendingText.isEmpty()); ASSERT(m_pendingText.isEmpty());
m_taskQueue.append(task); m_taskQueue.append(task);
} }
...@@ -533,7 +537,7 @@ void HTMLConstructionSite::setCompatibilityModeFromDoctype(const String& name, c ...@@ -533,7 +537,7 @@ void HTMLConstructionSite::setCompatibilityModeFromDoctype(const String& name, c
void HTMLConstructionSite::processEndOfFile() void HTMLConstructionSite::processEndOfFile()
{ {
ASSERT(currentNode()); ASSERT(currentNode());
flush(); flush(FlushAlways);
openElements()->popAll(); openElements()->popAll();
} }
...@@ -541,7 +545,7 @@ void HTMLConstructionSite::finishedParsing() ...@@ -541,7 +545,7 @@ void HTMLConstructionSite::finishedParsing()
{ {
// We shouldn't have any queued tasks but we might have pending text which we need to promote to tasks and execute. // We shouldn't have any queued tasks but we might have pending text which we need to promote to tasks and execute.
ASSERT(m_taskQueue.isEmpty()); ASSERT(m_taskQueue.isEmpty());
flush(); flush(FlushAlways);
m_document->finishedParsing(); m_document->finishedParsing();
} }
...@@ -689,7 +693,7 @@ void HTMLConstructionSite::insertTextNode(const String& string, WhitespaceMode w ...@@ -689,7 +693,7 @@ void HTMLConstructionSite::insertTextNode(const String& string, WhitespaceMode w
// The nextChild != dummy.nextChild case occurs whenever foster parenting happened and we hit a new text node "<table>a</table>b" // The nextChild != dummy.nextChild case occurs whenever foster parenting happened and we hit a new text node "<table>a</table>b"
// In either case we have to flush the pending text into the task queue before making more. // In either case we have to flush the pending text into the task queue before making more.
if (!m_pendingText.isEmpty() && (m_pendingText.parent != dummyTask.parent || m_pendingText.nextChild != dummyTask.nextChild)) if (!m_pendingText.isEmpty() && (m_pendingText.parent != dummyTask.parent || m_pendingText.nextChild != dummyTask.nextChild))
flushPendingText(); flushPendingText(FlushAlways);
m_pendingText.append(dummyTask.parent, dummyTask.nextChild, string, whitespaceMode); m_pendingText.append(dummyTask.parent, dummyTask.nextChild, string, whitespaceMode);
} }
......
...@@ -92,6 +92,14 @@ enum WhitespaceMode { ...@@ -92,6 +92,14 @@ enum WhitespaceMode {
AllWhitespace, AllWhitespace,
}; };
enum FlushMode {
// Flush pending text. Flush queued tasks.
FlushAlways,
// Flush pending text if node has length limit. Flush queued tasks.
FlushIfAtTextLimit,
};
class AtomicHTMLToken; class AtomicHTMLToken;
class Document; class Document;
class Element; class Element;
...@@ -113,16 +121,16 @@ public: ...@@ -113,16 +121,16 @@ public:
void executeQueuedTasks(); void executeQueuedTasks();
// flushPendingText turns pending text into queued Text insertions, but does not execute them. // flushPendingText turns pending text into queued Text insertions, but does not execute them.
void flushPendingText(); void flushPendingText(FlushMode);
// Called before every token in HTMLTreeBuilder::processToken, thus inlined: // Called before every token in HTMLTreeBuilder::processToken, thus inlined:
void flush() void flush(FlushMode mode)
{ {
if (!hasPendingTasks()) if (!hasPendingTasks())
return; return;
flushPendingText(); flushPendingText(mode);
executeQueuedTasks(); // NOTE: Possible reentrancy via JavaScript execution. executeQueuedTasks(); // NOTE: Possible reentrancy via JavaScript execution.
ASSERT(!hasPendingTasks()); ASSERT(mode == FlushIfAtTextLimit || !hasPendingTasks());
} }
bool hasPendingTasks() bool hasPendingTasks()
......
...@@ -498,9 +498,10 @@ void HTMLDocumentParser::processParsedChunkFromBackgroundParser(PassOwnPtr<Parse ...@@ -498,9 +498,10 @@ void HTMLDocumentParser::processParsedChunkFromBackgroundParser(PassOwnPtr<Parse
ASSERT(!m_token); ASSERT(!m_token);
} }
// Make sure any pending text nodes are emitted before returning. // Make sure all required pending text nodes are emitted before returning.
// This leaves "script", "style" and "svg" nodes text nodes intact.
if (!isStopped()) if (!isStopped())
m_treeBuilder->flush(); m_treeBuilder->flush(FlushIfAtTextLimit);
} }
void HTMLDocumentParser::pumpPendingSpeculations() void HTMLDocumentParser::pumpPendingSpeculations()
...@@ -635,7 +636,7 @@ void HTMLDocumentParser::pumpTokenizer(SynchronousMode mode) ...@@ -635,7 +636,7 @@ void HTMLDocumentParser::pumpTokenizer(SynchronousMode mode)
// There should only be PendingText left since the tree-builder always flushes // There should only be PendingText left since the tree-builder always flushes
// the task queue before returning. In case that ever changes, crash. // the task queue before returning. In case that ever changes, crash.
if (mode == ForceSynchronous) if (mode == ForceSynchronous)
m_treeBuilder->flush(); m_treeBuilder->flush(FlushAlways);
RELEASE_ASSERT(!isStopped()); RELEASE_ASSERT(!isStopped());
if (session.needsYield) if (session.needsYield)
......
...@@ -402,7 +402,7 @@ void HTMLTreeBuilder::processToken(AtomicHTMLToken* token) ...@@ -402,7 +402,7 @@ void HTMLTreeBuilder::processToken(AtomicHTMLToken* token)
// Any non-character token needs to cause us to flush any pending text immediately. // Any non-character token needs to cause us to flush any pending text immediately.
// NOTE: flush() can cause any queued tasks to execute, possibly re-entering the parser. // NOTE: flush() can cause any queued tasks to execute, possibly re-entering the parser.
m_tree.flush(); m_tree.flush(FlushAlways);
m_shouldSkipLeadingNewline = false; m_shouldSkipLeadingNewline = false;
switch (token->type()) { switch (token->type()) {
...@@ -2689,7 +2689,7 @@ void HTMLTreeBuilder::processTokenInForeignContent(AtomicHTMLToken* token) ...@@ -2689,7 +2689,7 @@ void HTMLTreeBuilder::processTokenInForeignContent(AtomicHTMLToken* token)
return; return;
} }
m_tree.flush(); m_tree.flush(FlushAlways);
HTMLStackItem* adjustedCurrentNode = adjustedCurrentStackItem(); HTMLStackItem* adjustedCurrentNode = adjustedCurrentStackItem();
switch (token->type()) { switch (token->type()) {
......
...@@ -82,8 +82,9 @@ public: ...@@ -82,8 +82,9 @@ public:
// Done, close any open tags, etc. // Done, close any open tags, etc.
void finished(); void finished();
// Synchronously empty any queues, possibly creating more DOM nodes. // Synchronously flush pending text and queued tasks, possibly creating more DOM nodes.
void flush() { m_tree.flush(); } // Flushing pending text depends on |mode|.
void flush(FlushMode mode) { m_tree.flush(mode); }
void setShouldSkipLeadingNewline(bool shouldSkip) { m_shouldSkipLeadingNewline = shouldSkip; } void setShouldSkipLeadingNewline(bool shouldSkip) { m_shouldSkipLeadingNewline = shouldSkip; }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment