https://codereview.chromium.org/24269003/diff/1/src/heap.cc
File src/heap.cc (right):
https://codereview.chromium.org/24269003/diff/1/src/heap.cc#newcode75
src/heap.cc:75: #if defined(ANDROID) || V8_TARGET_ARCH_MIPS
On 2013/09/19 14:50:57, Hannes Payer wrote:
I never liked the ifdef here. Now it's time to get rid of it while you
are
there. It would be great if the embedder (chrome, d8) would set the
right
values. It also shouldn't be a binary decision. We have beefier and
weaker
phones. Their could be arbitrary size values, depending on the device.
WDYT?
I don't like this ifdef particularly either, but I don't think forcing
the embedder to do this is the right way to do this. The embedder
doesn't necessarily know what a good set of defaults are for different
devices, and it would just end up moving a single set of per-arch
constants to sets of constants in every single embedder, which are then
just going to diverge and cause unexpected performance differences as
they diverge. If the embedder really knows what it wants these to be it
can still override them using SetResources, but I think it's good to
have sensible defaults in here.
https://codereview.chromium.org/24269003/diff/1/src/heap.cc#newcode93
src/heap.cc:93: reserved_semispace_size_(DEFAULT_SEMISPACE_MAX_SIZE),
On 2013/09/19 14:50:57, Hannes Payer wrote:
Here we could set the values to proper default values, like our
default values
for non-mobile devices now.
I'm not sure what you mean here?
https://codereview.chromium.org/24269003/diff/1/src/heap.cc#newcode6688
src/heap.cc:6688: max_executable_size_ = DEFAULT_EXECUTABLE_MAX_SIZE /
2;
On 2013/09/19 14:50:57, Hannes Payer wrote:
The embedder would just set the right values, no memory_constrained
guard
needed.
The embedder doesn't need to set these values (if they are 0 then no
changes are made in this method). This branch is simply to set the
initial set of default values correctly based on the flag (see the
comment above - this should really be alongside the defaults set in the
constructor, but can't due to the fact the flags are set late).
https://codereview.chromium.org/24269003/diff/1/src/heap.h
File src/heap.h (right):
https://codereview.chromium.org/24269003/diff/1/src/heap.h#newcode2342
src/heap.h:2342: bool limits_changed_from_defaults_;
On 2013/09/19 14:50:57, Hannes Payer wrote:
Why do we need that? If the embedder decides to overwrite multiple
times that
should be ok? Or is there a problem?
This is not to prevent the embedder overwriting things multiple times
(that's fine, and still works after this CL), it is to prevent the
is_memory_constrained flag resetting the default values if the embedder
has already changed the values from their defaults (i.e., if the
embedder specifically sets limits, treat the preferentially to any
changes which would be incurred by setting the is_memory_constrained
flag).
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.
