Use same blob format for internal and external snapshots.

Use same blob format for internal and external snapshots.

R=vogelheim@chromium.org

[vogelheim1, 2014-12-09 13:26:50]

vogelheim@google.com changed reviewers:
+ vogelheim@google.com

[vogelheim1, 2014-12-09 13:26:50]

lgtm

I'm not convinced of the "GYPFLAGS += .." in the Makefile, please have another
look at it. All else looks good.

https://codereview.chromium.org/787033002/diff/1/Makefile
File Makefile (right):

https://codereview.chromium.org/787033002/diff/1/Makefile#newcode42
Makefile:42: GYPFLAGS += -Dv8_use_external_startup_data=1
This looks like a rather surprising difference between Makefile and gyp or GN.
Not sure that's a good idea....

https://codereview.chromium.org/787033002/diff/1/src/mksnapshot.cc
File src/mksnapshot.cc (right):

https://codereview.chromium.org/787033002/diff/1/src/mksnapshot.cc#newcode54
src/mksnapshot.cc:54: MaybeWriteStartupBlob(blob);
If we have one format for both, why would we need both the raw file and the
startup blob? Aren't they identical now?

(If so, I think the raw_file member, command line option, etc. can all go.)

https://codereview.chromium.org/787033002/diff/1/src/mksnapshot.cc#newcode68
src/mksnapshot.cc:68: void MaybeWriteRawFile(const i::Vector<const i::byte>&
blob) const {
The two MaybeWrite* methods are identical except for naming. We probably only
need one method, even if we write both files.

https://codereview.chromium.org/787033002/diff/1/src/natives-external.cc
File src/natives-external.cc (right):

https://codereview.chromium.org/787033002/diff/1/src/natives-external.cc#newc...
src/natives-external.cc:15: #endif  // V8_USE_EXTERNAL_STARTUP_DATA
Good idea, thanks!

[vogelheim1, 2014-12-09 13:27:45]

vogelheim@google.com changed reviewers:
- vogelheim@google.com

[Yang, 2014-12-09 14:40:37]

https://codereview.chromium.org/787033002/diff/1/Makefile
File Makefile (right):

https://codereview.chromium.org/787033002/diff/1/Makefile#newcode42
Makefile:42: GYPFLAGS += -Dv8_use_external_startup_data=1
On 2014/12/09 13:26:50, vogelheim1 wrote:
> This looks like a rather surprising difference between Makefile and gyp or GN.
> Not sure that's a good idea....

Thanks for catching this. I added this line for testing only. I changed this now
so that we can use snapshot=external as make option.

https://codereview.chromium.org/787033002/diff/1/src/mksnapshot.cc
File src/mksnapshot.cc (right):

https://codereview.chromium.org/787033002/diff/1/src/mksnapshot.cc#newcode54
src/mksnapshot.cc:54: MaybeWriteStartupBlob(blob);
On 2014/12/09 13:26:50, vogelheim1 wrote:
> If we have one format for both, why would we need both the raw file and the
> startup blob? Aren't they identical now?
> 
> (If so, I think the raw_file member, command line option, etc. can all go.)

Good point. I removed the --raw-file option altogether.

https://codereview.chromium.org/787033002/diff/1/src/mksnapshot.cc#newcode68
src/mksnapshot.cc:68: void MaybeWriteRawFile(const i::Vector<const i::byte>&
blob) const {
On 2014/12/09 13:26:50, vogelheim1 wrote:
> The two MaybeWrite* methods are identical except for naming. We probably only
> need one method, even if we write both files.

Done.

[Yang, 2014-12-09 14:40:41]

The CQ bit was checked by yangguo@chromium.org

[commit-bot: I haz the power, 2014-12-09 14:41:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/787033002/20001

[commit-bot: I haz the power, 2014-12-09 15:12:19]

Committed patchset #2 (id:20001)

[Michael Achenbach, 2014-12-09 16:18:30]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/792563002/ by machenbach@chromium.org.

The reason for reverting is: [sheriff] breaks
http://build.chromium.org/p/client.v8/builders/V8-Blink%20Linux%2064%20%28dbg....