Bug 64862 - Switch isFixedSize() virtual function to inline one.
: Switch isFixedSize() virtual function to inline one.
Status: RESOLVED DUPLICATE of bug 71824
: WebKit
CSS
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
: 64262
  Show dependency treegraph
 
Reported: 2011-07-20 04:32 PST by
Modified: 2012-01-08 15:28 PST (History)


Attachments
Switch isFixedSize virtual function to inline one. (3.50 KB, patch)
2011-07-20 04:38 PST, Tamas Czene
eric: review-
Review Patch | Details | Formatted Diff | Diff


Note

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


Description From 2011-07-20 04:32:40 PST
Removing the virtual functions - removing the calls - makes measurable performance progression on CSS.

We measured the following performance progressions on some very popular websites, for example:
msn.com: 33,6%
yandex.ru: 3%
------- Comment #1 From 2011-07-20 04:38:03 PST -------
Created an attachment (id=101448) [details]
Switch isFixedSize virtual function to inline one.

The number of variables created during the test.
m_isFixedSize 725
The max number of variables in the memory during the test.
m_isFixedsize 57
------- Comment #2 From 2011-07-20 13:25:31 PST -------
See my comments in https://bugs.webkit.org/show_bug.cgi?id=64262.
------- Comment #3 From 2011-09-12 17:01:08 PST -------
(From update of attachment 101448 [details])
Please respond to Dave Hyatt.  Again, memory hit.
------- Comment #4 From 2011-10-11 17:09:37 PST -------
I believe there are some wasted bits that could be put to good use for a perf enhancement without increasing the memory overhead. Please give a chance to upload an RFC patch and some repeatable numbers.
------- Comment #5 From 2011-10-19 14:55:38 PST -------
View in context: https://bugs.webkit.org/attachment.cgi?id=101448&action=review

Does this change result in a measurable performance progression? 
After cleaning up, try PerformanceTests/Parser/html5-full-render.html
with 20 runs on tip of tree (ToT) with and without the patch.

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

I believe the preference is for a protected setter.

> Source/WebCore/css/CSSImageGeneratorValue.h:31
> +#include <wtf/AlwaysInline.h>

As mentioned on the related bugs, no need for AlwaysInline because the accessors are trivial functions.

> Source/WebCore/css/CSSImageGeneratorValue.h:52
> -    virtual bool isFixedSize() const { return false; }
> +    ALWAYS_INLINE bool isFixedSize() const { return m_isFixedSize; }

Ditto.

> Source/WebCore/css/CSSImageGeneratorValue.h:71
>      RefPtr<StyleGeneratedImage> m_image;
>      bool m_accessedImage;
> +    bool m_isFixedSize;
>  

As with https://bugs.webkit.org/show_bug.cgi?id=64865 there is almost a whole word of wasted bits in this structure already.
I suggest amending this patch to ensure that the bools are part of the same bitfield.
------- Comment #6 From 2011-10-31 21:44:58 PST -------
Testing 20 runs with PerformanceTests/Parser/html5-full-render.html

At ToT:
Testing 6092696 byte document in 13 500000 byte chunks.
Running 20 times
Ignoring warm-up run (18914)
18792 18919 19019 19500 19449 19532 19488 19498 19525 19518
19451 19620 19572 19560 19372 19037 18872 18780 18834 19016

avg 19267.7
median 19450
stdev 303.52728048727346
min 18780
max 19620

With amended patch:
Testing 6092696 byte document in 13 500000 byte chunks.
Running 20 times
Ignoring warm-up run (20471)
18841 18862 18914 18809 19376 19510 19461 19442 19559 19440
19457 19532 19467 19506 19492 19050 18856 18782 18874 18791

avg 19201.05
median 19408
stdev 310.9341529970614
min 18782
max 19559

Parsing speeds at ToT seem to have doubled since the last time I ran this test.
Unfortunately, the variance also seems to have increased now swamping any
minor performance increment.
The progression between ToT and this patch is just 0.3% or 0.2 stdevs, so it
is not a clear winner.

It is worth noting that the only consumer of CSSImageGeneratorValue::isFixedSize()
is CSSImageGeneratorValue::generatedImage().
I would not expect it to be called often enough to produce a measurable penalty.
------- Comment #7 From 2012-01-08 15:22:25 PST -------
The action requested in this bug was taken in:
http://trac.webkit.org/changeset/99577
------- Comment #8 From 2012-01-08 15:28:52 PST -------

*** This bug has been marked as a duplicate of bug 71824 ***