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());
On 2010/08/06 07:51:13, Søren Gjesse wrote:
Long line.
Done.
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")
On 2010/08/06 07:51:13, Søren Gjesse wrote:
max rwx size -> max size of executable memory
Done.
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;
On 2010/08/06 07:51:13, Søren Gjesse wrote:
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.
Done.
http://codereview.chromium.org/3030048/diff/33003/19004#newcode89
src/heap.cc:89: int Heap::max_executable_size_ = 4 * 16MB + 1*GB;
On 2010/08/06 07:51:13, Søren Gjesse wrote:
Ditto (just move it to after the #endif).
Done.
http://codereview.chromium.org/3030048/diff/33003/19004#newcode95
src/heap.cc:95: int Heap::max_executable_size_ = 4 * 8*MB + 512*MB;
On 2010/08/06 07:51:13, Søren Gjesse wrote:
Ditto.
Done.
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;
On 2010/08/06 07:51:13, Søren Gjesse wrote:
Maybe add ASSERT_GE(capacity_, capacity_executable_);
Done.
http://codereview.chromium.org/3030048/diff/33003/19007#newcode355
src/spaces.cc:355: if (size_executable_ + static_cast<size_t>(requested)
capacity_executable_) {
On 2010/08/06 07:51:13, Søren Gjesse wrote:
Long line.
Done.
http://codereview.chromium.org/3030048/diff/33003/19008
File src/spaces.h (right):
http://codereview.chromium.org/3030048/diff/33003/19008#newcode270
src/spaces.h:270: };
Good catch, I guess this has the same effect as what I suggested for
this: http://code.google.com/p/v8/issues/detail?id=810
However, I think that the modification to the LargeObjectChunk::size()
and set_size() are still relevant. I will include the change in the
next patch set, please have a look at it.
I ended up just setting kAllocationWatermarkOffsetShift to 4 instead. I
gave the change you suggested a shot, but it created a lot of problems
because PageFlag's are currently used as bitmasks. All references to
each PageFlag would have to be checked and updated to make this work.
Its all over spaces-inl.h Ex.
flags_ = (flags_ & ~WATERMARK_INVALIDATED) |
watermark_invalidated_mark_;
On 2010/08/06 11:40:43, Vyacheslav Egorov wrote:
There is a subtle (implicit) dependency here:
kAllocationWatermarkOffsetShift
depends on number of flags.
Refactoring suggestion: change enum values from bitmasks to bit
positions
[modify GetPageFlag/SetPageFlag to use shift]
enum PageFlag {
IS_NORMAL_PAGE = 0,
WAS_IN_USE_BEFORE_MC,
WATERMARK_INVALIDATED,
IS_EXECUTABLE,
kNumberOfPageFlags
};
static const int kAllocationWatermarkOffsetShift = kNumberOfPageFlags;
http://codereview.chromium.org/3030048/diff/33003/19008#newcode641
src/spaces.h:641: // Maximum space size in bytes.
On 2010/08/06 07:51:13, Søren Gjesse wrote:
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.
Done.
http://codereview.chromium.org/3030048/show
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
