Bug 89898 – Fast path for simple transform parsing

• Component: CSS
• Status: RESOLVED FIXED

[Antti Koivisto]

When manipulating transforms using script, the transform value parsing can show up in profiles pretty heavily. We can optimize it easily by implementing a fast path that does not spin up the full CSS parser, like we already do for several other common value types.

[Antti Koivisto]

Created attachment 149337 [details]
patch

[Antti Koivisto]

<rdar://problem/11740369>

[Anders Carlsson]

Comment on attachment 149337 [details]
patch

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

> Source/WebCore/css/CSSParser.cpp:1009
> +static bool parseTransformArguments(PassRefPtr<WebKitCSSTransformValue> transformValue, CharType* characters, unsigned length, unsigned start, unsigned expectedCount)
> +{

I don't think this needs to take a PassRefPtr, just a raw pointer would be fine.

> Source/WebCore/css/CSSParser.cpp:1033
> +    // Very long strings probably have multiple components which we don't handle here.
> +    if (string.length() < 13 || string.length() > 32)

I'd be nice if you explained the less than check as well.

> Source/WebCore/css/CSSParser.cpp:1038
> +    UChar c9 = toASCIILowerUnchecked(string[9]);
> +    UChar c10 = toASCIILowerUnchecked(string[10]);

Is it OK to use unchecked here?

[Oliver Hunt]

Comment on attachment 149337 [details]
patch

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

>> Source/WebCore/css/CSSParser.cpp:1033
>> +    if (string.length() < 13 || string.length() > 32)
> 
> I'd be nice if you explained the less than check as well.

I don't like the magic numbers here.  This almost feels like something we want to auto generate.

[Antti Koivisto]

http://trac.webkit.org/changeset/121175, with fixes

[Antti Koivisto]

(In reply to comment #4)
> I don't like the magic numbers here.  This almost feels like something we want to auto generate.

I made them constants.