[Webkit-unassigned] [Bug 86165] HTML parser should yield more to improve perceived page load time

Sat Jun 9 09:20:57 PDT 2012

https://bugs.webkit.org/show_bug.cgi?id=86165

Tony Gentilcore <tonyg at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #146375|review?                     |review-
               Flag|                            |

--- Comment #9 from Tony Gentilcore <tonyg at chromium.org>  2012-06-09 09:20:56 PST ---
(From update of attachment 146375)
View in context: https://bugs.webkit.org/attachment.cgi?id=146375&action=review

The approach looks great. Just a few nits to address and then this should be good to land.

> Source/WebCore/ChangeLog:11
> +        seconds once a script has been seen.

It would be nice to explain a bit more rationale in the ChangeLog since that's the first place people will look when understanding why this was done.

Something like:
"We want the parser to yield at least every 500ms to keep the page somewhat responsive and allow painting. Since it would be too expensive to check the time after each token, the previous heuristic was to check every 4,096 tokens. That works fine for most tokens, but a script may spend an arbitrary amount of time executing.

This patch fixes that by causing the parser to to check the elapsed time immediately after executing each script."

BTW, don't do anything in this patch, but have you considered whether 500ms is a reasonable constant?

> Source/WebCore/html/parser/HTMLParserScheduler.h:79
> +            if (session.processedTokens > m_parserChunkSize)

I'm not sure we need this. At the point of yielding, it should be fine to reset processedTokens, right?

> LayoutTests/fast/events/event-yield-timing.html:1
> +<html>

It doesn't seem right for a test which takes 2s to live in fast/, but I can't find a better place and I know we haven't been strict about what goes in fast/. In any case, putting this in fast/parser would be much more appropriate than fast/events.

> LayoutTests/fast/events/event-yield-timing.html:6
> +        window.layoutTestController.dumpAsText();

Recommend including ../js/resources/js-test-pre.js. It will call dumpAsText() for you and provide some helper methods like testPassed(), testFailed() and shouldBeGreaterThanOrEqual().

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.