This new patch moves everything into MemoryAllocator as you suggested Slava.

Thanks!


http://codereview.chromium.org/7352007/diff/1/src/platform-linux.cc
File src/platform-linux.cc (right):

http://codereview.chromium.org/7352007/diff/1/src/platform-linux.cc#newcode373
src/platform-linux.cc:373: void* mbase = mmap(NULL, msize +
Page::kPageSize, prot,
On 2011/07/12 23:23:29, Vyacheslav Egorov wrote:
V8 uses "one argument per line" convention.

Done.

http://codereview.chromium.org/7352007/diff/1/src/platform-linux.cc#newcode382
src/platform-linux.cc:382: mbase += Page::kPageSize;
On 2011/07/12 23:23:29, Vyacheslav Egorov wrote:
I am a bit concerned that we are spreading (and duplicating) guard
page
management across platform-* files instead of managing it uniformly in
the
MemoryAllocator which seems more appropriate place to me (OS::* should
be just a
low-level OS interface).

Done.

http://codereview.chromium.org/7352007/diff/1/src/platform-win32.cc
File src/platform-win32.cc (right):

http://codereview.chromium.org/7352007/diff/1/src/platform-win32.cc#newcode916
src/platform-win32.cc:916: msize + Page::kPageSize,
On 2011/07/12 23:23:29, Vyacheslav Egorov wrote:
This looks slightly suspicious to me.

MemoryAllocator knows about 64k allocation granularity on Windows and
allocates
space for 8 8k Pages per call to OS::Allocate.

Here you basically ask for 8*8k + 8k which will waste 7*8k of VAS
which
MemoryAllocator tried to avoid.

Done.

http://codereview.chromium.org/7352007/

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

Reply via email to