Yes, I'll take a look. On Wed, Jul 1, 2009 at 6:41 PM, <[email protected]> wrote:
> 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 -~----------~----~----~----~------~----~------~--~---
