Bug 77158 – Speed up the prefix matching of cssPropertyName()

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Benjamin Poulain]

The function cssPropertyName() shows up in some benchmarks.

Fix the prefix matching before doing https://bugs.webkit.org/show_bug.cgi?id=74782

[Benjamin Poulain]

Created attachment 124207 [details]
Patch

[Geoffrey Garen]

Comment on attachment 124207 [details]
Patch

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

r=me

> Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp:72
> +static inline bool matchesCSSPropertyNamePrefixEnding(const StringImpl& propertyName, const char (&prefix)[prefixCStringLength])

The word "Ending" in this function name confused me. I think it would be better without it: "matchesCSSPropertyNamePrefix".

> Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp:101
> +static PropertyNamePrefix getCSSPropertyNamePrefix(const StringImpl& propertyName)

Your patch does one switch in getCSSPropertyNamePrefix(), then feeds the result into a second switch in cssPropertyName(). It would be more efficient to merge getCSSPropertyNamePrefix() into cssPropertyName(), so you only have to switch once.

[Benjamin Poulain]

> The word "Ending" in this function name confused me. I think it would be better without it: "matchesCSSPropertyNamePrefix".

Good point, that is confusing.

> Your patch does one switch in getCSSPropertyNamePrefix(), then feeds the result into a second switch in cssPropertyName(). It would be more efficient to merge getCSSPropertyNamePrefix() into cssPropertyName(), so you only have to switch once.

I did the two switch() for readability. I'll remove it.

Thanks for the review :)

[Benjamin Poulain]

Committed r106154: <http://trac.webkit.org/changeset/106154>

[Darin Adler]

Moving all JavaScriptGlue bugs to JavaScriptCore. The JavaScriptGlue framework itself is long gone. And most of the more recent bugs put in this component were put there by people who thought this was for some other aspect of “JavaScript glue” and have nothing to do with the actual original reason for the existence of this component, which was an OS-X-only framework named JavaScriptGlue.