Bug 84268

Summary: CSSValuePool: Make numeric value caches fixed-size arrays.
Product: WebKit Reporter: Andreas Kling <kling>
Component: CSSAssignee: Andreas Kling <kling>
Status: RESOLVED FIXED    
Severity: Normal CC: macpherson, menard, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
eric: review-
Patch with less derp
koivisto: review-
Patch :'( koivisto: review+

Description Andreas Kling 2012-04-18 12:57:33 PDT 
This would be more space and time efficient than using hash maps.
Comment 1 Andreas Kling 2012-04-18 12:58:58 PDT 
Created attachment 137745 [details]
Patch
Comment 2 Eric Seidel (no email) 2012-04-18 13:01:47 PDT 
Comment on attachment 137745 [details]
Patch

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

> Source/WebCore/css/CSSValuePool.cpp:87
> +    if (value < 0 || value > maximumCacheableIntegerValue)

Shouldn't this be >= max?
Comment 3 Eric Seidel (no email) 2012-04-18 13:03:07 PDT 
Comment on attachment 137745 [details]
Patch

Farts.  It appears you're running off the end of your array by allowing 256.
Comment 4 Andreas Kling 2012-04-18 13:05:35 PDT 
Created attachment 137749 [details]
Patch with less derp
Comment 5 Antti Koivisto 2012-04-18 13:23:37 PDT 
Comment on attachment 137749 [details]
Patch with less derp

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

> Source/WebCore/css/CSSValuePool.h:73
> +    static const int maximumCacheableIntegerValue = 256;
> +
> +    RefPtr<CSSPrimitiveValue> m_pixelValueCache[maximumCacheableIntegerValue];
> +    RefPtr<CSSPrimitiveValue> m_percentValueCache[maximumCacheableIntegerValue];
> +    RefPtr<CSSPrimitiveValue> m_numberValueCache[maximumCacheableIntegerValue];

Naming is off. Your maximumCacheableIntegerValue is not actually cacheable.
Comment 6 Andreas Kling 2012-04-18 13:44:41 PDT 
Created attachment 137758 [details]
Patch :'(
Comment 7 Antti Koivisto 2012-04-18 13:45:49 PDT 
Comment on attachment 137758 [details]
Patch :'(

r=me
Comment 8 Andreas Kling 2012-04-18 14:02:25 PDT 
Committed r114558: <http://trac.webkit.org/changeset/114558>