http://codereview.chromium.org/504026/diff/6001/6005
File src/objects.h (right):

http://codereview.chromium.org/504026/diff/6001/6005#newcode919
src/objects.h:919: #ifndef V8_HOST_ARCH_64_BIT
just an idea: maybe instead of STATIC_CHECK declare kMapPageIndexBits as
32 - (kForwardingOffsetBits + kMapPageOffsetBits)?

Up to you.

http://codereview.chromium.org/504026/diff/6001/6005#newcode925
src/objects.h:925: // Bit masks covering the different parts the
encodeding.
nit: encod<de>ing

http://codereview.chromium.org/504026/diff/6001/6004
File src/spaces.h (left):

http://codereview.chromium.org/504026/diff/6001/6004#oldcode1744
src/spaces.h:1744: static const int kMaxMapPageIndex = (1 <<
MapWord::kMapPageIndexBits) - 1;
On 2009/12/16 22:19:41, Søren Gjesse wrote:
> On 2009/12/16 16:26:30, antonm wrote:
> > BTW, do I miss something or this constant should be increased to be
something
> > like (1 << (MapWord::kMapPageIndexBits + kMapAlignmentBits -
> > kObjectAlignmentBits))?
> >
> > If yes, could you merge them to get rid of duplication?

> This is correct, as we need one entry for each possible page of map
space. What
> is wrong however is the calculation of kMaxMapSpaceSize which should
be

>    static const int kMaxMapSpaceSize =
>        (1 << (MapWord::kMapPageIndexBits)) * Page::kPageSize;

> Thanks for spotting this inconsistency.

You're right.

The fact that Chromium didn't crash for dashboard makes me somewhat
uneasy as map space might have grown to the size when pointers are not
encodable (but probably this point wasn't reached).

http://codereview.chromium.org/504026/diff/6001/6004
File src/spaces.h (right):

http://codereview.chromium.org/504026/diff/6001/6004#newcode74
src/spaces.h:74: #define ASSERT_MAP_ALIGNED(address)                \
nit: maybe align \ to \ above?

http://codereview.chromium.org/504026

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

Reply via email to