Snapshots: Add --extra-code flag to mksnapshot which lets you specify a file

Snapshots: Add --extra-code flag to mksnapshot which lets you specify a file
with more JS code that is loaded into the VM before writing the snapshot.  Get
rid of the hard coded limit on the partial snapshot cache size.  This change
disables most of the serializer tests for the snapshot build of the VM: It's
getting too complicated to support both booting from a snapshot and then
creating a new snapshot from the same VM or loading more code with another
snapshot in the same VM.
Committed: http://code.google.com/p/v8/source/detail?r=11871

[ulan, 2012-06-19 09:42:15]

What is the motivation/background for this CL? Is there an open issue I could
read?

http://codereview.chromium.org/10574013/diff/1/src/isolate.cc
File src/isolate.cc (right):

http://codereview.chromium.org/10574013/diff/1/src/isolate.cc#newcode1610
src/isolate.cc:1610: for (int i = 0; i < capacity; i++) {
i < length ?

http://codereview.chromium.org/10574013/diff/1/src/mksnapshot.cc
File src/mksnapshot.cc (right):

http://codereview.chromium.org/10574013/diff/1/src/mksnapshot.cc#newcode314
src/mksnapshot.cc:314: context->Enter();
Move context->Enter(), context->Exit() inside the branch?

http://codereview.chromium.org/10574013/diff/1/src/mksnapshot.cc#newcode335
src/mksnapshot.cc:335: fprintf(stderr, "Failed to read '%s': errno %d\n", name,
errno);
Indentation.

Do we want to exit(1) here too?

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

http://codereview.chromium.org/10574013/diff/1/src/serialize.cc#newcode1188
src/serialize.cc:1188: if (isolate->serialize_partial_snapshot_cache_length() <=
i) {
I don't get this part. If the branch is ever taken, who puts the undefined_value
in cache[i]?

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

http://codereview.chromium.org/10574013/diff/1/src/snapshot-common.cc#newcode63
src/snapshot-common.cc:63: bool Snapshot::WeHaveASnapshotToStartFrom() {
Can we avoid "We" in the name? :)

[Erik Corry, 2012-06-19 10:20:46]

http://codereview.chromium.org/10574013/diff/1/src/isolate.cc
File src/isolate.cc (right):

http://codereview.chromium.org/10574013/diff/1/src/isolate.cc#newcode1610
src/isolate.cc:1610: for (int i = 0; i < capacity; i++) {
On 2012/06/19 09:42:15, ulan wrote:
> i < length ?

Done.

http://codereview.chromium.org/10574013/diff/1/src/mksnapshot.cc
File src/mksnapshot.cc (right):

http://codereview.chromium.org/10574013/diff/1/src/mksnapshot.cc#newcode314
src/mksnapshot.cc:314: context->Enter();
On 2012/06/19 09:42:15, ulan wrote:
> Move context->Enter(), context->Exit() inside the branch?

Done.

http://codereview.chromium.org/10574013/diff/1/src/mksnapshot.cc#newcode335
src/mksnapshot.cc:335: fprintf(stderr, "Failed to read '%s': errno %d\n", name,
errno);
On 2012/06/19 09:42:15, ulan wrote:
> Indentation.
> 
> Do we want to exit(1) here too?

Done.

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

http://codereview.chromium.org/10574013/diff/1/src/serialize.cc#newcode1188
src/serialize.cc:1188: if (isolate->serialize_partial_snapshot_cache_length() <=
i) {
On 2012/06/19 09:42:15, ulan wrote:
> I don't get this part. If the branch is ever taken, who puts the
undefined_value
> in cache[i]?

The VisitPointers call below.

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

http://codereview.chromium.org/10574013/diff/1/src/snapshot-common.cc#newcode63
src/snapshot-common.cc:63: bool Snapshot::WeHaveASnapshotToStartFrom() {
On 2012/06/19 09:42:15, ulan wrote:
> Can we avoid "We" in the name? :)

Done.

[ulan, 2012-06-19 13:13:12]

LGTM