[webkit-reviews] review denied: [Bug 108027] Enable the preload scanner on the background parser thread : [Attachment 184903] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Jan 27 17:13:30 PST 2013


Adam Barth <abarth at webkit.org> has denied Eric Seidel <eric at webkit.org>'s
request for review:
Bug 108027: Enable the preload scanner on the background parser thread
https://bugs.webkit.org/show_bug.cgi?id=108027

Attachment 184903: Patch
https://bugs.webkit.org/attachment.cgi?id=184903&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=184903&action=review


IMHO, we should use CompactHTMLTokens rather than HTMLTokens.  This patch means
we're creating 2x the number of strings on the background thread (because both
the preload scanner and the compact HTML token make strings for every tag and
attribute).  Our profiles tell us that we're spending a significant amount of
time creating strings, which means we don't want to double that work.

> Source/WebCore/html/parser/HTMLDocumentParser.cpp:476
> +    KURL documentURL = document()->url().copy();

Is this a thread-safe copy?

> Source/WebCore/html/parser/HTMLPreloadScanner.cpp:95
> +	       String attributeName(iter->m_name.data(), iter->m_name.size());

Here we're making another copy of every attribute name and value.  If we used
CompactHTMLToken, then we could use the String we've already created.


More information about the webkit-reviews mailing list