LGTM with the comments addressed.


http://codereview.chromium.org/3030048/diff/33003/19002
File src/api.cc (right):

http://codereview.chromium.org/3030048/diff/33003/19002#newcode3187
src/api.cc:3187:
heap_statistics->set_total_heap_size_executable(i::Heap::CommittedMemoryExecutable());
Long line.

http://codereview.chromium.org/3030048/diff/33003/19003
File src/flag-definitions.h (right):

http://codereview.chromium.org/3030048/diff/33003/19003#newcode183
src/flag-definitions.h:183: DEFINE_int(max_executable_size, 0, "max rwx
size")
max rwx size -> max size of executable memory

http://codereview.chromium.org/3030048/diff/33003/19004
File src/heap.cc (right):

http://codereview.chromium.org/3030048/diff/33003/19004#newcode83
src/heap.cc:83: int Heap::max_executable_size_ = 4 * 2*MB + 192*MB;
You should just set Heap::max_executable_size_ to the value of
Heap::max_old_generation_size_, as code is never allocated in the young
generation, but in its own part of old generation.

http://codereview.chromium.org/3030048/diff/33003/19004#newcode89
src/heap.cc:89: int Heap::max_executable_size_ = 4 * 16MB + 1*GB;
Ditto (just move it to after the #endif).

http://codereview.chromium.org/3030048/diff/33003/19004#newcode95
src/heap.cc:95: int Heap::max_executable_size_ = 4 * 8*MB + 512*MB;
Ditto.

http://codereview.chromium.org/3030048/diff/33003/19007
File src/spaces.cc (right):

http://codereview.chromium.org/3030048/diff/33003/19007#newcode300
src/spaces.cc:300: size_executable_ = 0;
Maybe add ASSERT_GE(capacity_, capacity_executable_);

http://codereview.chromium.org/3030048/diff/33003/19007#newcode355
src/spaces.cc:355: if (size_executable_ + static_cast<size_t>(requested)
capacity_executable_) {
Long line.

http://codereview.chromium.org/3030048/diff/33003/19008
File src/spaces.h (right):

http://codereview.chromium.org/3030048/diff/33003/19008#newcode641
src/spaces.h:641: // Maximum space size in bytes.
Please update the comments to indicate that capacity_ is the total
capacity, and capacity_executable_ is the maximum part of capacity_
which can be executable.

http://codereview.chromium.org/3030048/show

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to