Introduce Heap::SelectSpace helper for allocations.

Introduce Heap::SelectSpace helper for allocations.

R=hpayer@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=16894

[Hannes Payer (out of office), 2013-08-20 09:49:19]

I left some comments how we could clean up and unify the code even further. I
stopped after line 5572. WDYT?

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc#newcode2844
src/heap.cc:2844: AllocationSpace space = (pretenure == TENURED) ?
OLD_DATA_SPACE : NEW_SPACE;
You can also use the select function here.

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc#newcode4044
src/heap.cc:4044: AllocationSpace space = (pretenure == TENURED) ?
OLD_DATA_SPACE : NEW_SPACE;
You can also use the select function here.

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc#newcode4144
src/heap.cc:4144: MaybeObject* maybe_result;
We could also clean up the code allocation with a proper select function then
just calling space->AllocateRaw. lo_space could implement a AllocateRaw(size)
wrapper for AllocateRaw(size, EXECUTABLE);

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc#newcode4357
src/heap.cc:4357: { MaybeObject* maybe_result =
We could do a proper space selection here as well. arguments_object_size could
become really big...?

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc#newcode4873
src/heap.cc:4873: if (always_allocate()) {
You could also use the select function here... but object_size should be never
larger than the large object allocation limit. Can we add an assert?

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc#newcode4951
src/heap.cc:4951: if (always_allocate()) {
Likewise.

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc#newcode5414
src/heap.cc:5414: AllocateRaw(size, OLD_DATA_SPACE, OLD_DATA_SPACE);
For cases like that we could add an AllocateRaw(size, space) function which is a
wrapper for AllocateRaw(size, space1, space2) if space1 == space2.

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc#newcode5439
src/heap.cc:5439: return size <= Page::kMaxNonCodeHeapObjectSize
Can we merge this function with AllocateRawFixedArray(int length, PretenureFlag
pretenure)?

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc#newcode5576
src/heap.cc:5576: AllocateRaw(size, OLD_DATA_SPACE, OLD_DATA_SPACE);
here we could use the AllocateRaw(size, space) wrapper

https://codereview.chromium.org/22925004/diff/3001/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/22925004/diff/3001/src/heap.h#newcode2084
src/heap.h:2084: // size and the given pretenuring decision.
... and the given preferred old space if old data space or old pointer space is
selected ...

[Michael Starzinger, 2013-09-23 11:35:04]

Addressed some comments after offline discussion. PTAL.

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc#newcode2844
src/heap.cc:2844: AllocationSpace space = (pretenure == TENURED) ?
OLD_DATA_SPACE : NEW_SPACE;
On 2013/08/20 09:49:19, Hannes Payer wrote:
> You can also use the select function here.

Done.

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc#newcode4044
src/heap.cc:4044: AllocationSpace space = (pretenure == TENURED) ?
OLD_DATA_SPACE : NEW_SPACE;
On 2013/08/20 09:49:19, Hannes Payer wrote:
> You can also use the select function here.

Done.

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc#newcode4144
src/heap.cc:4144: MaybeObject* maybe_result;
On 2013/08/20 09:49:19, Hannes Payer wrote:
> We could also clean up the code allocation with a proper select function then
> just calling space->AllocateRaw. lo_space could implement a AllocateRaw(size)
> wrapper for AllocateRaw(size, EXECUTABLE);

As discussed offline: Let's do this in a separate CL.

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc#newcode4357
src/heap.cc:4357: { MaybeObject* maybe_result =
On 2013/08/20 09:49:19, Hannes Payer wrote:
> We could do a proper space selection here as well. arguments_object_size could
> become really big...?

As discussed offline: Let's do this in a separate CL.

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc#newcode5414
src/heap.cc:5414: AllocateRaw(size, OLD_DATA_SPACE, OLD_DATA_SPACE);
On 2013/08/20 09:49:19, Hannes Payer wrote:
> For cases like that we could add an AllocateRaw(size, space) function which is
a
> wrapper for AllocateRaw(size, space1, space2) if space1 == space2.

As discussed offline: Let's do this in a separate CL.

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc#newcode5439
src/heap.cc:5439: return size <= Page::kMaxNonCodeHeapObjectSize
On 2013/08/20 09:49:19, Hannes Payer wrote:
> Can we merge this function with AllocateRawFixedArray(int length,
PretenureFlag
> pretenure)?

As discussed offline: Let's do this in a separate CL.

https://codereview.chromium.org/22925004/diff/3001/src/heap.cc#newcode5576
src/heap.cc:5576: AllocateRaw(size, OLD_DATA_SPACE, OLD_DATA_SPACE);
On 2013/08/20 09:49:19, Hannes Payer wrote:
> here we could use the AllocateRaw(size, space) wrapper

As discussed offline: Let's do this in a separate CL.

https://codereview.chromium.org/22925004/diff/3001/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/22925004/diff/3001/src/heap.h#newcode2084
src/heap.h:2084: // size and the given pretenuring decision.
On 2013/08/20 09:49:19, Hannes Payer wrote:
> ... and the given preferred old space if old data space or old pointer space
is
> selected ...

Done.

[Hannes Payer (out of office), 2013-09-23 12:00:01]

LGTM, one comment comma nit

https://codereview.chromium.org/22925004/diff/8001/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/22925004/diff/8001/src/heap.h#newcode2084
src/heap.h:2084: // size, pretenuring decision and preferred old-space.
comma missing: pretenuring decision, and

[Michael Starzinger, 2013-09-23 13:40:03]

Thanks for the review. Rebasing. Landing.

https://codereview.chromium.org/22925004/diff/8001/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/22925004/diff/8001/src/heap.h#newcode2084
src/heap.h:2084: // size, pretenuring decision and preferred old-space.
On 2013/09/23 12:00:01, Hannes Payer wrote:
> comma missing: pretenuring decision, and

Done.

[Michael Starzinger, 2013-09-23 14:26:49]

Manually closed after committing and script crashed.