[Webkit-unassigned] [Bug 10773] Memory usage grows when reloading google.com/ig

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=10773





------- Comment #23 from sanjay12 at gmail.com  2007-01-19 04:13 PDT -------
Created an attachment (id=12554)
 --> (http://bugs.webkit.org/attachment.cgi?id=12554&action=view)
A better fix?

I'm submitting this as hopefully a solid way to fix the problem. I haven't ran
the test suite against it yet, but at least in fairly extensive browsing of
many different pages which were JS-intensive, nothing appeared to broke, and it
most definitely fixed this particular kind of memory leak (though it still has
not fixed gmail). If the method I use is agreeable then I'll of course run the
automated tests and fix any regression/speed issues if they pop up, but I
honestly can't imagine it being noticably more costly than what's currently in
there.

The basic premise of my patch is to store off pointers to a Document,
JSDocument pair whenever a JSDocument is created in toJS(). Once those are
stored off, in ScriptInterpreter::mark rather than doing what it currently does
with the DOM wrappers, it iterates through the cache, and checks the refcount
of the Document to determine whether the JSDocument and any DOM wrappers
associated with it need to be marked.

My understanding of the Document refcount is as such (please correct me if I'm
wrong):

1. If a Document doesn't have a JSDocument associated with it (which only
happens in the case of no JS whatsoever in the file), it means its refcount
will be 1 while it's being used, and then it hits 0 and takes care off itself.
But, since no JSDocument is created, the pair won't be added to the list of
Documents to check during mark, so it's behavior will be no different than it
is currently.

2. If a Document w/ a JSDocument has a refcount greater than 1, it means it's
currently in use, and as such any DOM wrappers associated with that particular
document need to be marked.

3. If a Document w/ a JSDocument has a refcount of 1, it means that the ONLY
thing left pointing to it is its JSDocument. In this case, don't mark any of
the DOM wrappers associated with that document. When the collector then gets to
its sweep phase, all the DOM wrappers for this document will be swept,
including its JSDocument. When the JSDocument gets swept, Document loses its
last refcount, and then gets deleted itself.


Now, I know that the more popular solution seems to be to make it such that
JSDocument::mark handles checking whether or not it marks itself, and then if
it does, have it mark all the wrappers associated with it. The logic of my
patch is very similar (eg., the document ultimately decides who's getting
marked), but I think my implementation has less risk involved with it. It
seemed to me that in order for the JSDocument::mark approach to work, a few
things would be necessary:

1. First, regardless of whether or not you do it this way, you are still going
to need to keep track of all the JSDocuments that currently exist somehow,
since nothing seems to do this currently. Now it's true that the wrapper hash
map does get Document,JSDocument pairs inserted into it, but it has every other
DOM wrapper inserted into it as well, which means it'd be terribly inefficient
for the purpose of tracking which JSDocuments exist.

2. But even then, how does JSDocument itself know whether or not it shouldn't
mark itself? It's going to coexist with the Document object, and it's always
going to be referenced by some other node in the scenario which causes this
memory leak. So it seems like the only way it'd be able to make that decision
would be through it's Document, in which case assuming that my logic was
correct, the refCount way would be the quickest to know whether or not native
code still needs the document or not.

3. The third, and perhaps biggest issue as I saw it was that currently, when
you have a live JSDocument and run mark on it, it just marks itself and the
JSDocumentPrototype JSObjects. It does not have any links between itself and
any of the elements in the actual file, so in order to get this to work the
interpreter's behavior would have to be changed such that when it starts up on
a new document it creates its new JSDocument as soon as it can, and the either
store off the pointer to it, or passes it around a lot. Then you'd have to
change quite a few entry points where new JSObjects are created, and make sure
that if they're part of the DOM hierarchy, that they get added into the
JSDocument tree structure, or however it is that the JSObjects are linked
together such that if JSDocument was to mark itself, there would be an easy
tree to traverse through in order to make sure all its children were marked as
well. Now it could be that this part of the code is still a little over my
head, but from looking at it, it seemed to me that the solution I implemented
would just work very similar to what was desired, and additionally be much less
risky than changing the whole JSObject hierarchy.

*Please note that I was rather tired when I wrote this, so if parts don't make
sense I'll clarify tomorrow.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.