Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(60)

Unified Diff: Source/core/html/parser/HTMLDocumentParser.cpp

Issue 258013009: Make the HTMLDocumentParser yield more aggressively Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: Created 6 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: Source/core/html/parser/HTMLDocumentParser.cpp
diff --git a/Source/core/html/parser/HTMLDocumentParser.cpp b/Source/core/html/parser/HTMLDocumentParser.cpp
index aa791dbb3611a8a93b61c4a6a18d0ff88facc558..76797259f2cdfbb8779186f2d021e270ef4f5c13 100644
--- a/Source/core/html/parser/HTMLDocumentParser.cpp
+++ b/Source/core/html/parser/HTMLDocumentParser.cpp
@@ -325,23 +325,11 @@ void HTMLDocumentParser::didReceiveParsedChunkFromBackgroundParser(PassOwnPtr<Pa
{
TRACE_EVENT0("webkit", "HTMLDocumentParser::didReceiveParsedChunkFromBackgroundParser");
- // alert(), runModalDialog, and the JavaScript Debugger all run nested event loops
- // which can cause this method to be re-entered. We detect re-entry using
- // hasActiveParser(), save the chunk as a speculation, and return.
- if (isWaitingForScripts() || !m_speculations.isEmpty() || document()->activeParserCount() > 0) {
- m_preloader->takeAndPreload(chunk->preloads);
- m_speculations.append(chunk);
- return;
- }
-
- // processParsedChunkFromBackgroundParser can cause this parser to be detached from the Document,
- // but we need to ensure it isn't deleted yet.
- RefPtr<HTMLDocumentParser> protect(this);
-
- ASSERT(m_speculations.isEmpty());
- chunk->preloads.clear(); // We don't need to preload because we're going to parse immediately.
+ m_preloader->takeAndPreload(chunk->preloads);
m_speculations.append(chunk);
abarth-chromium 2014/04/29 18:29:00 Should we rename m_speculations? I'm not sure tha
- pumpPendingSpeculations();
+ // If we're already paused waiting for scripts, no need to schedule a resume.
+ if (!isWaitingForScripts())
+ m_parserScheduler->scheduleForResume();
}
void HTMLDocumentParser::didReceiveEncodingDataFromBackgroundParser(const DocumentEncodingData& data)
@@ -436,6 +424,11 @@ void HTMLDocumentParser::processParsedChunkFromBackgroundParser(PassOwnPtr<Parse
break;
}
+ // FIXME: Currently this always consumes an entire chunk at once thus the time
+ // between deadline checks is controlled by BackgroundHTMLParser.cpp's pendingTokenLimit.
+ // We could make this code able to yield within a chunk, which would separate the
+ // question of how much the tokenizer should batch into a chunk from how long
+ // the main thread should spend treebuilding before yielding.
for (Vector<CompactHTMLToken>::const_iterator it = tokens->begin(); it != tokens->end(); ++it) {
ASSERT(!isWaitingForScripts());
@@ -483,7 +476,8 @@ void HTMLDocumentParser::processParsedChunkFromBackgroundParser(PassOwnPtr<Parse
void HTMLDocumentParser::pumpPendingSpeculations()
{
// FIXME: Share this constant with the parser scheduler.
- const double parserTimeLimit = 0.500;
+ // FIXME: This could be much larger during intial load where it may be OK to hog the main thread.
+ const double parserTimeLimit = 0.008;
abarth-chromium 2014/04/29 18:29:00 Again, this seems like a value the scheduler might
// ASSERT that this object is both attached to the Document and protected.
ASSERT(refCount() >= 2);

Powered by Google App Engine
This is Rietveld 408576698