Bug 12659 - JS objects not collected after closing window @ ebay.com/maps.google.com
Summary: JS objects not collected after closing window @ ebay.com/maps.google.com
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P1 Normal
Assignee: Geoffrey Garen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2007-02-06 23:31 PST by Maciej Stachowiak
Modified: 2007-02-27 14:41 PST (History)
3 users (show)

See Also:


Attachments
patch (8.95 KB, patch)
2007-02-27 12:26 PST, Geoffrey Garen
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maciej Stachowiak 2007-02-06 23:31:33 PST
2007-01-25 10:38:22 Chris Petersen:
* SUMMARY
As part of my ongoing performance testing for Leopard, I have been asked to measure for memory leaks on sites that use heavy Javascript. I chose two specific test site: www.ebay.com and maps.google.com . My testing was done with Private browsing turned on and  " Disable WebCore Caches "  checked on (in the Debug caches window)

From my testing , I was seeing remaining referenced javascript objects (in the caches window) after I closed the window that contained these specific sites. 

After closing the window that contained www.ebay.com, the caches window was showing total of 5808 (JS objects) that was referenced.

After closing the window that contained www.maps.google.com, the caches window was showing total of 9823 (JS objects) that was referenced.


Note:  BEFORE I started the steps below, I completed the following. Clear cache and set a empty page as the default in Safari.  Enable Private browsing and turn on "Disable Webcore Caches"


* STEPS TO REPRODUCE
1. On stock 9A348, go to http://maps.google.com or  http://www.ebay.com
2. After each site loads, complete a address search at  http://maps.google.com  or a item search at  http://www.ebay.com
3. Note the amount of JS object are listed in the Caches window when search is completed.
4. Now, close this window and click the "Refresh" button 
5. Notice the number of JS objects are reduced but some remain

In my case, I was seeing the following after closing the window:

After closing http://maps.google.com , the JS objects remained at Reference 1 - Total: 9823 (STRING)
After closing http://www.ebay.com , the JS objects remained at Reference 1 - Total: 5808 (STRING)

2007-01-25 13:19:40 Geoff Garen:
Big memory leak => blocker.

2007-01-29 16:21:18 Lee Bernhard:
Perf BRB: Adding OSX-Perf Leopard Critical and OSX-Perf Memory.  Recommend P1 for Leopard.

2007-01-29 18:08:30 Geoff Garen:
I guess " blocker" means P1, anyway.

2007-02-05 13:15:58 Stephanie Lewis:
By virtue of being Safari Blocker Reviewed, these have been Safari BRB Reviewed

<rdar://problem/4954306>
Comment 1 Feng Qian 2007-02-22 13:57:04 PST
It is probably because properties were not cleared properly when closing the window. In the same window you visited ebay or maps.google, open 'about:blank', and exit, no leaks happen.

(In reply to comment #0)
> 2007-01-25 10:38:22 Chris Petersen:
> * SUMMARY
> As part of my ongoing performance testing for Leopard, I have been asked to
> measure for memory leaks on sites that use heavy Javascript. I chose two
> specific test site: www.ebay.com and maps.google.com . My testing was done with
> Private browsing turned on and  " Disable WebCore Caches "  checked on (in the
> Debug caches window)
> 
> From my testing , I was seeing remaining referenced javascript objects (in the
> caches window) after I closed the window that contained these specific sites. 
> 
> After closing the window that contained www.ebay.com, the caches window was
> showing total of 5808 (JS objects) that was referenced.
> 
> After closing the window that contained www.maps.google.com, the caches window
> was showing total of 9823 (JS objects) that was referenced.
> 
> 
> Note:  BEFORE I started the steps below, I completed the following. Clear cache
> and set a empty page as the default in Safari.  Enable Private browsing and
> turn on "Disable Webcore Caches"
> 
> 
> * STEPS TO REPRODUCE
> 1. On stock 9A348, go to http://maps.google.com or  http://www.ebay.com
> 2. After each site loads, complete a address search at  http://maps.google.com 
> or a item search at  http://www.ebay.com
> 3. Note the amount of JS object are listed in the Caches window when search is
> completed.
> 4. Now, close this window and click the "Refresh" button 
> 5. Notice the number of JS objects are reduced but some remain
> 
> In my case, I was seeing the following after closing the window:
> 
> After closing http://maps.google.com , the JS objects remained at Reference 1 -
> Total: 9823 (STRING)
> After closing http://www.ebay.com , the JS objects remained at Reference 1 -
> Total: 5808 (STRING)
> 
> 2007-01-25 13:19:40 Geoff Garen:
> Big memory leak => blocker.
> 
> 2007-01-29 16:21:18 Lee Bernhard:
> Perf BRB: Adding OSX-Perf Leopard Critical and OSX-Perf Memory.  Recommend P1
> for Leopard.
> 
> 2007-01-29 18:08:30 Geoff Garen:
> I guess " blocker" means P1, anyway.
> 
> 2007-02-05 13:15:58 Stephanie Lewis:
> By virtue of being Safari Blocker Reviewed, these have been Safari BRB Reviewed
> 
> <rdar://problem/4954306>
> 

Comment 2 Maciej Stachowiak 2007-02-22 19:10:19 PST
Might be fixed already, or if not, may be fixed by http://bugs.webkit.org/show_bug.cgi?id=12850
Comment 3 Geoffrey Garen 2007-02-26 07:06:29 PST
Along the lines of what Ian said, if you do a manual GC after closing the window, all the objects get reclaimed. So I think there's something wrong with our "GC when closing the WebView" logic, where we GC just a little bit early, before some significant reference has been cleared.
Comment 4 Geoffrey Garen 2007-02-26 07:20:34 PST
I see a far less significant number of live objects @ maps.google.com, so this may have been partially fixed by bug 12850. Still, I see a large number of live objects @ ebay.com.
Comment 5 Feng Qian 2007-02-26 11:20:07 PST
I doubt this leak has anything to do with bug 12850 which would not release retained objects by going to "about:blank" page. Like Geoffrey said, it has something to do with not calling GC when closing a window. 

BTW, I also saw that a web page having several Frames triggers GC multiple times when leaving the page, each Frame destructor triggers a GC, which is very unnecessary IMH.


(In reply to comment #4)
> I see a far less significant number of live objects @ maps.google.com, so this
> may have been partially fixed by bug 12850. Still, I see a large number of live
> objects @ ebay.com.
> 

Comment 6 Geoffrey Garen 2007-02-26 12:35:31 PST
I have a partial fix for this in my tree. Assigning bug to me.
Comment 7 Geoffrey Garen 2007-02-26 13:16:13 PST
> BTW, I also saw that a web page having several Frames triggers GC multiple
> times when leaving the page, each Frame destructor triggers a GC, which is very
> unnecessary IMH.

Cool. I've filed bug 12900 on that.
Comment 8 Geoffrey Garen 2007-02-27 12:26:55 PST
Created attachment 13408 [details]
patch
Comment 9 Maciej Stachowiak 2007-02-27 12:40:06 PST
Comment on attachment 13408 [details]
patch

r=me

Is there any way to make a test case for this?
Comment 10 Geoffrey Garen 2007-02-27 14:41:00 PST
Committed revision 19894 (with test).