100893 2012-10-31 14:47:18 -0700 RenderArena has a memory leak and poor efficiency 2012-11-02 15:44:46 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 cevans cevans ap darin eric ggaren hyatt inferno kling koivisto mjs slewis thorton webkit.review.bot oldest_to_newest 755640 0 cevans 2012-10-31 14:47:18 -0700 1) Leak! RenderArena has lifetime tied to a parent Document, but if it grows without bound during that lifetime, it is still a leak -- and one that won't ever get spotted by a memory tool. The intent is that objects within a RenderArena get re-used after a free, but free sizes were only tracked up to 400 bytes. This was historically adequate but now we have (sizes on 64-bit Linux Chromium): RenderNamedFlowThread: 720 bytes RenderSVGText: 480 bytes Possibly more. So these objects leak by definition. Ugh. 2) Incorrectly sized recycler array on 64-bit. The granularity of alloc / free size on 64-bit is 8 bytes: sizeof(void*). However, free list buckets are maintained with a granulatity of 4 bytes across all platforms. Seems like the move to 64-bit computing passed RenderArena by :-P 3) Wasteful interaction with underlying memory allocator backing the arena chunks Whilst the RenderArena constructor has a default size of 4096 bytes, which sounds nice and page-sized for the underlying malloc() allocator, Arena adds a few more bytes to the malloc() size for metadata. On my system, this leads to malloc(4131). tcmalloc (as used by Chromium and also Mac for fastMalloc, as is the case here) has size buckets in this vicinity of 4096 and 4608. So malloc(4131) wastes at least 10% of memory -- and that's not counting the non-perfect tesselation of 4608 into page-sized multiples either. 4) Excessive interaction with the underlying memory alloctor Even a tiny and very simple render (small text file) uses 16k of RenderArena size, so it's wasteful to go into malloc() 4 times. After all, RenderArena is trying to avoid going into malloc() as a goal. No need to go to town here but the default allocation size of 4096 could use a little upping. 755688 1 171735 cevans 2012-10-31 15:49:02 -0700 Created attachment 171735 Patch 755812 2 171735 eric 2012-10-31 19:49:37 -0700 Comment on attachment 171735 Patch LGTM. 755813 3 eric 2012-10-31 19:50:04 -0700 CCing Apple folks who might like to see this go by. 755815 4 eric 2012-10-31 19:51:10 -0700 It seems horrible to me that each RenderSVGText is nearly a half k. :( 755830 5 171735 webkit.review.bot 2012-10-31 20:21:23 -0700 Comment on attachment 171735 Patch Clearing flags on attachment: 171735 Committed r133119: <http://trac.webkit.org/changeset/133119> 755831 6 webkit.review.bot 2012-10-31 20:21:26 -0700 All reviewed patches have been landed. Closing bug. 756413 7 darin 2012-11-01 10:22:13 -0700 (In reply to comment #4) > It seems horrible to me that each RenderSVGText is nearly a half k. :( No kidding! 757682 8 cevans 2012-11-02 15:44:46 -0700 Just for kicks, here's a rough breakdown of RenderSVGText's size: - Inherits from WebCore::RenderSVGBlock (size: 176 === sizeof(RenderBlock)) - Has member of type WebCore::SVGTextLayoutAttributesBuilder (size: 232) - Has member of type WebCore::AffineTransform (size: 48) Interesting that sizeof(WebCore::RenderBlock) isn't exactly small. 171735 2012-10-31 15:49:02 -0700 2012-10-31 20:21:22 -0700 Patch wk_100893.diff text/plain 5676 cevans SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEzMzA4MCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDMzIEBACisyMDEyLTEwLTMxICBDaHJpcyBF dmFucyAgPGNldmFuc0Bnb29nbGUuY29tPgorCisgICAgICAgIFJlbmRlckFyZW5hIGhhcyBhIG1l bW9yeSBsZWFrIGFuZCBwb29yIGVmZmljaWVuY3kKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTEwMDg5MworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIDEpIEF2b2lkIG1lbW9yeSBsZWFrIHRoYXQgcGVyc2lzdHMg Zm9yIHRoZSBEb2N1bWVudCBsaWZldGltZSBieQorICAgICAgICBpbmNyZWFzaW5nIHJlY3ljbGVk IHNpemUgYnVja2V0cyB1cCB0byAxMDI0LiBJdCB3YXMgcHJldmlvdXNseSA0MDAsCisgICAgICAg IGFuZCBzaXplb2YoUmVuZGVyTmFtZWRGbG93VGhyZWFkKSAvIHNpemVvZihSZW5kZXJTVkdUZXh0 KSBib3RoIGJsZXcgdGhpcworICAgICAgICBxdW90YS4gQW4gYXNzZXJ0IHdhcyBhZGRlZCB0byBw cmV2ZW50IHRoaXMgaGFwcGVuaW5nIGFnYWluLgorCisgICAgICAgIDIpIEZpeCB0aGUgc2l6ZSBv ZiB0aGUgcmVjeWxlZCBzaXplIGJ1Y2tldCBhcnJheSBvbiA2NC1iaXQuIFdlIG9ubHkKKyAgICAg ICAgbmVlZCA4IGJ5dGUgZ3JhbnVsYXJpdHkgb24gNjQtYml0LCBidXQgd2UgaGFkIDQuCisKKyAg ICAgICAgMykgVHJ5IGFuZCBwYXNzIHBvd2VyLW9mLXR3byBzaXplcyB0byB0aGUgdW5kZXJseWlu ZyBtYWxsb2MoKSBjYWxsLCBzbworICAgICAgICB0aGF0IHdlJ3JlIHNwYWNlIGVmZmljaWVudC4g V2Ugbm93IHRha2UgQXJlbmEgbWV0YWRhdGEgaW50byBhY2NvdW50LgorCisgICAgICAgIDQpIERv dWJsZSB0aGUgZGVmYXVsdCBSZW5kZXJBcmVuYSBzaXplIGFsbG9jYXRpb24gdG8gODE5MiBieXRl cy4gRXZlbgorICAgICAgICBmb3IgYSByZW5kZXIgb2YgYSB0cml2aWFsIHRleHQgZmlsZSwgNDA5 NiBieXRlcyBpcyBub3QgZW5vdWdoIHRvIHByZXZlbnQKKyAgICAgICAgZXh0cmEgY2FsbHMgaW50 byB0aGUgdW5kZXJseWluZyBtYWxsb2MoKSBmb3IgbW9yZSBhcmVuYSBwb29sLgorCisgICAgICAg ICogcmVuZGVyaW5nL1JlbmRlckFyZW5hLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlJlbmRlckFy ZW5hOjpSZW5kZXJBcmVuYSk6IEFkanVzdCBhcmVuYSBzaXplIHNvIHRoYXQgd2UgcGFzcyBvbiB0 aGUgcGFnZS1zaXplZCBtdWx0aXBsZSB0byB0aGUgdW5kZXJseWluZyBtYWxsb2MoKSBpbXBsZW1l bnRhdGlvbi4KKyAgICAgICAgKFdlYkNvcmU6OlJlbmRlckFyZW5hOjphbGxvY2F0ZSk6CisgICAg ICAgIChXZWJDb3JlOjpSZW5kZXJBcmVuYTo6ZnJlZSk6IEFzc2VydCB0aGF0IHRoZSBhbGxvY2F0 aW9uIHNpemUgaXMgaGFuZGxlZCBieSBvdXIgcmVjeWNsaW5nIGJ1Y2tldHMuCisgICAgICAgICog cmVuZGVyaW5nL1JlbmRlckFyZW5hLmg6CisgICAgICAgIChXZWJDb3JlKTogTWFpbnRhaW4gZnJl ZSBidWNrZXRzIHVwIHRvIDEwMjQgYnl0ZXMgdG8gYXZvaWQgbWVtb3J5IGxlYWsuCisgICAgICAg IChSZW5kZXJBcmVuYSk6IERvdWJsZSB0aGUgZGVmYXVsdCBhbGxvY2F0aW9uIHNpemUgYW5kIGhh bmRsZSA2NC1iaXQgc3lzdGVtcyBtb3JlIGVmZmljaWVudGx5LgorCiAyMDEyLTEwLTMxICBTaGVy aWZmIEJvdCAgPHdlYmtpdC5yZXZpZXcuYm90QGdtYWlsLmNvbT4KIAogICAgICAgICBVbnJldmll d2VkLCByb2xsaW5nIG91dCByMTMzMDQ0LgpJbmRleDogU291cmNlL1dlYkNvcmUvcmVuZGVyaW5n L1JlbmRlckFyZW5hLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1Jl bmRlckFyZW5hLmgJKHJldmlzaW9uIDEzMzAwOSkKKysrIFNvdXJjZS9XZWJDb3JlL3JlbmRlcmlu Zy9SZW5kZXJBcmVuYS5oCSh3b3JraW5nIGNvcHkpCkBAIC00MSwxMiArNDEsMTIgQEAKIAogbmFt ZXNwYWNlIFdlYkNvcmUgewogCi1zdGF0aWMgY29uc3Qgc2l6ZV90IGdNYXhSZWN5Y2xlZFNpemUg PSA0MDA7CitzdGF0aWMgY29uc3Qgc2l6ZV90IGdNYXhSZWN5Y2xlZFNpemUgPSAxMDI0OwogCiBj bGFzcyBSZW5kZXJBcmVuYSB7CiAgICAgV1RGX01BS0VfTk9OQ09QWUFCTEUoUmVuZGVyQXJlbmEp OyBXVEZfTUFLRV9GQVNUX0FMTE9DQVRFRDsKIHB1YmxpYzoKLSAgICBSZW5kZXJBcmVuYSh1bnNp Z25lZCBhcmVuYVNpemUgPSA0MDk2KTsKKyAgICBSZW5kZXJBcmVuYSh1bnNpZ25lZCBhcmVuYVNp emUgPSA4MTkyKTsKICAgICB+UmVuZGVyQXJlbmEoKTsKIAogICAgIC8vIE1lbW9yeSBtYW5hZ2Vt ZW50IGZ1bmN0aW9ucwpAQCAtNjIsOSArNjIsMTAgQEAKIAogICAgIC8vIFRoZSBtYXNrIHVzZWQg dG8gc2VjdXJlIHRoZSByZWN5Y2xlZCBmcmVlbGlzdCBwb2ludGVycy4KICAgICB1aW50cHRyX3Qg bV9tYXNrOwotICAgIC8vIFRoZSByZWN5Y2xlciBhcnJheSBpcyBzcGFyc2Ugd2l0aCB0aGUgaW5k aWNlcyBiZWluZyBtdWx0aXBsZXMgb2YgNCwKLSAgICAvLyBpLmUuLCAwLCA0LCA4LCAxMiwgMTYs IDIwLCAuLi4KLSAgICB2b2lkKiBtX3JlY3ljbGVyc1tnTWF4UmVjeWNsZWRTaXplID4+IDJdOwor ICAgIC8vIFRoZSByZWN5Y2xlciBhcnJheSBpcyBzcGFyc2Ugd2l0aCB0aGUgaW5kaWNlcyBiZWlu ZyBtdWx0aXBsZXMgb2YgdGhlCisgICAgLy8gcm91bmRpbmcgc2l6ZSwgc2l6ZW9mKHZvaWQqKSwg aS5lLiwgMCwgNCwgOCwgMTIsIDE2LCAyMCwgLi4uIG9uIDMyLWJpdC4KKyAgICBzdGF0aWMgY29u c3Qgc2l6ZV90IGtSZWN5Y2xlclNoaWZ0ID0gKHNpemVvZih2b2lkKikgPT0gOCkgPyAzIDogMjsK KyAgICB2b2lkKiBtX3JlY3ljbGVyc1tnTWF4UmVjeWNsZWRTaXplID4+IGtSZWN5Y2xlclNoaWZ0 XTsKIAogICAgIHNpemVfdCBtX3RvdGFsU2l6ZTsKICAgICBzaXplX3QgbV90b3RhbEFsbG9jYXRl ZDsKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJBcmVuYS5jcHAKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlckFyZW5hLmNwcAkocmV2aXNp b24gMTMzMDA5KQorKysgU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlckFyZW5hLmNwcAko d29ya2luZyBjb3B5KQpAQCAtNzAsNiArNzAsMTEgQEAKICAgICA6IG1fdG90YWxTaXplKDApCiAg ICAgLCBtX3RvdGFsQWxsb2NhdGVkKDApCiB7CisgICAgQVNTRVJUKGFyZW5hU2l6ZSA+IHNpemVv ZihBcmVuYSkgKyBBUkVOQV9BTElHTl9NQVNLKTsKKyAgICAvLyBUaGUgdW5kZXJseWluZyBBcmVu YSBjbGFzcyBhbGxvY2F0ZXMgc29tZSBtZXRhZGF0YSBvbiB0b3Agb2Ygb3VyCisgICAgLy8gcmVx dWVzdGVkIHNpemUuIEZhY3RvciB0aGlzIGluIHNvIHRoYXQgd2UgY2FuIGdldCBwZXJmZWN0IHBv d2VyLW9mLXR3bworICAgIC8vIGFsbG9jYXRpb24gc2l6ZXMgcGFzc2VkIHRvIHRoZSB1bmRlcmx5 aW5nIG1hbGxvYygpIGNhbGwuCisgICAgYXJlbmFTaXplIC09IChzaXplb2YoQXJlbmEpICsgQVJF TkFfQUxJR05fTUFTSyk7CiAgICAgLy8gSW5pdGlhbGl6ZSB0aGUgYXJlbmEgcG9vbAogICAgIElO SVRfQVJFTkFfUE9PTCgmbV9wb29sLCAiUmVuZGVyQXJlbmEiLCBhcmVuYVNpemUpOwogCkBAIC05 Nyw2ICsxMDIsNyBAQAogCiB2b2lkKiBSZW5kZXJBcmVuYTo6YWxsb2NhdGUoc2l6ZV90IHNpemUp CiB7CisgICAgQVNTRVJUKHNpemUgPD0gZ01heFJlY3ljbGVkU2l6ZSAtIDMyKTsKICAgICBtX3Rv dGFsU2l6ZSArPSBzaXplOwogCiAjaWZkZWYgQUREUkVTU19TQU5JVElaRVIKQEAgLTExMSwyMSAr MTE3LDE2IEBACiAgICAgaGVhZGVyLT5zaWduYXR1cmUgPSBzaWduYXR1cmU7CiAgICAgcmV0dXJu IHN0YXRpY19jYXN0PGNoYXIqPihibG9jaykgKyBkZWJ1Z0hlYWRlclNpemU7CiAjZWxzZQotICAg IHZvaWQqIHJlc3VsdCA9IDA7Ci0KICAgICAvLyBFbnN1cmUgd2UgaGF2ZSBjb3JyZWN0IGFsaWdu bWVudCBmb3IgcG9pbnRlcnMuICBJbXBvcnRhbnQgZm9yIFRydTY0CiAgICAgc2l6ZSA9IFJPVU5E VVAoc2l6ZSwgc2l6ZW9mKHZvaWQqKSk7CiAKLSAgICAvLyBDaGVjayByZWN5Y2xlcnMgZmlyc3QK LSAgICBpZiAoc2l6ZSA8IGdNYXhSZWN5Y2xlZFNpemUpIHsKLSAgICAgICAgY29uc3Qgc2l6ZV90 IGluZGV4ID0gc2l6ZSA+PiAyOworICAgIGNvbnN0IHNpemVfdCBpbmRleCA9IHNpemUgPj4ga1Jl Y3ljbGVyU2hpZnQ7CiAKLSAgICAgICAgcmVzdWx0ID0gbV9yZWN5Y2xlcnNbaW5kZXhdOwotICAg ICAgICBpZiAocmVzdWx0KSB7Ci0gICAgICAgICAgICAvLyBOZWVkIHRvIG1vdmUgdG8gdGhlIG5l eHQgb2JqZWN0Ci0gICAgICAgICAgICB2b2lkKiBuZXh0ID0gTWFza1B0cigqKCh2b2lkKiopcmVz dWx0KSwgbV9tYXNrKTsKLSAgICAgICAgICAgIG1fcmVjeWNsZXJzW2luZGV4XSA9IG5leHQ7Ci0g ICAgICAgIH0KKyAgICB2b2lkKiByZXN1bHQgPSBtX3JlY3ljbGVyc1tpbmRleF07CisgICAgaWYg KHJlc3VsdCkgeworICAgICAgICAvLyBOZWVkIHRvIG1vdmUgdG8gdGhlIG5leHQgb2JqZWN0Cisg ICAgICAgIHZvaWQqIG5leHQgPSBNYXNrUHRyKCooKHZvaWQqKilyZXN1bHQpLCBtX21hc2spOwor ICAgICAgICBtX3JlY3ljbGVyc1tpbmRleF0gPSBuZXh0OwogICAgIH0KIAogICAgIGlmICghcmVz dWx0KSB7CkBAIC0xNDEsNiArMTQyLDcgQEAKIAogdm9pZCBSZW5kZXJBcmVuYTo6ZnJlZShzaXpl X3Qgc2l6ZSwgdm9pZCogcHRyKQogeworICAgIEFTU0VSVChzaXplIDw9IGdNYXhSZWN5Y2xlZFNp emUgLSAzMik7CiAgICAgbV90b3RhbFNpemUgLT0gc2l6ZTsKIAogI2lmZGVmIEFERFJFU1NfU0FO SVRJWkVSCkBAIC0xNTgsMTMgKzE2MCwxMCBAQAogICAgIC8vIEVuc3VyZSB3ZSBoYXZlIGNvcnJl Y3QgYWxpZ25tZW50IGZvciBwb2ludGVycy4gIEltcG9ydGFudCBmb3IgVHJ1NjQKICAgICBzaXpl ID0gUk9VTkRVUChzaXplLCBzaXplb2Yodm9pZCopKTsKIAotICAgIC8vIFNlZSBpZiBpdCdzIGEg c2l6ZSB0aGF0IHdlIHJlY3ljbGUKLSAgICBpZiAoc2l6ZSA8IGdNYXhSZWN5Y2xlZFNpemUpIHsK LSAgICAgICAgY29uc3Qgc2l6ZV90IGluZGV4ID0gc2l6ZSA+PiAyOwotICAgICAgICB2b2lkKiBj dXJyZW50VG9wID0gbV9yZWN5Y2xlcnNbaW5kZXhdOwotICAgICAgICBtX3JlY3ljbGVyc1tpbmRl eF0gPSBwdHI7Ci0gICAgICAgICooKHZvaWQqKilwdHIpID0gTWFza1B0cihjdXJyZW50VG9wLCBt X21hc2spOwotICAgIH0KKyAgICBjb25zdCBzaXplX3QgaW5kZXggPSBzaXplID4+IGtSZWN5Y2xl clNoaWZ0OworICAgIHZvaWQqIGN1cnJlbnRUb3AgPSBtX3JlY3ljbGVyc1tpbmRleF07CisgICAg bV9yZWN5Y2xlcnNbaW5kZXhdID0gcHRyOworICAgICooKHZvaWQqKilwdHIpID0gTWFza1B0cihj dXJyZW50VG9wLCBtX21hc2spOwogI2VuZGlmCiB9CiAK