Bug 84268

Summary:

CSSValuePool: Make numeric value caches fixed-size arrays.

Product:

WebKit

Reporter:

Andreas Kling <kling>

Component:

CSS

Assignee:

Andreas Kling <kling>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

macpherson, menard, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	eric: review-
Patch with less derp	koivisto: review-
Patch :'(	koivisto: review+

Andreas Kling

Reported 2012-04-18 12:57:33 PDT

This would be more space and time efficient than using hash maps.

Attachments
Patch (4.58 KB, patch) 2012-04-18 12:58 PDT, Andreas Kling	eric: review-	Details Formatted Diff Diff
Patch with less derp (4.58 KB, patch) 2012-04-18 13:05 PDT, Andreas Kling	koivisto: review-	Details Formatted Diff Diff
Patch :'( (4.59 KB, patch) 2012-04-18 13:44 PDT, Andreas Kling	koivisto: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Andreas Kling

Comment 1 2012-04-18 12:58:58 PDT

Created attachment 137745 [details] Patch

Eric Seidel (no email)

Comment 2 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?

Eric Seidel (no email)

Comment 3 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.

Andreas Kling

Comment 4 2012-04-18 13:05:35 PDT

Created attachment 137749 [details] Patch with less derp

Antti Koivisto

Comment 5 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.

Andreas Kling

Comment 6 2012-04-18 13:44:41 PDT

Created attachment 137758 [details] Patch :'(

Antti Koivisto

Comment 7 2012-04-18 13:45:49 PDT

Comment on attachment 137758 [details] Patch :'( r=me

Andreas Kling

Comment 8 2012-04-18 14:02:25 PDT

Committed r114558: <http://trac.webkit.org/changeset/114558>

Note You need to log in before you can comment on or make changes to this bug.