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 15:01:34, rmcilroy wrote:
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.
Well, what I mean is that we should have these ifdefs in chromium and in
d8. Right now we have Android/MIPS or others, X64 or others, and on top
of that resource constrained. We should keep X64 ifdef since this one is
a true global mode for v8. But apart from that we have 3 classes of
devices right now:
- large memory device
- regular memory device
- low memory device
It would be actually awesome if the embedder would set arbitrary memory
sizes, based on the system state. But this is future work.
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 15:01:34, rmcilroy wrote:
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?
What I meant is to set the values to
reserved_semispace_size_(8 * Max(LUMP_OF_MEMORY, Page::kPageSize)),
max_semispace_size_(8 * Max(LUMP_OF_MEMORY, Page::kPageSize)),
initial_semispace_size_(Page::kPageSize),
max_old_generation_size_(700ul * LUMP_OF_MEMORY),
max_executable_size_(256l * LUMP_OF_MEMORY),
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 15:01:34, rmcilroy wrote:
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).
But SetResourceConstraints could just set it to the right values. That
would work, right?
https://codereview.chromium.org/24269003/
--
--
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.
