Bug 162241 – Reduce allocations in URLParser

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Reduce allocations in URLParser

[Alex Christensen]

Created attachment 289267 [details]
Patch

[Chris Dumez]

Comment on attachment 289267 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=289267&action=review

Sorry, I did not have time to do a proper review yet. Just a few comments.

> Source/WebCore/ChangeLog:9
> +        1. When URLParsing fails, use the original String for things like href.

Do we really care about optimizing the failure case?

> Source/WebCore/platform/URLParser.cpp:873
> +        return parse(input, input.characters8(), input.length(), base, encoding);

Passing input in addition to the characters is a bit odd. Since it seems to be used only to optimize the failure case, I am not sure it is worth it.

> Source/WebCore/platform/URLParser.cpp:1543
> +        for (size_t i = 0; i < m_asciiBuffer.size(); ++i)

There is an appendVector().

> Source/WebCore/platform/URLParser.cpp:1546
> +            buffer.uncheckedAppend(m_unicodeFragmentBuffer[i]);

There is an appendVector().

[Alex Christensen]

Created attachment 289280 [details]
Patch

[Chris Dumez]

Comment on attachment 289280 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=289280&action=review

r=me

> Source/WebCore/platform/URLParser.cpp:115
> +static void appendCodePoint(UChar32 codePoint, Vector<UChar>& destination)

I think it may look better if we reversed those parameters.

> Source/WebCore/platform/URLParser.cpp:1925
> +            for (size_t i = 0; i < domain.length(); i++)

++i

> Source/WebCore/platform/URLParser.cpp:1926
> +                ascii.uncheckedAppend(toASCIILower(domain.characters8()[i]));

Please cache characters8() before the loop

> Source/WebCore/platform/URLParser.cpp:1929
> +        Vector<LChar, defaultInlineBufferSize> ascii;

Can we refactor so that there is only 1 vector and 1 return statement?

> Source/WebCore/platform/URLParser.cpp:1933
> +            ascii.uncheckedAppend(toASCIILower(domain.characters16()[i]));

Please cache characters16() before the loop

[Alex Christensen]

Created attachment 289402 [details]
Patch

[Alex Christensen]

http://trac.webkit.org/changeset/206177

[Alex Christensen]

*** Bug 162045 has been marked as a duplicate of this bug. ***