Allocate all executable code within a 2 GB code range.

Allocate all executable code within a 2 GB code range.

Committed: http://code.google.com/p/v8/source/detail?r=3004

[William Hesse, 2009-09-29 14:52:07]

Will add a C test that exercises this, and test on all platforms with both 2GB
and smaller ranges.  A 5MB range fails on two tests, after much allocation and
deallocation.

[Mads Ager (chromium), 2009-09-30 10:01:43]

LGTM

http://codereview.chromium.org/244022/diff/1001/2004
File src/spaces.cc (right):

http://codereview.chromium.org/244022/diff/1001/2004#newcode170
Line 170: FreeBlock entire_range =
{static_cast<Address>(code_range_->address()),
I would prefer if FreeBlock was a class with a constructor.

http://codereview.chromium.org/244022/diff/1001/2004#newcode232
Line 232: // Finally, crash the system.
I would remove this line.  The call does not always end up crashing the system.
:)

http://codereview.chromium.org/244022/diff/1001/2004#newcode257
Line 257: FreeBlock free_item = {static_cast<Address>(address), length};
I would prefer that FreeBlock has a constructor.

http://codereview.chromium.org/244022/diff/1001/2003
File src/spaces.h (right):

http://codereview.chromium.org/244022/diff/1001/2003#newcode323
Line 323: //  manages a range of virtual memory.
Remove extra space at start of line.

http://codereview.chromium.org/244022/diff/1001/2003#newcode336
Line 336: static inline  bool contains(void* address);
Remove extra space before bool.

http://codereview.chromium.org/244022/diff/5002/5007
File test/cctest/test-alloc.cc (right):

http://codereview.chromium.org/244022/diff/5002/5007#newcode181
Line 181: void* base = CodeRange::AllocateRawMemory(requested_size,
&allocated_size);
Line too long?

[Erik Corry, 2009-09-30 10:56:42]

http://codereview.chromium.org/244022/diff/5002/5006
File src/spaces.cc (right):

http://codereview.chromium.org/244022/diff/5002/5006#newcode188
Line 188: for (current_allocation_block_index_++;
Always incrementing this means we sort and merge more often than necessary, no?

[Erik Corry, 2009-09-30 11:03:21]

Ah, I see now the answer to my question was "no".

LGTM

[Kevin Millikin (Chromium), 2009-09-30 11:04:18]

Drive by.

http://codereview.chromium.org/244022/diff/5002/5004
File src/heap.cc (right):

http://codereview.chromium.org/244022/diff/5002/5004#newcode1929
Line 1929: ASSERT(!CodeRange::exists() || CodeRange::contains(code));
Please do not pass a Code* to a function expecting Address or void*. 
CodeRange::contains(code->address()).

http://codereview.chromium.org/244022/diff/5002/5004#newcode1974
Line 1974: ASSERT(!CodeRange::exists() || CodeRange::contains(code));
Same.

http://codereview.chromium.org/244022/diff/5002/5005
File src/spaces.h (right):

http://codereview.chromium.org/244022/diff/5002/5005#newcode336
Line 336: static inline  bool contains(void* address);
Consider making address an Address rather than void* (and casting it at the one
place you need to).

Get rid of the extra space between inline and bool.

[William Hesse, 2009-10-01 07:57:27]

http://codereview.chromium.org/244022/diff/1001/2004
File src/spaces.cc (right):

http://codereview.chromium.org/244022/diff/1001/2004#newcode170
Line 170: FreeBlock entire_range =
{static_cast<Address>(code_range_->address()),
On 2009/09/30 10:01:43, Mads Ager wrote:
> I would prefer if FreeBlock was a class with a constructor.

Done.

http://codereview.chromium.org/244022/diff/1001/2004#newcode232
Line 232: // Finally, crash the system.
On 2009/09/30 10:01:43, Mads Ager wrote:
> I would remove this line.  The call does not always end up crashing the
system.
> :)

Done.

http://codereview.chromium.org/244022/diff/1001/2004#newcode257
Line 257: FreeBlock free_item = {static_cast<Address>(address), length};
On 2009/09/30 10:01:43, Mads Ager wrote:
> I would prefer that FreeBlock has a constructor.

Done.

http://codereview.chromium.org/244022/diff/1001/2003
File src/spaces.h (right):

http://codereview.chromium.org/244022/diff/1001/2003#newcode323
Line 323: //  manages a range of virtual memory.
On 2009/09/30 10:01:43, Mads Ager wrote:
> Remove extra space at start of line.

Done.

http://codereview.chromium.org/244022/diff/1001/2003#newcode336
Line 336: static inline  bool contains(void* address);
On 2009/09/30 10:01:43, Mads Ager wrote:
> Remove extra space before bool.

Done.