31675 2009-11-19 10:16:22 -0800 Memory use grows grows possibly unbounded in this JavaScript Array test case 2010-01-15 10:48:47 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Mac (Intel) OS X 10.6 RESOLVED FIXED HasReduction, InRadar, Regression P2 Normal --- 1 jorgechamorro ggaren ap ggaren jorgechamorro mjs oliver webkit.review.bot oldest_to_newest 165014 0 jorgechamorro 2009-11-19 10:16:22 -0800 Hi, I've run this code in Safari, FF, Opera & Chrome (*). Only Safari fails to GC a's old content: I've been running it in the consoles (just in case it matters). //Create an array of 8M elements and fill each element with an empty string: a= new Array(Math.pow(2,23)); n= a.length; while(n--){a[n]="";}; n= a.length; //Repeat again a few times more until it hangs/stops responding (happens both in Safari and in any nightly build): a= new Array(Math.pow(2,23)); n= a.length; while(n--){a[n]="";}; n= a.length; a= new Array(Math.pow(2,23)); n= a.length; while(n--){a[n]="";}; n= a.length; a= new Array(Math.pow(2,23)); n= a.length; while(n--){a[n]="";}; n= a.length; a= new Array(Math.pow(2,23)); n= a.length; while(n--){a[n]="";}; n= a.length; a= new Array(Math.pow(2,23)); n= a.length; while(n--){a[n]="";}; n= a.length; a's old content should eventually be GC'ed, but it isn't and accumulates on each run: it can be seen with a "top" or in the activity monitor. Other browsers do it well. BTW, why does each array element use so much memory ? This 8M array filled with "" occupies over 5x more memory in Safari than in Chrome or in Opera. (!) Cheers, (*)FF2.0.0.20, FF3.0.13, FF3.5.4, FF3.6b2, Chrome 4.0.249.0, Opera 10.01. -- jorge@jorgechamorro.com 165020 1 jorgechamorro 2009-11-19 10:44:12 -0800 Added keyword regression: it's ok in Safari 3.2.3. 165041 2 jorgechamorro 2009-11-19 11:53:08 -0800 Prepare the activity monitor and then surf to: http://jorgechamorro.com/bug31675.html RUN and see the memory increasing on each RUN... until it hangs. -- Jorge. 165064 3 mrowe 2009-11-19 12:48:44 -0800 <rdar://problem/7409500> 165075 4 oliver 2009-11-19 13:41:42 -0800 The obvious reason for the large memory usage relative to other engines is that we don't coalesce array storage when an array is built from top to bottom :-/ 178791 5 ggaren 2010-01-08 14:03:10 -0800 What I see in one iteration of this test: - Firefox uses about 350MB but promptly returns to 0. So, it seems to use a sparse array that is promptly GC'd. - Chrome uses about 25MB but crashes with larger arrays. So, it seems to use a dense array. - Safari uses about 600MB and does not promptly return to 0. I'm going to investigate why Safari's sparse array is so space-hungry, and why it doesn't return to 0. Making the array dense once it's filled might help a bit, but it won't help with the initial 600MB. 178816 6 ggaren 2010-01-08 14:42:45 -0800 malloc-history confirms that, for a single run, 268MB is allocated by the sparse map. That's enough space for about 2 * 2^23 entries. (There are 2^23 entries, and the map doesn't want to be more than half full.) 178860 7 ggaren 2010-01-08 16:06:34 -0800 Also, it looks like arrays don't report extra cost for their sparse storage, which would explain, on multiple runs, why no arrays are ever GC'd, and memory keeps growing. 178879 8 46178 ggaren 2010-01-08 16:50:32 -0800 Created attachment 46178 Patch 178880 9 ggaren 2010-01-08 16:54:21 -0800 (ChangeLog tabs fixed in local copy.) 178881 10 webkit.review.bot 2010-01-08 16:55:18 -0800 Attachment 46178 did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 JavaScriptCore/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] JavaScriptCore/ChangeLog:9: Line contains tab character. [whitespace/tab] [5] JavaScriptCore/ChangeLog:11: Line contains tab character. [whitespace/tab] [5] JavaScriptCore/ChangeLog:15: Line contains tab character. [whitespace/tab] [5] Total errors found: 4 178883 11 46178 oliver 2010-01-08 16:59:43 -0800 Comment on attachment 46178 Patch r=me 178884 12 ggaren 2010-01-08 17:02:54 -0800 Committed r53025: <http://trac.webkit.org/changeset/53025> 179136 13 jorgechamorro 2010-01-10 07:37:36 -0800 (In reply to comment #12) > Committed r53025: <http://trac.webkit.org/changeset/53025> Hi Geoffrey, Sorry for spoiling the fun :-) but there must be -I believe- something else... I've put two buttons more in the test page, one to null "a", and one to reload the page: ISTM that a's (old contents) memory is never freed/gc'ed/released, not even after nulling a nor after nulling a and reloading, nor after nulling a and closing the window/tab, even when/if other tabs/windows would need it. Chrome, Opera and FF eventually release it, but our much beloved Safari still doesn't. Oh, and btw, memory use still grows up to 2GB (!) in a MBP in 64 bits mode (~1GB in 32bits mode), but in Chrome in takes less than 100MB, in Opera less than 300Mb, and in FF less that 500MB ! May it have to do with the page cache ? Cheers, -- Jorge. 179530 14 ggaren 2010-01-11 13:58:53 -0800 (In reply to comment #13) Your new test uses more memory than your old test because your new test allocates a larger array. I don't know what combination of "f()" and/or "null" and/or "reload" you're using, so I'm not sure if I tested the way you did. I don't know how you measured whether memory was freed, but I found that reloading and closing the window did free memory. I suspect you may be re-reporting bug 28676. > Oh, and btw, memory use still grows up to 2GB (!) in a MBP in 64 bits mode > (~1GB in 32bits mode), but in Chrome in takes less than 100MB, in Opera less > than 300Mb, and in FF less that 500MB ! I don't think the Chrome numbers are relevant, since they represent the prima facia unacceptable policy of blindly allocating buffers to user requested sizes, even if doing so means crashing. But it would be nice to do something a bit smarter in the case of large arrays that start out sparse and become non-sparse. > May it have to do with the page cache ? No. 181275 15 jorgechamorro 2010-01-15 10:48:47 -0800 (In reply to comment #14) > > I don't know how you measured whether memory was freed, but I found that > reloading and closing the window did free memory. I click on run() then on a=null the on run() then on a=null etc many times, and the memory use (as seen in the activity monitor system memory pie chart (in yellow)) keeps ever increasing. A reload() does not release it either. Nor closing the tab/window. But a cmd-Q does. > I suspect you may be re-reporting bug 28676. Maybe. If the memory is ever deallocated it is not being returned to the system. But ISTM that it's not being reused internally either: if it were, why would memory use keep ever increasing (?). 46178 2010-01-08 16:50:32 -0800 2010-01-08 16:59:43 -0800 Patch bug-31675-20100108165030.patch text/plain 2911 ggaren SW5kZXg6IEphdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBKYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDUzMDI0KQorKysgSmF2YVNjcmlwdENvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjAgQEAKKzIwMTAtMDEtMDggIEdlb2ZmcmV5 IEdhcmVuICA8Z2dhcmVuQGFwcGxlLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBNZW1vcnkgdXNlIGdyb3dzIGdyb3dzIHBvc3NpYmx5IHVuYm91 bmRlZCBpbiB0aGlzIEphdmFTY3JpcHQgQXJyYXkgdGVzdCBjYXNlCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zMTY3NQorCisJVGhpcyBmaXhlcyBvbmUg b2JzZXJ2ZWQgYnVnIGluIHRoaXMgdGVzdCBjYXNlLCB3aGljaCBpcyB0aGF0CisJYXJyYXlzIGRv bid0IHJlcG9ydCBleHRyYSBjb3N0IGZvciB0aGUgc3BhcnNlIHZhbHVlIG1hcHMuCisKKwlTdW5T cGlkZXIgcmVwb3J0cyBhIHNtYWxsIHNwZWVkdXAuCisKKyAgICAgICAgKiBydW50aW1lL0pTQXJy YXkuY3BwOgorICAgICAgICAoSlNDOjpKU0FycmF5OjpwdXRTbG93Q2FzZSk6IFJlcG9ydCBleHRy YSBtZW1vcnkgY29zdCBmb3IKKwl0aGUgc3BhcnNlIHZhbHVlIG1hcC4KKyAgICAgICAgKiBydW50 aW1lL0pTQXJyYXkuaDogCisKIDIwMTAtMDEtMDggIFlvbmcgTGkgIDx5b2xpQHJpbS5jb20+CiAK ICAgICAgICAgUmV2aWV3ZWQgYnkgRGFyaW4gQWRsZXIuCkluZGV4OiBKYXZhU2NyaXB0Q29yZS9y dW50aW1lL0pTQXJyYXkuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIEphdmFTY3JpcHRDb3JlL3J1bnRpbWUv SlNBcnJheS5jcHAJKHJldmlzaW9uIDUyOTkzKQorKysgSmF2YVNjcmlwdENvcmUvcnVudGltZS9K U0FycmF5LmNwcAkod29ya2luZyBjb3B5KQpAQCAtMzI5LDEzICszMjksMjQgQEAgTkVWRVJfSU5M SU5FIHZvaWQgSlNBcnJheTo6cHV0U2xvd0Nhc2UoRQogICAgICAgICB9CiAKICAgICAgICAgLy8g V2UgbWlzcyBzb21lIGNhc2VzIHdoZXJlIHdlIGNvdWxkIGNvbXBhY3QgdGhlIHN0b3JhZ2UsIHN1 Y2ggYXMgYSBsYXJnZSBhcnJheSB0aGF0IGlzIGJlaW5nIGZpbGxlZCBmcm9tIHRoZSBlbmQKLSAg ICAgICAgLy8gKHdoaWNoIHdpbGwgb25seSBiZSBjb21wYWN0ZWQgYXMgd2UgcmVhY2ggaW5kaWNl cyB0aGF0IGFyZSBsZXNzIHRoYW4gY3V0b2ZmKSAtIGJ1dCB0aGlzIG1ha2VzIHRoZSBjaGVjayBt dWNoIGZhc3Rlci4KKyAgICAgICAgLy8gKHdoaWNoIHdpbGwgb25seSBiZSBjb21wYWN0ZWQgYXMg d2UgcmVhY2ggaW5kaWNlcyB0aGF0IGFyZSBsZXNzIHRoYW4gTUlOX1NQQVJTRV9BUlJBWV9JTkRF WCkgLSBidXQgdGhpcyBtYWtlcyB0aGUgY2hlY2sgbXVjaCBmYXN0ZXIuCiAgICAgICAgIGlmICgo aSA+IE1BWF9TVE9SQUdFX1ZFQ1RPUl9JTkRFWCkgfHwgIWlzRGVuc2VFbm91Z2hGb3JWZWN0b3Io aSArIDEsIHN0b3JhZ2UtPm1fbnVtVmFsdWVzSW5WZWN0b3IgKyAxKSkgewogICAgICAgICAgICAg aWYgKCFtYXApIHsKICAgICAgICAgICAgICAgICBtYXAgPSBuZXcgU3BhcnNlQXJyYXlWYWx1ZU1h cDsKICAgICAgICAgICAgICAgICBzdG9yYWdlLT5tX3NwYXJzZVZhbHVlTWFwID0gbWFwOwogICAg ICAgICAgICAgfQotICAgICAgICAgICAgbWFwLT5zZXQoaSwgdmFsdWUpOworCisgICAgICAgICAg ICBwYWlyPFNwYXJzZUFycmF5VmFsdWVNYXA6Oml0ZXJhdG9yLCBib29sPiByZXN1bHQgPSBtYXAt PmFkZChpLCB2YWx1ZSk7CisgICAgICAgICAgICBpZiAoIXJlc3VsdC5zZWNvbmQpIHsgLy8gcHJl LWV4aXN0aW5nIGVudHJ5CisgICAgICAgICAgICAgICAgcmVzdWx0LmZpcnN0LT5zZWNvbmQgPSB2 YWx1ZTsKKyAgICAgICAgICAgICAgICByZXR1cm47CisgICAgICAgICAgICB9CisKKyAgICAgICAg ICAgIHNpemVfdCBjYXBhY2l0eSA9IG1hcC0+Y2FwYWNpdHkoKTsKKyAgICAgICAgICAgIGlmIChj YXBhY2l0eSAhPSBzdG9yYWdlLT5yZXBvcnRlZE1hcENhcGFjaXR5KSB7CisgICAgICAgICAgICAg ICAgSGVhcDo6aGVhcCh0aGlzKS0+cmVwb3J0RXh0cmFNZW1vcnlDb3N0KChjYXBhY2l0eSAtIHN0 b3JhZ2UtPnJlcG9ydGVkTWFwQ2FwYWNpdHkpICogKHNpemVvZih1bnNpZ25lZCkgKyBzaXplb2Yo SlNWYWx1ZSkpKTsKKyAgICAgICAgICAgICAgICBzdG9yYWdlLT5yZXBvcnRlZE1hcENhcGFjaXR5 ID0gY2FwYWNpdHk7CisgICAgICAgICAgICB9CiAgICAgICAgICAgICByZXR1cm47CiAgICAgICAg IH0KICAgICB9CkluZGV4OiBKYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTQXJyYXkuaAo9PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09Ci0tLSBKYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTQXJyYXkuaAkocmV2aXNpb24gNTI5OTMp CisrKyBKYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTQXJyYXkuaAkod29ya2luZyBjb3B5KQpAQCAt MzIsNiArMzIsNyBAQCBuYW1lc3BhY2UgSlNDIHsKICAgICAgICAgdW5zaWduZWQgbV9udW1WYWx1 ZXNJblZlY3RvcjsKICAgICAgICAgU3BhcnNlQXJyYXlWYWx1ZU1hcCogbV9zcGFyc2VWYWx1ZU1h cDsKICAgICAgICAgdm9pZCogbGF6eUNyZWF0aW9uRGF0YTsgLy8gQSBKU0FycmF5IHN1YmNsYXNz IGNhbiB1c2UgdGhpcyB0byBmaWxsIHRoZSB2ZWN0b3IgbGF6aWx5LgorICAgICAgICBzaXplX3Qg cmVwb3J0ZWRNYXBDYXBhY2l0eTsKICAgICAgICAgSlNWYWx1ZSBtX3ZlY3RvclsxXTsKICAgICB9 OwogCg==