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.

Reply via email to