Bug 31675 - Memory use grows grows possibly unbounded in this JavaScript Array test case
Summary: Memory use grows grows possibly unbounded in this JavaScript Array test case
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh Intel OS X 10.6
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords: HasReduction, InRadar, Regression
Depends on:
Blocks:
 
Reported: 2009-11-19 10:16 PST by Jorge
Modified: 2010-01-15 10:48 PST (History)
6 users (show)

See Also:


Attachments
Patch (2.84 KB, patch)
2010-01-08 16:50 PST, Geoffrey Garen
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jorge 2009-11-19 10:16:22 PST
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
Comment 1 Jorge 2009-11-19 10:44:12 PST
Added keyword regression: it's ok in Safari 3.2.3.
Comment 2 Jorge 2009-11-19 11:53:08 PST
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.
Comment 3 Mark Rowe (bdash) 2009-11-19 12:48:44 PST
<rdar://problem/7409500>
Comment 4 Oliver Hunt 2009-11-19 13:41:42 PST
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 :-/
Comment 5 Geoffrey Garen 2010-01-08 14:03:10 PST
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.
Comment 6 Geoffrey Garen 2010-01-08 14:42:45 PST
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.)
Comment 7 Geoffrey Garen 2010-01-08 16:06:34 PST
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.
Comment 8 Geoffrey Garen 2010-01-08 16:50:32 PST
Created attachment 46178 [details]
Patch
Comment 9 Geoffrey Garen 2010-01-08 16:54:21 PST
(ChangeLog tabs fixed in local copy.)
Comment 10 WebKit Review Bot 2010-01-08 16:55:18 PST
Attachment 46178 [details] 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
Comment 11 Oliver Hunt 2010-01-08 16:59:43 PST
Comment on attachment 46178 [details]
Patch

r=me
Comment 12 Geoffrey Garen 2010-01-08 17:02:54 PST
Committed r53025: <http://trac.webkit.org/changeset/53025>
Comment 13 Jorge 2010-01-10 07:37:36 PST
(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.
Comment 14 Geoffrey Garen 2010-01-11 13:58:53 PST
(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.
Comment 15 Jorge 2010-01-15 10:48:47 PST
(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 (?).