Bug 162228 – URLParser should parse serialized valid URLs faster than unknown input

• Component: New Bugs
• Status: RESOLVED FIXED

[Alex Christensen]

URLParser should parse serialized valid URLs faster than unknown input

[Alex Christensen]

Created attachment 289250 [details]
Patch

[Chris Dumez]

Comment on attachment 289250 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        No new tests.  No change in behavior.

nit: extra space :)

> Source/WebCore/platform/URLParser.cpp:855
> +template<typename CharacterType, bool serialized>

Why the extra template parameter here? It is not used.

> Source/WebCore/platform/URLParser.cpp:890
> +template<typename CharacterType, bool serialized>

If we reversed the parameters, we could let the compiler deduct the second one

> Source/WebCore/platform/URLParser.cpp:891
> +void incrementIterator(CodePointIterator<CharacterType>& iterator)

I would call this something like "skipWhileTabOrNewline" and move the first ++iterator out. This seems clearer and more reusable.
The call sites can then do:
++iterator;
if (!serialized)
    skipWhileTabOrNewline();

Alternatively, since this seems to be used a lot, you could simply rename this function to something clearer like: incrementIteratorSkippingTabAndNewLine.

> Source/WebCore/platform/URLParser.cpp:898
> +template<typename CharacterType, bool serialized>

Would look nicer at call sites to use an enum class instead of a boolean. e.g. enum class IsSerialized { No, Yes };

If we reversed the parameters, we could let the compiler deduct the second one

[Alex Christensen]

Created attachment 289268 [details]
Patch

[Alex Christensen]

Comment on attachment 289268 [details]
Patch

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

> Source/WebCore/platform/URLParser.cpp:439
> +template<bool serialized>
>  inline static void utf8PercentEncode(UChar32 codePoint, Vector<LChar>& destination, bool(*isInCodeSet)(UChar32))

I made these templates instead of another parameter to be more consistent with the other uses of serialized.  The generated code is equivalent once optimized.

> Source/WebCore/platform/URLParser.cpp:890
> +    const bool serialized = true;
> +    if (input.is8Bit())
> +        return parse<serialized>(input.characters8(), input.length(), { }, UTF8Encoding());

I named the bool instead of adding an enum.  This makes this call site easier to read, and it makes the use sites less cluttered.

> Source/WebCore/platform/URLParser.cpp:1111
> +                incrementIteratorSkippingTabAndNewLine<serialized>(c);

I chose the verbose name.

[Alex Christensen]

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