Avoid calling OS::TotalPhysicalMemory in defaults.cc, since this fails in the chrome renderer sandb…

include/v8-defaults.h

Index: include/v8-defaults.h
diff --git a/include/v8-defaults.h b/include/v8-defaults.h
index 381a48210d1b9895fbda4e4c89eb0e51fa61b6d0..4edba558a186a7e572cfe58058631be5b886dd52 100644
--- a/include/v8-defaults.h
+++ b/include/v8-defaults.h
@@ -35,6 +35,16 @@
  */
 namespace v8 {
 
+
+/**
+ * Initializes the v8-defaults subsystem.  Should be called before
+ * ConfigureResourceConstraintsForCurrentPlatform or
+ * SetDefaultResourceConstraintsForCurrentPlatform.
+ */
+void V8_EXPORT InitializeDefaultsForCurrentPlatform(
+    uint64_t total_physical_memory);

[Benedikt Meurer, 2013/10/24 18:09:02]

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);

[rmcilroy, 2013/10/24 21:06:56]

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).

[danno, 2013/10/25 07:26:15]

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.

+
+
 /**
  * Configures the constraints with reasonable default values based on the
  * capabilities of the current device the VM is running on.