[webkit-reviews] review denied: [Bug 80250] [JSC] Cache the CSSPropertyID in JSCSSStyleDeclaration for performance optimization : [Attachment 130071] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Kentaro Hara
<haraken at chromium.org>'s request for review:
Bug 80250: [JSC] Cache the CSSPropertyID in JSCSSStyleDeclaration for
performance optimization
https://bugs.webkit.org/show_bug.cgi?id=80250

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

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=130071&action=review


> Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp:181
> +    typedef HashMap<StringImpl*, CSSPropertyInfo*> CSSPropertyInfoMap;
> +    DEFINE_STATIC_LOCAL(CSSPropertyInfoMap, map, ());
> +    CSSPropertyInfo* propertyInfo = map.get(propertyNameString);

Oh, I see the difference with my test now, your key is the pointer to string,
not the string.

This does not seems like a good idea, nothing ensure you will keep this
StringImpl in memory.