Add functionality to the renderer thread to purge memory. Not yet called....

Add functionality to the renderer thread to purge memory.  Not yet called.

BUG=23400
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=28415

[darin (slow to review), 2009-10-08 07:05:48]

http://codereview.chromium.org/267021/diff/1/3
File chrome/renderer/render_thread.cc (right):

http://codereview.chromium.org/267021/diff/1/3#newcode535
Line 535: while (!v8::V8::IdleNotification())
are you sure this is guaranteed to terminate?  i would prefer that we just call
IdleNotification twice (or some other small number) instead of looping forever.

http://codereview.chromium.org/267021/diff/1/4
File chrome/renderer/render_thread.h (right):

http://codereview.chromium.org/267021/diff/1/4#newcode177
Line 177: void OnPurgeMemory();
how about calling this OnPurgeCaches instead?  same for the IPC message.  "purge
memory" sounds like it might have negative side-effects.

[Peter Kasting, 2009-10-08 16:55:11]

http://codereview.chromium.org/267021/diff/1/3
File chrome/renderer/render_thread.cc (right):

http://codereview.chromium.org/267021/diff/1/3#newcode535
Line 535: while (!v8::V8::IdleNotification())
On 2009/10/08 07:05:48, darin wrote:
> are you sure this is guaranteed to terminate?

Yes.

>  i would prefer that we just call
> IdleNotification twice (or some other small number) instead of looping
forever.

If you call it a fixed number of times, you currently need 8 calls (IIRC).  The
looping solution is the one the V8 guys told me to use.

http://codereview.chromium.org/267021/diff/1/4
File chrome/renderer/render_thread.h (right):

http://codereview.chromium.org/267021/diff/1/4#newcode177
Line 177: void OnPurgeMemory();
On 2009/10/08 07:05:48, darin wrote:
> how about calling this OnPurgeCaches instead?

But we're not just purging caches.  We're purging things like V8 and tcmalloc. 
And the host side of this is called from a class called MemoryPurger with a
command-line switch of --purge-memory-button and UI that says "Purge memory".  I
think referring only to "caches" would be misleading.

> "purge
> memory" sounds like it might have negative side-effects.

Yes, it might, in terms of speed.  Or was that not the side effect you were
thinking of?

[darin (slow to review), 2009-10-08 17:44:49]

> If you call it a fixed number of times, you currently need 8 calls (IIRC). 
The
> looping solution is the one the V8 guys told me to use.

I didn't realize this was their suggestion.  OK.


> But we're not just purging caches.  We're purging things like V8 and tcmalloc.


I know, but you are only purging disposable stuff.  These are things that you
can get back.  In that sense, they are more like caches.  Purging memory sounds
like getting rid of things that you can't recover.  Anyways, it's a small nit. 
I won't object further if you want to stick with purge memory.

[Peter Kasting, 2009-10-08 17:51:03]

On 2009/10/08 17:44:49, darin wrote:
> > But we're not just purging caches.  We're purging things like V8 and
tcmalloc.
> 
> I know, but you are only purging disposable stuff.  These are things that you
> can get back.  In that sense, they are more like caches.  Purging memory
sounds
> like getting rid of things that you can't recover.

I've left the name the same but added more to the comment on the message
declaration to try and clarify what this means.  See if you think it helps, I'm
waiting on the try servers to OK the patch anyway.

[darin (slow to review), 2009-10-08 18:15:40]

OK, LGTM