New snapshot framework. Doesn't work on ARM yet (code targets...

New snapshot framework.  Doesn't work on ARM yet (code targets
are different).  Is able to deserialize the whole heap and run
some stuff.  Not available as the primary snapshot system yet.

[Mads Ager (chromium), 2009-10-26 11:14:04]

LGTM with a bunch of nits.

http://codereview.chromium.org/335009/diff/1/20
File src/heap.h (right):

http://codereview.chromium.org/335009/diff/1/20#newcode307
Line 307: static bool linear_allocation() {
Move this one after the other always_allocate_* getter?

http://codereview.chromium.org/335009/diff/1/16
File src/serialize.cc (right):

http://codereview.chromium.org/335009/diff/1/16#newcode1444
Line 1444: GlobalHandles::TearDown();
Shouldn't we be checking that there are no global handles at this point?

http://codereview.chromium.org/335009/diff/1/16#newcode1758
Line 1758: : source_(source),
Four space indent.

http://codereview.chromium.org/335009/diff/1/16#newcode1792
Line 1792: new_page = (old_fullness >> Page::kPageSizeBits) + 1;
current_page + 1?

http://codereview.chromium.org/335009/diff/1/16#newcode1802
Line 1802: Page::kPageAlignmentMask) ==
Funky indentation.  I would do:

ASSERT((reinterpret_cast<intptr_t>(new_object->address()) &
        Page::kPageAlignmentMask) ==
       ((old_fullness & Page::kPageAlignmentMask) +
        Page::kObjectStartOffset));

http://codereview.chromium.org/335009/diff/1/16#newcode1899
Line 1899: // returns a true of the new object was in young space and false
otherwise.
returns true if the new ...

http://codereview.chromium.org/335009/diff/1/16#newcode2017
Line 2017: : sink_(sink),
Four space indent.

http://codereview.chromium.org/335009/diff/1/7
File src/serialize.h (right):

http://codereview.chromium.org/335009/diff/1/7#newcode356
Line 356: SnapshotByteSource(const byte* array, int length)
Please add spaces between the methods in this class.

http://codereview.chromium.org/335009/diff/1/7#newcode377
Line 377: return accumulator;
Put an UNREACHABLE() in front of this return as documentation?

Is all of this getting inlined?  Maybe have the fast case here and the rest in
the implementation file?

http://codereview.chromium.org/335009/diff/1/7#newcode405
Line 405: static const int kSmiBias = 16;
Could you add a comment about the bias?

http://codereview.chromium.org/335009/diff/1/7#newcode422
Line 422: // Create a deserializer. The snapshot is held in str and has size
len.
Create a deserializer from a byte source?  The rest of the comment looks
out-dated.

http://codereview.chromium.org/335009/diff/1/7#newcode436
Line 436: virtual void VisitExternalReferences(Address* start, Address* end) {
I would add space before and after the methods that do not fit on a single line
for readability.

http://codereview.chromium.org/335009/diff/1/7#newcode477
Line 477: const int max_shift = ((kPointerSize * kBitsPerByte) / 7) * 7;
Move to implementation file?

http://codereview.chromium.org/335009/diff/1/7#newcode495
Line 495: void Finalize(int* y) {}
Add a comment? What does Finalize do? What is y?

http://codereview.chromium.org/335009/diff/1/7#newcode504
Line 504: TAGGED_REPRESENTATION,      // Deserialize as a tagged object
reference.
How about just "Tagged object reference."?

http://codereview.chromium.org/335009/diff/1/7#newcode505
Line 505: CODE_TARGET_REPRESENTATION  // Deserialize a reference to first
instruction.
How about just "Reference to first instruction in code target."?

http://codereview.chromium.org/335009/diff/1/15
File src/spaces.cc (right):

http://codereview.chromium.org/335009/diff/1/15#newcode1834
Line 1834: // There is no next page in this space.  Try free list allocation.
Update comment.

http://codereview.chromium.org/335009/diff/1/15#newcode2238
Line 2238: // There is no next page in this space.  Try free list allocation.
Update comment.

http://codereview.chromium.org/335009/diff/1/2
File test/cctest/test-serialize.cc (right):

http://codereview.chromium.org/335009/diff/1/2#newcode190
Line 190: // We have to create one context.  One reason for this is that the
One reason for this is so that the builtins ...

[Erik Corry, 2009-10-27 09:49:57]

http://codereview.chromium.org/335009/diff/1/16
File src/serialize.cc (right):

http://codereview.chromium.org/335009/diff/1/16#newcode1444
Line 1444: GlobalHandles::TearDown();
On 2009/10/26 11:14:05, Mads Ager wrote:
> Shouldn't we be checking that there are no global handles at this point?

No.  Even if there are global handles we want to discard them.  Comment
clarified.