Provide a mechanism for the embedder to set a TotalPhysicalMemory callback function.

Provide a mechanism for the embedder to set a TotalPhysicalMemory callback function.

This is necessary due to the fact that sysconf (used by OS::TotalPhysicalMemory)
is blocked by the Chromium renderer's sandbox.

BUG=None

[rmcilroy, 2013-10-18 13:02:53]

Jakob: I chatted with Daniel and we decided to go with the callback mechanism. 
PTAL.

[Benedikt Meurer, 2013-10-21 07:05:54]

The whole idea of adding the total physical memory stuff to V8 was that we don't
want to duplicate the code. But if we need a callback function here for Chrome,
then we seem to actually duplicate the code anyway, or am I missing something?
Besides that, we should not add any new calls to Isolate::*Current() in our
code. Why can't we pass the total physical memory as optional parameter when
creating the isolate?

[Sven Panne, 2013-10-21 07:11:04]

NOT LGTM. The API is wrong, and the whole concept is a bit strange: I don't
really believe that probing the physical memory in any way is the right way to
go, but even if I did: This is something *global*, not isolate-local.

Even if we want to keep this physical memory concept, the API has to change to
make it clear that this is a global setting. An ugly hack would be pretending
that it is isolate-local, but then we should pass the isolate and mark it
clearly as a hack.

https://codereview.chromium.org/29053002/diff/1/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/29053002/diff/1/src/api.cc#newcode6440
src/api.cc:6440: i::Isolate* isolate = i::Isolate::Current();
o_O This is fundamentally wrong, and I can't see how we can ever remove
Current() here. Note that this is on death row already, and I will object any CL
which introduces further calls of it.

[rmcilroy, 2013-10-21 10:16:50]

On 2013/10/21 07:11:04, Sven Panne wrote:
> NOT LGTM. The API is wrong, and the whole concept is a bit strange: I don't
> really believe that probing the physical memory in any way is the right way to
> go, but even if I did: This is something *global*, not isolate-local.
> 
> Even if we want to keep this physical memory concept, the API has to change to
> make it clear that this is a global setting. An ugly hack would be pretending
> that it is isolate-local, but then we should pass the isolate and mark it
> clearly as a hack.

OK, I was following the behaviour of the other SetXXXCallbackFunction methods,
which also save the function callback on the isolate even though they also
represent global things (e.g., the counter function, etc.), but I agree this is
global not isolate-local.  If I make this a global operation it will need to set
a function pointer on a static variable somewhere (possibly in defaults.cc,
using AtomicWord?) - would that be acceptable, or can you think of an
alternative?

> The whole idea of adding the total physical memory stuff to V8 was that we
don't
> want to duplicate the code. But if we need a callback function here for
Chrome,
> then we seem to actually duplicate the code anyway, or am I missing something?
> Besides that, we should not add any new calls to Isolate::*Current() in our
> code. Why can't we pass the total physical memory as optional parameter when
> creating the isolate?

Chrome is a special case where it is in a sandbox and so has to provide it's own
implementation of this function which retrieves the value before the sandbox is
sealed. Even if we removed the OS::TotalPhysicalMemory() function from V8, d8
(or any other embedder) would need to pass it's own total physical memory
function, therefore we would still need this code in the V8 codebase for d8.  We
can't pass the physical memory as an optional parameter when the isolate is
created because Chrome uses the default isolate, which doesn't have a
constructor to which you can pass parameters.

[rmcilroy, 2013-10-24 13:53:53]

Dropping this in favour of https://codereview.chromium.org/40233002/ as
discussed offline.