Added Kevin as reviewer.

Kevin, could you please review this as well, and especially verify that
the use of the constants kSpaceTagSize and kSpaceTagMask are as I think,
and that adding the additional space does not limit the upper bound on
the old space.

Also the max size of the global property cell space is (by accident) set
to kMaxMapSpaceSize (when allocated in heap.cc) - 8MB. Any better
suggestion? Just set it to old_generation_size_ (512MB)?

New snapshot uploaded.


http://codereview.chromium.org/151152/diff/1017/13
File src/heap.cc (right):

http://codereview.chromium.org/151152/diff/1017/13#newcode89
Line 89: int Heap::old_generation_size_ = 512*MB;
On 2009/07/01 15:33:54, Kasper Lund wrote:
> Do you need to change this too?

I don't think so (see other comment regarding kSpaceTagSize).

http://codereview.chromium.org/151152/diff/1017/19
File src/serialize.cc (right):

http://codereview.chromium.org/151152/diff/1017/19#newcode48
Line 48: // - MAP and OLD spaces: 15 bits of page number, 11 bits of
word offset in page
On 2009/07/01 15:33:54, Kasper Lund wrote:
> This limits the old space size to 256M, right? I wonder if that's
okay...

This limitation is only on the snapshot. The constants kSpaceTagSize and
kSpaceTagMask (from globals.h) are used for encoding pointers in the
snapshot, and for signalling the requested number of bytes when
allocation fails. As far as I can see this should not limit the size of
old space. Added Kevin as reviewer to make sure.

http://codereview.chromium.org/151152/diff/1017/18
File src/spaces.cc (right):

http://codereview.chromium.org/151152/diff/1017/18#newcode1990
Line 1990: // FixedSizeSpace implementation
On 2009/07/01 15:33:54, Kasper Lund wrote:
> This is a weird name. The space grows and shrinks.

Changed it to PagedSpaceForFixedSizedObjects - don't know whether thats
is better.

http://codereview.chromium.org/151152/diff/1017/16
File src/spaces.h (right):

http://codereview.chromium.org/151152/diff/1017/16#newcode1501
Line 1501: explicit FixedSizeSpace(int max_capacity, AllocationSpace id,
On 2009/07/01 15:33:54, Kasper Lund wrote:
> Doesn't have to be explicit.

Done.

http://codereview.chromium.org/151152

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

Reply via email to