64862 2011-07-20 04:32:40 -0700 Switch isFixedSize() virtual function to inline one. 2012-01-08 15:28:52 -0800 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) All All RESOLVED DUPLICATE 71824 P2 Normal --- 64262 1 tczene davidbarr davidbarr hyatt loki shanestephens oldest_to_newest 439737 0 tczene 2011-07-20 04:32:40 -0700 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% 439739 1 101448 tczene 2011-07-20 04:38:03 -0700 Created attachment 101448 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 439968 2 hyatt 2011-07-20 13:25:31 -0700 See my comments in https://bugs.webkit.org/show_bug.cgi?id=64262. 465966 3 101448 eric 2011-09-12 17:01:08 -0700 Comment on attachment 101448 Switch isFixedSize virtual function to inline one. Please respond to Dave Hyatt. Again, memory hit. 482055 4 davidbarr 2011-10-11 17:09:37 -0700 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. 487036 5 davidbarr 2011-10-19 14:55:38 -0700 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. 493737 6 davidbarr 2011-10-31 21:44:58 -0700 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. 531835 7 davidbarr 2012-01-08 15:22:25 -0800 The action requested in this bug was taken in: http://trac.webkit.org/changeset/99577 531842 8 davidbarr 2012-01-08 15:28:52 -0800 *** This bug has been marked as a duplicate of bug 71824 *** 101448 2011-07-20 04:38:03 -0700 2011-09-12 17:01:08 -0700 Switch isFixedSize virtual function to inline one. 0001-Switch-isFixedSize-virtual-function-to-inline-one.patch text/plain 3581 tczene RnJvbSA0MTM3ZjI1ZTJiY2I0NjRiNGM5OThlMTAyZmI1ODdlOTY5Mzc4NWFlIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBUYW1hcyBDemVuZSA8Q3plbmUuVGFtYXNAc3R1ZC51LXN6ZWdl ZC5odT4KRGF0ZTogV2VkLCAyMCBKdWwgMjAxMSAxMzozMzo1NiArMDIwMApTdWJqZWN0OiBbUEFU Q0hdIFN3aXRjaCBpc0ZpeGVkU2l6ZSgpIHZpcnR1YWwgZnVuY3Rpb24gdG8gaW5saW5lIG9uZS4K Ci0tLQogU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nICAgICAgICAgICAgICAgICAgICAgIHwgICAx NiArKysrKysrKysrKysrKysrCiBTb3VyY2UvV2ViQ29yZS9jc3MvQ1NTQ2FudmFzVmFsdWUuaCAg ICAgICAgICAgfCAgICAyICstCiBTb3VyY2UvV2ViQ29yZS9jc3MvQ1NTSW1hZ2VHZW5lcmF0b3JW YWx1ZS5jcHAgfCAgICAxICsKIFNvdXJjZS9XZWJDb3JlL2Nzcy9DU1NJbWFnZUdlbmVyYXRvclZh bHVlLmggICB8ICAgIDQgKysrLQogNCBmaWxlcyBjaGFuZ2VkLCAyMSBpbnNlcnRpb25zKCspLCAy IGRlbGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCBmZDZhNzVkLi5kMjMwMGFlIDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9n CkBAIC0xLDMgKzEsMTkgQEAKKzIwMTEtMDctMjAgIFRhbWFzIEN6ZW5lICA8Q3plbmUuVGFtYXNA c3R1ZC51LXN6ZWdlZC5odT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBTd2l0Y2ggaXNGaXhlZFNpemUoKSB2aXJ0dWFsIGZ1bmN0aW9uIHRvIGlubGlu ZSBvbmUuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02 NDg2MgorCisgICAgICAgIFJlbW92aW5nIHRoZSB2aXJ0dWFsIGZ1bmN0aW9ucyAtIHJlbW92aW5n IHRoZSBjYWxscyAtIG1ha2VzIG1lYXN1cmFibGUgcGVyZm9ybWFuY2UgcHJvZ3Jlc3Npb24gb24g Q1NTLgorCisgICAgICAgICogY3NzL0NTU0NhbnZhc1ZhbHVlLmg6CisgICAgICAgIChXZWJDb3Jl OjpDU1NDYW52YXNWYWx1ZTo6Q1NTQ2FudmFzVmFsdWUpOlJlbW92ZSB2aXJ0dWFsIGZ1bmN0aW9u LCBhZGQgYWRkaXRpb25hbCB2YWx1ZSB0byB0aGUgdmFyaWFibGUuCisgICAgICAgICogY3NzL0NT U0ltYWdlR2VuZXJhdG9yVmFsdWUuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Q1NTSW1hZ2VHZW5l cmF0b3JWYWx1ZTo6Q1NTSW1hZ2VHZW5lcmF0b3JWYWx1ZSk6SW5pdGlhbGl6ZSB0aGUgdmFyaWFi bGUuCisgICAgICAgICogY3NzL0NTU0ltYWdlR2VuZXJhdG9yVmFsdWUuaDoKKyAgICAgICAgKFdl YkNvcmU6OkNTU0ltYWdlR2VuZXJhdG9yVmFsdWU6OmlzRml4ZWRTaXplKTpTd2l0Y2ggdmlydHVh bCBmdW5jdGlvbiB0byBpbmxpbmUgZnVuY3Rpb24uCisKIDIwMTEtMDctMjAgIFNoZXJpZmYgQm90 ICA8d2Via2l0LnJldmlldy5ib3RAZ21haWwuY29tPgogCiAgICAgICAgIFVucmV2aWV3ZWQsIHJv bGxpbmcgb3V0IHI5MTI4NS4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2Nzcy9DU1NDYW52 YXNWYWx1ZS5oIGIvU291cmNlL1dlYkNvcmUvY3NzL0NTU0NhbnZhc1ZhbHVlLmgKaW5kZXggMjdi OGYyYi4uYjZjNmE5NCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvY3NzL0NTU0NhbnZhc1Zh bHVlLmgKKysrIGIvU291cmNlL1dlYkNvcmUvY3NzL0NTU0NhbnZhc1ZhbHVlLmgKQEAgLTQxLDcg KzQxLDYgQEAgcHVibGljOgogICAgIHZpcnR1YWwgU3RyaW5nIGNzc1RleHQoKSBjb25zdDsKIAog ICAgIHZpcnR1YWwgUGFzc1JlZlB0cjxJbWFnZT4gaW1hZ2UoUmVuZGVyT2JqZWN0KiwgY29uc3Qg SW50U2l6ZSYpOwotICAgIHZpcnR1YWwgYm9vbCBpc0ZpeGVkU2l6ZSgpIGNvbnN0IHsgcmV0dXJu IHRydWU7IH0KICAgICB2aXJ0dWFsIEludFNpemUgZml4ZWRTaXplKGNvbnN0IFJlbmRlck9iamVj dCopOwogCiAgICAgdm9pZCBzZXROYW1lKGNvbnN0IFN0cmluZyYgbmFtZSkgeyBtX25hbWUgPSBu YW1lOyB9CkBAIC01MCw2ICs0OSw3IEBAIHByaXZhdGU6CiAgICAgQ1NTQ2FudmFzVmFsdWUoKQog ICAgICAgICA6IG1fZWxlbWVudCgwKQogICAgIHsKKyAgICAgICAgbV9pc0ZpeGVkU2l6ZSA9IHRy dWU7CiAgICAgfQogCiAgICAgdmlydHVhbCB2b2lkIGNhbnZhc0NoYW5nZWQoSFRNTENhbnZhc0Vs ZW1lbnQqLCBjb25zdCBGbG9hdFJlY3QmIGNoYW5nZWRSZWN0KTsKZGlmZiAtLWdpdCBhL1NvdXJj ZS9XZWJDb3JlL2Nzcy9DU1NJbWFnZUdlbmVyYXRvclZhbHVlLmNwcCBiL1NvdXJjZS9XZWJDb3Jl L2Nzcy9DU1NJbWFnZUdlbmVyYXRvclZhbHVlLmNwcAppbmRleCA3ODRmNDM4Li43YTE4Y2I0IDEw MDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9jc3MvQ1NTSW1hZ2VHZW5lcmF0b3JWYWx1ZS5jcHAK KysrIGIvU291cmNlL1dlYkNvcmUvY3NzL0NTU0ltYWdlR2VuZXJhdG9yVmFsdWUuY3BwCkBAIC0z Nyw2ICszNyw3IEBAIG5hbWVzcGFjZSBXZWJDb3JlIHsKIAogQ1NTSW1hZ2VHZW5lcmF0b3JWYWx1 ZTo6Q1NTSW1hZ2VHZW5lcmF0b3JWYWx1ZSgpCiA6IG1fYWNjZXNzZWRJbWFnZShmYWxzZSkKKywg bV9pc0ZpeGVkU2l6ZShmYWxzZSkKIHsKIH0KIApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUv Y3NzL0NTU0ltYWdlR2VuZXJhdG9yVmFsdWUuaCBiL1NvdXJjZS9XZWJDb3JlL2Nzcy9DU1NJbWFn ZUdlbmVyYXRvclZhbHVlLmgKaW5kZXggNDY1MjRmYi4uMDM3YTg3YyAxMDA2NDQKLS0tIGEvU291 cmNlL1dlYkNvcmUvY3NzL0NTU0ltYWdlR2VuZXJhdG9yVmFsdWUuaAorKysgYi9Tb3VyY2UvV2Vi Q29yZS9jc3MvQ1NTSW1hZ2VHZW5lcmF0b3JWYWx1ZS5oCkBAIC0yOCw2ICsyOCw3IEBACiAKICNp bmNsdWRlICJDU1NWYWx1ZS5oIgogI2luY2x1ZGUgIkludFNpemVIYXNoLmgiCisjaW5jbHVkZSA8 d3RmL0Fsd2F5c0lubGluZS5oPgogI2luY2x1ZGUgPHd0Zi9IYXNoTWFwLmg+CiAjaW5jbHVkZSA8 d3RmL0hhc2hDb3VudGVkU2V0Lmg+CiAjaW5jbHVkZSA8d3RmL1JlZlB0ci5oPgpAQCAtNDgsNyAr NDksNyBAQCBwdWJsaWM6CiAKICAgICBTdHlsZUdlbmVyYXRlZEltYWdlKiBnZW5lcmF0ZWRJbWFn ZSgpOwogICAgIAotICAgIHZpcnR1YWwgYm9vbCBpc0ZpeGVkU2l6ZSgpIGNvbnN0IHsgcmV0dXJu IGZhbHNlOyB9CisgICAgQUxXQVlTX0lOTElORSBib29sIGlzRml4ZWRTaXplKCkgY29uc3QgeyBy ZXR1cm4gbV9pc0ZpeGVkU2l6ZTsgfQogICAgIHZpcnR1YWwgSW50U2l6ZSBmaXhlZFNpemUoY29u c3QgUmVuZGVyT2JqZWN0KikgeyByZXR1cm4gSW50U2l6ZSgpOyB9CiAgICAgCiBwcm90ZWN0ZWQ6 CkBAIC02Niw2ICs2Nyw3IEBAIHByb3RlY3RlZDoKICAgICAKICAgICBSZWZQdHI8U3R5bGVHZW5l cmF0ZWRJbWFnZT4gbV9pbWFnZTsKICAgICBib29sIG1fYWNjZXNzZWRJbWFnZTsKKyAgICBib29s IG1faXNGaXhlZFNpemU7CiAKIHByaXZhdGU6CiAgICAgdmlydHVhbCBib29sIGlzSW1hZ2VHZW5l cmF0b3JWYWx1ZSgpIGNvbnN0IHsgcmV0dXJuIHRydWU7IH0KLS0gCjEuNy4xCgo=