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.
https://codereview.chromium.org/29053002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
