On 2013/09/20 07:12:32, danno wrote:
I don't think adding more #ifdefs to V8 or adding memory_constrained is
the
right way to go. The core of V8 is not the best place to consolidate the
logic
for the multiple devices and configurations where it needs to run well.
Defaults
should be just that, defaults, and I really don't think they should be
device
or
OS specific (at least at the V8 level) or do probing on the memory
system...
then they would no longer be defaults.
I would really like to see this split up. V8 should have device
independent
defaults for memory (perhaps except for 32 vs. 64 bits, where they should
double), and I am skeptical about the approach of letting
embedder-specific
flags trickle into the core implementation. The embedder should specify
appropriate memory limits, but we can still make this easy by creating a
memory-defaults.cc somewhere in the source tree that is used by d8 and any
other
embedder where the collective knowledge of "the right thing to do for a
platform" with the ifdefs and platform/OS-specific probing happens to
figure
out
what the overrides should be before passing them in.
To be clear, I was not adding any new #ifdefs here - V8 already does device
specific configuration of the defaults here, this CL just moved the #ifdefs
around to enable runtime information to be taken into account as well. I
understood that the is_memory_constrained flag was a stopgap measure to
isolate
changes made to V8 for low-end devices so we didn't impact performance, but
from
the discussion here (and on the Blink CL which plumbed that flag though) it
seems that we should just drop that flag and move to an approach where the
V8
embedder sets more fine-grained memory constraint (e.g., total available
memory)
on V8, and V8 sizes heap spaces and does other things like code object
pre-aging
based on this. How does this sound to everyone?
I agree that the defaults should not be set in a platform specific way
(e.g.,
Android vs. x86, etc.), in part for the very reason I implemented this CL -
there is a massive range in device capability even on just the Android
platform.
However, I do think it is reasonable for V8 to default to reasonable sizes
based on the devices available memory, rather than hard coding
device-independent defaults which only really work well for a single device
(e.g., a Desktop with 2GB of RAM). WDYT?
If we go down the route of the default being based on values known only at
runtime, then we need some kind of memory probing in V8 (maybe still in
somewhere like memory-defaults.cc) in order to calculate these defaults.
On the
other hand, if we only want to change these values if the embedder specifies
memory constraints, then I think it would be best to have some clear entry
point
to initialize V8 where it's explicit to the embedder that they should be
setting
these constraints. This is difficult right now due to the default isolate,
and I
don't think having this in SetResourceConstraints would work since it is
easy to
call this too late (after the heap has been set up), and I don't think many
embedders even call it (Chrome doesn't for the main thread, and only uses
it to
set the stack size for worker threads). Any thoughts on this?
IMHO, the defaults should be values which we expect to work well for the
current
device V8 is running on, and the embedder should only have to configure V8
if
they know they are doing something unusual (e.g., having many independent
isolates or setting stack limits for worker threads).
I'll try and set up a meeting to discuss this face-to-face, since I would
really
like to get a solution which everyone is happy with.
https://codereview.chromium.org/24269003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.