Teach gen_snapshot how to create script snapshots if given a core snapshot to create them against.

Teach gen_snapshot how to create script snapshots if given a core snapshot to create them against.

Require an explicit --snapshot_kind option instead of inferring the desired snapshot kind from the combination of output file options.

R=asiva@google.com

Committed: https://github.com/dart-lang/sdk/commit/b6a9be6e393286119800c2aea6f0081bed094313

[rmacnak, 2017-02-14 00:57:41]

Description was changed from

==========
Teach gen_snapshot how to create script snapshots if given a core snapshot to
create them against.
==========

to

==========
Teach gen_snapshot how to create script snapshots if given a core snapshot to
create them against.

Require an explicit --snapshot_kind option instead of inferring the desired
snapshot kind from the combination of output file options.
==========

[rmacnak, 2017-02-14 01:05:00]

rmacnak@google.com changed reviewers:
+ asiva@google.com

[rmacnak, 2017-02-14 01:05:01]

For example,

+ ./out/DebugX64/gen_snapshot \
  --snapshot_kind=script \
  --vm_snapshot_data=out/DebugX64/gen/runtime/bin/vm_isolate_snapshot.bin \
  --isolate_snapshot_data=out/DebugX64/gen/runtime/bin/isolate_snapshot.bin \
  --script_snapshot=hello.scriptsnap \ 
  /usr/local/google/home/rmacnak/hello.dart
+ ./out/DebugX64/dart hello.scriptsnap
Hello, world!

[siva, 2017-02-14 03:56:09]

We should try and move as much of the snapshot related code into
snapshot_utils.[h|cc] files and try to use them from bin/main.cc and
bin/gen_snapshot.cc

https://codereview.chromium.org/2692223002/diff/40001/runtime/bin/gen_snapsho...
File runtime/bin/gen_snapshot.cc (right):

https://codereview.chromium.org/2692223002/diff/40001/runtime/bin/gen_snapsho...
runtime/bin/gen_snapshot.cc:74: };
why not move this to snapshot_utils.h and share it with main?

https://codereview.chromium.org/2692223002/diff/40001/runtime/bin/gen_snapsho...
runtime/bin/gen_snapshot.cc:229: }
Can this also be moved to snapshot_utils.cc and shared with the version that
exists in main.cc

https://codereview.chromium.org/2692223002/diff/40001/runtime/bin/gen_snapsho...
runtime/bin/gen_snapshot.cc:473: }
This function exists in snapshot_utils.cc, can we use that instead of defining
yet another one here?

[rmacnak, 2017-02-14 23:39:39]

I think we need some deeper changes to gen_snapshot.cc to make it look more like
main.cc to effectively share some of this code. Currently we'd need to pass
flags through to control shutdown logic.

https://codereview.chromium.org/2692223002/diff/40001/runtime/bin/gen_snapsho...
File runtime/bin/gen_snapshot.cc (right):

https://codereview.chromium.org/2692223002/diff/40001/runtime/bin/gen_snapsho...
runtime/bin/gen_snapshot.cc:229: }
On 2017/02/14 03:56:09, siva wrote:
> Can this also be moved to snapshot_utils.cc and shared with the version that
> exists in main.cc

They're different: main.cc doesn't generate core snapshots and gen_snapshot.cc
doesn't generate app-jit snapshots.

Also sadly the argument parsers in gen_snapshot and main have different
signatures.

https://codereview.chromium.org/2692223002/diff/40001/runtime/bin/gen_snapsho...
runtime/bin/gen_snapshot.cc:473: }
On 2017/02/14 03:56:09, siva wrote:
> This function exists in snapshot_utils.cc, can we use that instead of defining
> yet another one here?

The shutdown logic is different between gen_snapshot and main. Using
Snapshot::WriteScript here results in hangs on errors.

[siva, 2017-02-15 11:29:32]

lgtm

https://codereview.chromium.org/2692223002/diff/60001/runtime/bin/gen_snapsho...
File runtime/bin/gen_snapshot.cc (right):

https://codereview.chromium.org/2692223002/diff/60001/runtime/bin/gen_snapsho...
runtime/bin/gen_snapshot.cc:75: static SnapshotKind snapshot_kind = kNone;
why not start with a default of kCore, that way the default core snapshot
generator would not require an option to be passed in.

[rmacnak, 2017-02-15 22:26:11]

https://codereview.chromium.org/2692223002/diff/60001/runtime/bin/gen_snapsho...
File runtime/bin/gen_snapshot.cc (right):

https://codereview.chromium.org/2692223002/diff/60001/runtime/bin/gen_snapsho...
runtime/bin/gen_snapshot.cc:75: static SnapshotKind snapshot_kind = kNone;
On 2017/02/15 11:29:32, siva wrote:
> why not start with a default of kCore, that way the default core snapshot
> generator would not require an option to be passed in.

Done.

[rmacnak, 2017-02-15 22:28:41]

Description was changed from

==========
Teach gen_snapshot how to create script snapshots if given a core snapshot to
create them against.

Require an explicit --snapshot_kind option instead of inferring the desired
snapshot kind from the combination of output file options.
==========

to

==========
Teach gen_snapshot how to create script snapshots if given a core snapshot to
create them against.

Require an explicit --snapshot_kind option instead of inferring the desired
snapshot kind from the combination of output file options.

R=asiva@google.com

Committed:
https://github.com/dart-lang/sdk/commit/b6a9be6e393286119800c2aea6f0081bed094313
==========

[rmacnak, 2017-02-15 22:28:42]

Committed patchset #5 (id:80001) manually as
b6a9be6e393286119800c2aea6f0081bed094313 (presubmit successful).