Bug 28676

Summary: Safari 4 not releasing memory back to the operating system
Product: WebKit Reporter: Jake Archibald <jaffathecake>
Component: New BugsAssignee: Geoffrey Garen <ggaren>
Status: ASSIGNED ---    
Severity: Normal CC: adachan, dglazkov, eric, ggaren, gns, mjs, webkit.review.bot, xan.lopez
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
URL: http://farm3.static.flickr.com/2552/3851167461_d56de44d47_o.png
Attachments:
Description Flags
patch
oliver: review-
patch
none
patch none

Description Jake Archibald 2009-08-24 02:10:34 PDT
Once Safari has claimed memory from the operating system, it doesn't seem to give it back, even though it's no longer using it.

Running the first test on the following page http://www.jakearchibald.co.uk/jsperformance/closurememory/1.html, this creates a 1mb string every second.

http://www.flickr.com/photos/jaffathecake/3851167461/sizes/o/

Chrome (and other browsers) release the memory back to the operating system when the test is ended, whereas Safari keeps it, even if the tab is closed. Safari will reuse the memory it has reserved, but it doesn't give it back to the operating system, even if it has claimed 700mb+.
Comment 1 Jake Archibald 2009-08-24 02:35:54 PDT
This is an issue in 526 (not tested the nightly builds) but the option wasn't available. Not sure which component this should be part of either.
Comment 2 Mark Rowe (bdash) 2009-08-24 12:32:03 PDT
Please test in a nightly build.  The memory allocator used by Safari on Windows has been improved so that memory is returned to the OS at a more aggressive rate.
Comment 3 Jake Archibald 2009-08-24 14:11:10 PDT
http://farm3.static.flickr.com/2554/3853034847_806c897794_o.png

The above is the same test in a nightly build downloaded today. Although it starts releasing memory, it's not at a useful rate.

In this test (and previous tests) one pixel on the graph == 1 second.
Comment 4 Mark Rowe (bdash) 2009-08-24 14:53:40 PDT
<rdar://problem/7165917>
Comment 5 Geoffrey Garen 2010-01-05 12:34:24 PST
*** Bug 22275 has been marked as a duplicate of this bug. ***
Comment 6 Geoffrey Garen 2010-02-17 17:09:43 PST
Created attachment 48950 [details]
patch
Comment 7 Eric Seidel (no email) 2010-02-17 17:12:08 PST
Attachment 48950 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/278140
Comment 8 WebKit Review Bot 2010-02-17 17:13:31 PST
Attachment 48950 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
JavaScriptCore/wtf/FastMalloc.cpp:1501:  TCMalloc_PageHeap::scavenge is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 WebKit Review Bot 2010-02-17 17:15:36 PST
Attachment 48950 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/279134
Comment 10 WebKit Review Bot 2010-02-17 17:37:29 PST
Attachment 48950 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/279154
Comment 11 WebKit Review Bot 2010-02-17 18:26:15 PST
Attachment 48950 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/280012
Comment 12 Oliver Hunt 2010-02-17 18:27:51 PST
Comment on attachment 48950 [details]
patch

you have removed two #endifs but only one #if this would break stuff
Comment 13 Geoffrey Garen 2010-02-18 00:00:47 PST
Created attachment 48978 [details]
patch

Now with more buildy goodness.
Comment 14 WebKit Review Bot 2010-02-18 00:04:47 PST
Attachment 48978 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
JavaScriptCore/wtf/FastMalloc.cpp:1503:  TCMalloc_PageHeap::scavenge is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Oliver Hunt 2010-02-18 13:32:36 PST
Comment on attachment 48978 [details]
patch

r=me
Comment 16 Geoffrey Garen 2010-02-18 14:19:40 PST
Fixed that edge case: Committed revision 54988.

Still analyzing other causes.

Clearing review flag.
Comment 17 Geoffrey Garen 2010-03-11 23:44:10 PST
Created attachment 50579 [details]
patch

Fixed a bug where the old code would release only one item from each size class per scavenge, potentially leaving large numbers of large-sized objects unreleased for a long time.
Comment 18 WebKit Review Bot 2010-03-11 23:48:13 PST
Attachment 50579 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
JavaScriptCore/wtf/FastMalloc.cpp:1385:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
JavaScriptCore/wtf/FastMalloc.cpp:1386:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
JavaScriptCore/wtf/FastMalloc.cpp:1387:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
JavaScriptCore/wtf/FastMalloc.cpp:1387:  min_free_committed_pages_since_last_scavenge_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/FastMalloc.cpp:1432:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
JavaScriptCore/wtf/FastMalloc.cpp:1438:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
JavaScriptCore/wtf/FastMalloc.cpp:1464:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
JavaScriptCore/wtf/FastMalloc.cpp:1507:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
JavaScriptCore/wtf/FastMalloc.cpp:1517:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
JavaScriptCore/wtf/FastMalloc.cpp:1518:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
JavaScriptCore/wtf/FastMalloc.cpp:1525:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
JavaScriptCore/wtf/FastMalloc.cpp:2401:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 12 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 19 WebKit Review Bot 2010-03-12 00:49:21 PST
Attachment 50579 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/631033
Comment 20 WebKit Review Bot 2010-03-12 01:01:19 PST
Attachment 50579 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/664010
Comment 21 Geoffrey Garen 2010-03-15 19:23:21 PDT
Committed revision 56028.

Still analyzing other causes.

Clearing review flag.