https://codereview.chromium.org/40233002/diff/1/include/v8-defaults.h
File include/v8-defaults.h (right):
https://codereview.chromium.org/40233002/diff/1/include/v8-defaults.h#newcode45
include/v8-defaults.h:45: uint64_t total_physical_memory);
On 2013/10/24 21:06:56, rmcilroy wrote:
On 2013/10/24 18:09:02, Benedikt Meurer wrote:
> There was probably some misunderstanding on our side. I thought we
settled on
> the idea of explicitly initializing a ResourceConstraints object as
discussed
> with danno. We don't want to have any more global state in V8.
>
> Can you just add the explicit total_physical_memory parameter to
> ConfigureResourceConstraintsForCurrentPlatform() below, drop the
> SetDefaultResourceConstraintsForCurrentPlatform() method and do the
following
in
> d8.cc (and similar in Blink)?
>
> ResourceConstraints constraints;
> if (!ConfigureResourceConstraintsForCurrentPlatform(&constraints,
> OS::TotalPhysicalMemory())) { Fatal... }
> SetResourceConstraints(isolate, &constraints);
No, this is what I discussed with Daniel over VC and suggested in my
email.
Unfortunately we can't pass the total physical memory as a parameter
in
ConfigureResourceConstraintsForPlatform() due to the reasons I
outlined in the
email thread, namely that Blink can't get the total physical memory
due to the
sandbox (same problem as V8), and there is no clean way to plumb
through a
resource constraint object from Chrome to the isolate creation points
in Blink
(especially for workers where the thread initialisation doesn't enter
Chrome
code).
I apologize, this misunderstanding appears to be my fault. I
misinterpreted your proposal, I thought we were just discussing where
ConfigureResourceConstraintsForPlatform needs to be called and not
passing around the ResourceConstraint struct through Blink in our VC. I
thought you were proposing adding the size to the existing
ConfigureResourceConstraintsForCurrentPlatform call, not creating a new
one. Sorry, may bad.
I do see a fundamental difference between passing around the total
memory size and the ResourceConstraints struct in Chrome/Blink. The
former seems like something that is useful in other contexts, and
therefore I'm sure that it's possible to get the support to thread this
through, while the second is V8-specific. So, I think "right" thing to
do is indeed pass the total physical memory to
ConfigureResourceConstraintsForPlatform(), and call that in the sandbox
immediately before SetResourceConstraints.
If you need help threading this through and somebody to help shepherd
you CL, I'd recommend getting in touch with Jochen Eisinger. He's in MUC
and has the clout to help get a change like this through.
https://codereview.chromium.org/40233002/
--
--
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.
