Refactoring of snapshots. This simplifies and improves

Refactoring of snapshots.  This simplifies and improves
the speed of deserializing code.  The current startup
time improvement for V8 is around 6%, but code deserialization
is speeded up disproportionately, and we will soon have more
code in the snapshot.
* Removed support for deserializing into large object space.
  The regular pages are 1Mbyte now and that is plenty.  This
  is a big simplification.
* Instead of reserving space for the snapshot we actually
  allocate it now.  This removes some special casing from
  the memory management and simplifies deserialization since
  we are just bumping a pointer rather than calling the
  normal allocation routines during deserialization.
* Record in the snapshot how much we need to boot up and
  allocate it instead of just assuming that allocations in
  a new VM will always be linear.
* In the snapshot we always address an object as a negative
  offset from the current allocation point.  We used to
  sometimes address from the start of the deserialized data,
  but this is less useful now that we have good support for
  roots and repetitions in the deserialization data.
* Code objects were previously deserialized (like other
  objects) by alternating raw data (deserialized with memcpy)
  and pointers (to external references, other objects, etc.).
  Now we deserialize code objects with a single memcpy,
  followed by a series of skips and pointers that partially
  overwrite the code we memcopied out of the snapshot.
  The skips are sometimes merged into the following
  instruction in the deserialization data to reduce dispatch
  time.
* Integers in the snapshot were stored in a variable length
  format that gives a compact representation for small positive
  integers.  This is still the case, but the new encoding can
  be decoded without branches or conditional instructions,
  which is faster on a modern CPU.
Committed: https://code.google.com/p/v8/source/detail?r=12505

[Yang, 2012-09-13 08:47:58]

LGTM with some comments.

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

http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode611
src/serialize.cc:611: isolate_->heap()->ReserveSpace(&reservations_[0],
&high_water_[0]);
why dereference and re-reference, and not simply:

ReserveSpace(reservations_, high_water_)

unless you want to make sure it is obvious that we are dealing with arrays?

http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode644
src/serialize.cc:644: isolate_->heap()->ReserveSpace(&reservations_[0],
&high_water_[0]);
Ditto.

http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode750
src/serialize.cc:750: current) + skip);                                         
    \
It seems easier readable to use Address instead of intptr_t.

http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode762
src/serialize.cc:762: reinterpret_cast<intptr_t>(current) + skip);              
    \
Ditto.

http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode1000
src/serialize.cc:1000: ASSERT(integer <= 0x3fffff);
I'd prefer integer < (1 << 22)

http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode1065
src/serialize.cc:1065: sink_->Put(kRawData + kPointerSize / 4, "Smi");
The 4 in here obviously refers to the factor 4 in COMMON_RAW_LENGTH. Can we
somehow make a constant out of this?

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

http://codereview.chromium.org/10918067/diff/1/src/serialize.h#newcode177
src/serialize.h:177: answer = *reinterpret_cast<const int32_t*>(data_ +
position_);
Since GetInt() uses this: we assume the least significant two bits to tell how
many bytes are part of the integer, so the byte order matters. This would only
work on little endian systems, right? The #else block seems to work for big
endian as well. I don't think we have any big endian system that reads
unaligned, but an assertion won't hurt?

http://codereview.chromium.org/10918067/diff/1/src/serialize.h#newcode182
src/serialize.h:182: answer |= data_[position_ + 3] << 24;
In PutInt we only take integers that can be encoded in 3 bytes. Why even fetch a
4th byte?

http://codereview.chromium.org/10918067/diff/1/src/serialize.h#newcode235
src/serialize.h:235: f(31, 124)
if the second arg is always 4x the first arg, why not make that part of f? Or at
least add a few STATIC_CHECK in some of the f.

http://codereview.chromium.org/10918067/diff/1/src/serialize.h#newcode546
src/serialize.h:546: code_object_ = o->IsCode();
I guess this could also be put into the initialization list.

http://codereview.chromium.org/10918067/diff/1/src/snapshot-common.cc
File src/snapshot-common.cc (right):

http://codereview.chromium.org/10918067/diff/1/src/snapshot-common.cc#newcode92
src/snapshot-common.cc:92: bool success = V8::Initialize(&deserializer);
Is there any plans to include a SnapshotByteSink that writes to file in
mksnapshot so that it can be read here?

[Erik Corry, 2012-09-13 12:13:35]

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

http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode611
src/serialize.cc:611: isolate_->heap()->ReserveSpace(&reservations_[0],
&high_water_[0]);
On 2012/09/13 08:47:59, Yang wrote:
> why dereference and re-reference, and not simply:
> 
> ReserveSpace(reservations_, high_water_)
> 
> unless you want to make sure it is obvious that we are dealing with arrays?

Done.

http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode644
src/serialize.cc:644: isolate_->heap()->ReserveSpace(&reservations_[0],
&high_water_[0]);
On 2012/09/13 08:47:59, Yang wrote:
> Ditto.

Ditto.

http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode750
src/serialize.cc:750: current) + skip);                                         
    \
On 2012/09/13 08:47:59, Yang wrote:
> It seems easier readable to use Address instead of intptr_t.

Done.

http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode762
src/serialize.cc:762: reinterpret_cast<intptr_t>(current) + skip);              
    \
On 2012/09/13 08:47:59, Yang wrote:
> Ditto.

Ditto

http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode1000
src/serialize.cc:1000: ASSERT(integer <= 0x3fffff);
On 2012/09/13 08:47:59, Yang wrote:
> I'd prefer integer < (1 << 22)

Done.

http://codereview.chromium.org/10918067/diff/1/src/serialize.cc#newcode1065
src/serialize.cc:1065: sink_->Put(kRawData + kPointerSize / 4, "Smi");
On 2012/09/13 08:47:59, Yang wrote:
> The 4 in here obviously refers to the factor 4 in COMMON_RAW_LENGTH. Can we
> somehow make a constant out of this?

Now the 4 is gone.

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

http://codereview.chromium.org/10918067/diff/1/src/serialize.h#newcode177
src/serialize.h:177: answer = *reinterpret_cast<const int32_t*>(data_ +
position_);
On 2012/09/13 08:47:59, Yang wrote:
> Since GetInt() uses this: we assume the least significant two bits to tell how
> many bytes are part of the integer, so the byte order matters. This would only
> work on little endian systems, right? The #else block seems to work for big
> endian as well. I don't think we have any big endian system that reads
> unaligned, but an assertion won't hurt?

Fixed to go into the slow case for big endian.

http://codereview.chromium.org/10918067/diff/1/src/serialize.h#newcode182
src/serialize.h:182: answer |= data_[position_ + 3] << 24;
On 2012/09/13 08:47:59, Yang wrote:
> In PutInt we only take integers that can be encoded in 3 bytes. Why even fetch
a
> 4th byte?

Otherwise it will do something different depending on the ifdefs.  I don't want
to rename it to
GetUnalignedIntOrGetUnaligned24BitValueDependingOnSomeObscureIfdefs because then
I would run into problems with the 80 character limit.

http://codereview.chromium.org/10918067/diff/1/src/serialize.h#newcode235
src/serialize.h:235: f(31, 124)
On 2012/09/13 08:47:59, Yang wrote:
> if the second arg is always 4x the first arg, why not make that part of f? Or
at
> least add a few STATIC_CHECK in some of the f.

Made it part of f, and moved this to the .cc file.

http://codereview.chromium.org/10918067/diff/1/src/serialize.h#newcode546
src/serialize.h:546: code_object_ = o->IsCode();
On 2012/09/13 08:47:59, Yang wrote:
> I guess this could also be put into the initialization list.

Done.

http://codereview.chromium.org/10918067/diff/1/src/snapshot-common.cc
File src/snapshot-common.cc (right):

http://codereview.chromium.org/10918067/diff/1/src/snapshot-common.cc#newcode92
src/snapshot-common.cc:92: bool success = V8::Initialize(&deserializer);
On 2012/09/13 08:47:59, Yang wrote:
> Is there any plans to include a SnapshotByteSink that writes to file in
> mksnapshot so that it can be read here?

This is currently only used for tests.

[Erik Corry, 2012-09-13 13:52:57]

I had to make some changes to the way immovable stubs were created, because we
don't want to have large-object objects in the snapshot any more.

[Erik Corry, 2012-09-13 13:54:21]

http://codereview.chromium.org/10918067/diff2/1:19/src/heap.cc
http://codereview.chromium.org/10918067/diff/19/src/code-stubs.cc
http://codereview.chromium.org/10918067/diff/19/test/cctest/test-mark-compact.cc

[Yang, 2012-09-13 14:31:30]

On 2012/09/13 13:54:21, Erik Corry wrote:
> http://codereview.chromium.org/10918067/diff2/1:19/src/heap.cc
> http://codereview.chromium.org/10918067/diff/19/src/code-stubs.cc
>
http://codereview.chromium.org/10918067/diff/19/test/cctest/test-mark-compact.cc

LGTM.