[Webkit-unassigned] [Bug 64262] Small speed up, which switches some virtual functions to inline ones.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=64262


Benjamin Poulain <benjamin at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #101148|review?                     |review-
               Flag|                            |




--- Comment #13 from Benjamin Poulain <benjamin at webkit.org>  2011-07-18 05:40:29 PST ---
(From update of attachment 101148)
View in context: https://bugs.webkit.org/attachment.cgi?id=101148&action=review

You are changing from virtual function to protected attribute for 3 features:
::isFixedSize()
::isQuirkValue()
::isPrimitiveValue()

In each case, you are increasing complexity of the code, and increasing the memory consumption per instance.

I would prefer to see 3 patches, one for each value. With numbers for performance and memory usage (ideally 3 bugs on bugzilla).

When changing something used all over the place like CSSValue, I think it is better to do small changes with a good rationale.

> Source/WebCore/css/CSSCanvasValue.h:52
> +        m_isFixedSize = true;

This should use the constructor initialiazation: m_isFixedSize(true);

> Source/WebCore/css/CSSPrimitiveValue.h:210
> +        m_isPrimitiveValue = true;

Initialization...

> Source/WebCore/css/CSSPrimitiveValue.h:220
> +        m_isPrimitiveValue = true;

Initialization...

> Source/WebCore/css/CSSPrimitiveValue.h:229
> +        m_isPrimitiveValue = true;

Initialization...

> Source/WebCore/css/CSSPrimitiveValue.h:243
> +        m_isPrimitiveValue = true;

Initialization...

> Source/WebCore/css/CSSPrimitiveValue.h:251
> +        m_isPrimitiveValue = true;

Initialization...

> Source/WebCore/css/CSSQuirkPrimitiveValue.h:43
> +        m_isQuirkValue = true;

Constructor initialization: m_isQuirkValue(true);

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.