Reland "Use same blob format for internal and external snapshots."

Review URL: https://codereview.chromium.org/791723004

Cr-Commit-Position: refs/heads/master@{#25741}

BUILD.gn

@@ -358,7 +358,6 @@ source_set("v8_nosnapshot") {
     "$target_gen_dir/libraries.cc",
     "$target_gen_dir/experimental-libraries.cc",
     "src/snapshot-empty.cc",
-    "src/snapshot-common.cc",
   ]
 
   configs -= [ "//build/config/compiler:chromium_code" ]
@@ -380,7 +379,6 @@ source_set("v8_snapshot") {
     "$target_gen_dir/libraries.cc",
     "$target_gen_dir/experimental-libraries.cc",
     "$target_gen_dir/snapshot.cc",
-    "src/snapshot-common.cc",
   ]
 
   configs -= [ "//build/config/compiler:chromium_code" ]
@@ -895,6 +893,7 @@ source_set("v8_base") {
     "src/serialize.h",
     "src/small-pointer-list.h",
     "src/smart-pointers.h",
+    "src/snapshot-common.cc",
     "src/snapshot-source-sink.cc",
     "src/snapshot-source-sink.h",
     "src/snapshot.h",

Makefile

@@ -82,6 +82,9 @@ endif
 ifeq ($(snapshot), off)
   GYPFLAGS += -Dv8_use_snapshot='false'
 endif
+ifeq ($(snapshot), external)
+  GYPFLAGS += -Dv8_use_external_startup_data=1
+endif
 # extrachecks=on/off
 ifeq ($(extrachecks), on)
   GYPFLAGS += -Dv8_enable_extra_checks=1 -Dv8_enable_handle_zapping=1

src/flag-definitions.h

@@ -715,12 +715,6 @@ DEFINE_STRING(testing_serialization_file, "/tmp/serdes",
 DEFINE_STRING(extra_code, NULL,
               "A filename with extra code to be included in"
               " the snapshot (mksnapshot only)")
-DEFINE_STRING(raw_file, NULL,
-              "A file to write the raw snapshot bytes to. "
-              "(mksnapshot only)")
-DEFINE_STRING(raw_context_file, NULL,
-              "A file to write the raw context "
-              "snapshot bytes to. (mksnapshot only)")
 DEFINE_STRING(startup_blob, NULL,
               "Write V8 startup blob file. "
               "(mksnapshot only)")

src/mksnapshot.cc

@@ -24,22 +24,13 @@ class SnapshotWriter {
  public:
   explicit SnapshotWriter(const char* snapshot_file)
       : fp_(GetFileDescriptorOrDie(snapshot_file)),
-        raw_file_(NULL),
-        raw_context_file_(NULL),
         startup_blob_file_(NULL) {}
 
   ~SnapshotWriter() {
     fclose(fp_);
-    if (raw_file_) fclose(raw_file_);
-    if (raw_context_file_) fclose(raw_context_file_);
     if (startup_blob_file_) fclose(startup_blob_file_);
   }
 
-  void SetRawFiles(const char* raw_file, const char* raw_context_file) {
-    raw_file_ = GetFileDescriptorOrDie(raw_file);
-    raw_context_file_ = GetFileDescriptorOrDie(raw_context_file);
-  }
-
   void SetStartupBlobFile(const char* startup_blob_file) {
     if (startup_blob_file != NULL)
       startup_blob_file_ = GetFileDescriptorOrDie(startup_blob_file);
@@ -47,34 +38,29 @@ class SnapshotWriter {
 
   void WriteSnapshot(const i::SnapshotData& sd,
                      const i::SnapshotData& csd) const {
-    WriteSnapshotFile(sd, csd);
-    MaybeWriteStartupBlob(sd, csd);
+    i::SnapshotByteSink sink;
+    sink.PutBlob(sd.RawData(), "startup");
+    sink.PutBlob(csd.RawData(), "context");
+    const i::Vector<const i::byte>& blob = sink.data().ToConstVector();
+
+    WriteSnapshotFile(blob);
+    MaybeWriteStartupBlob(blob);
   }
 
  private:
-  void MaybeWriteStartupBlob(const i::SnapshotData& sd,
-                             const i::SnapshotData& csd) const {
+  void MaybeWriteStartupBlob(const i::Vector<const i::byte>& blob) const {
     if (!startup_blob_file_) return;
 
-    i::SnapshotByteSink sink;
-
-    sink.PutBlob(sd.RawData(), "snapshot");
-    sink.PutBlob(csd.RawData(), "context");
-
-    const i::List<i::byte>& startup_blob = sink.data();
-    size_t written = fwrite(startup_blob.begin(), 1, startup_blob.length(),
-                            startup_blob_file_);
-    if (written != static_cast<size_t>(startup_blob.length())) {
+    size_t written = fwrite(blob.begin(), 1, blob.length(), startup_blob_file_);
+    if (written != static_cast<size_t>(blob.length())) {
       i::PrintF("Writing snapshot file failed.. Aborting.\n");
       exit(1);
     }
   }
 
-  void WriteSnapshotFile(const i::SnapshotData& snapshot_data,
-                         const i::SnapshotData& context_snapshot_data) const {
+  void WriteSnapshotFile(const i::Vector<const i::byte>& blob) const {
     WriteFilePrefix();
-    WriteData("", snapshot_data.RawData(), raw_file_);
-    WriteData("context_", context_snapshot_data.RawData(), raw_context_file_);
+    WriteData(blob);
     WriteFileSuffix();
   }
 
@@ -88,47 +74,29 @@ class SnapshotWriter {
   }
 
   void WriteFileSuffix() const {
+    fprintf(fp_, "const v8::StartupData Snapshot::SnapshotBlob() {\n");
+    fprintf(fp_, "  v8::StartupData blob;\n");
+    fprintf(fp_, "  blob.data = reinterpret_cast<const char*>(blob_data);\n");
+    fprintf(fp_, "  blob.raw_size = blob_size;\n");
+    fprintf(fp_, "  return blob;\n");
+    fprintf(fp_, "}\n\n");
     fprintf(fp_, "}  // namespace internal\n");
     fprintf(fp_, "}  // namespace v8\n");
   }
 
-  void WriteData(const char* prefix,
-                 const i::Vector<const i::byte>& source_data,
-                 FILE* raw_file) const {
-    MaybeWriteRawFile(&source_data, raw_file);
-    WriteData(prefix, source_data);
-  }
-
-  void MaybeWriteRawFile(const i::Vector<const i::byte>* data,
-                         FILE* raw_file) const {
-    if (!data || !raw_file) return;
-
-    // Sanity check, whether i::List iterators truly return pointers to an
-    // internal array.
-    DCHECK(data->end() - data->begin() == data->length());
-
-    size_t written = fwrite(data->begin(), 1, data->length(), raw_file);
-    if (written != static_cast<size_t>(data->length())) {
-      i::PrintF("Writing raw file failed.. Aborting.\n");
-      exit(1);
-    }
-  }
-
-  void WriteData(const char* prefix,
-                 const i::Vector<const i::byte>& source_data) const {
-    fprintf(fp_, "const byte Snapshot::%sdata_[] = {\n", prefix);
-    WriteSnapshotData(source_data);
+  void WriteData(const i::Vector<const i::byte>& blob) const {
+    fprintf(fp_, "static const byte blob_data[] = {\n");
+    WriteSnapshotData(blob);
     fprintf(fp_, "};\n");
-    fprintf(fp_, "const int Snapshot::%ssize_ = %d;\n", prefix,
-            source_data.length());
+    fprintf(fp_, "static const int blob_size = %d;\n", blob.length());
     fprintf(fp_, "\n");
   }
 
-  void WriteSnapshotData(const i::Vector<const i::byte>& data) const {
-    for (int i = 0; i < data.length(); i++) {
+  void WriteSnapshotData(const i::Vector<const i::byte>& blob) const {
+    for (int i = 0; i < blob.length(); i++) {
       if ((i & 0x1f) == 0x1f) fprintf(fp_, "\n");
       if (i > 0) fprintf(fp_, ",");
-      fprintf(fp_, "%u", static_cast<unsigned char>(data.at(i)));
+      fprintf(fp_, "%u", static_cast<unsigned char>(blob.at(i)));
     }
     fprintf(fp_, "\n");
   }
@@ -143,8 +111,6 @@ class SnapshotWriter {
   }
 
   FILE* fp_;
-  FILE* raw_file_;
-  FILE* raw_context_file_;
   FILE* startup_blob_file_;
 };
 
@@ -270,8 +236,6 @@ int main(int argc, char** argv) {
 
     {
       SnapshotWriter writer(argv[1]);
-      if (i::FLAG_raw_file && i::FLAG_raw_context_file)
-        writer.SetRawFiles(i::FLAG_raw_file, i::FLAG_raw_context_file);
       if (i::FLAG_startup_blob)
         writer.SetStartupBlobFile(i::FLAG_startup_blob);
       i::SnapshotData sd(snapshot_sink, ser);

src/natives-external.cc

@@ -10,6 +10,11 @@
 #include "src/snapshot-source-sink.h"
 #include "src/vector.h"
 
+#ifndef V8_USE_EXTERNAL_STARTUP_DATA
+#error natives-external.cc is used only for the external snapshot build.
+#endif  // V8_USE_EXTERNAL_STARTUP_DATA
+
+
 namespace v8 {
 namespace internal {
 
@@ -142,8 +147,7 @@ void SetNativesFromFile(StartupData* natives_blob) {
   DCHECK(natives_blob->data);
   DCHECK(natives_blob->raw_size > 0);
 
-  SnapshotByteSource bytes(reinterpret_cast<const byte*>(natives_blob->data),
-                           natives_blob->raw_size);
+  SnapshotByteSource bytes(natives_blob->data, natives_blob->raw_size);
   NativesHolder<CORE>::set(NativesStore::MakeFromScriptsSource(&bytes));
   NativesHolder<EXPERIMENTAL>::set(NativesStore::MakeFromScriptsSource(&bytes));
   DCHECK(!bytes.HasMore());
@@ -188,7 +192,7 @@ Vector<const char> NativesCollection<type>::GetScriptsSource() {
 
 
 // The compiler can't 'see' all uses of the static methods and hence
-// my chose to elide them. This we'll explicitly instantiate these.
+// my choice to elide them. This we'll explicitly instantiate these.
 template class NativesCollection<CORE>;
 template class NativesCollection<EXPERIMENTAL>;
 

src/serialize.h

@@ -865,8 +865,8 @@ class SnapshotData : public SerializedData {
   SnapshotData(const SnapshotByteSink& sink, const Serializer& ser);
 
   // Used when consuming.
-  explicit SnapshotData(const byte* data, int size)
-      : SerializedData(const_cast<byte*>(data), size) {
+  explicit SnapshotData(const Vector<const byte> snapshot)
+      : SerializedData(const_cast<byte*>(snapshot.begin()), snapshot.length()) {
     CHECK(IsSane());
   }
 

src/snapshot-common.cc

@@ -14,34 +14,56 @@
 namespace v8 {
 namespace internal {
 
+bool Snapshot::HaveASnapshotToStartFrom() {
+  return SnapshotBlob().data != NULL;
+}
 
-bool Snapshot::Initialize(Isolate* isolate) {
-  if (size_ > 0) {
-    base::ElapsedTimer timer;
-    if (FLAG_profile_deserialization) timer.Start();
-
-    SnapshotData snapshot_data(data_, size_);
-    Deserializer deserializer(&snapshot_data);
-    bool success = isolate->Init(&deserializer);
-    if (FLAG_profile_deserialization) {
-      double ms = timer.Elapsed().InMillisecondsF();
-      PrintF("[Snapshot loading and deserialization took %0.3f ms]\n", ms);
-    }
-    return success;
-  }
-  return false;
+
+const Vector<const byte> Snapshot::StartupSnapshot() {
+  DCHECK(HaveASnapshotToStartFrom());
+  const v8::StartupData blob = SnapshotBlob();
+  SnapshotByteSource source(blob.data, blob.raw_size);
+  const byte* data;
+  int length;
+  bool success = source.GetBlob(&data, &length);
+  CHECK(success);
+  return Vector<const byte>(data, length);
 }
 
 
-bool Snapshot::HaveASnapshotToStartFrom() {
-  return size_ != 0;
+const Vector<const byte> Snapshot::ContextSnapshot() {
+  DCHECK(HaveASnapshotToStartFrom());
+  const v8::StartupData blob = SnapshotBlob();
+  SnapshotByteSource source(blob.data, blob.raw_size);
+  const byte* data;
+  int length;
+  bool success = source.GetBlob(&data, &length);
+  success &= source.GetBlob(&data, &length);
+  CHECK(success);
+  return Vector<const byte>(data, length);
+}
+
+
+bool Snapshot::Initialize(Isolate* isolate) {
+  if (!HaveASnapshotToStartFrom()) return false;
+  base::ElapsedTimer timer;
+  if (FLAG_profile_deserialization) timer.Start();
+
+  SnapshotData snapshot_data(StartupSnapshot());
+  Deserializer deserializer(&snapshot_data);
+  bool success = isolate->Init(&deserializer);
+  if (FLAG_profile_deserialization) {
+    double ms = timer.Elapsed().InMillisecondsF();
+    PrintF("[Snapshot loading and deserialization took %0.3f ms]\n", ms);
+  }
+  return success;
 }
 
 
 Handle<Context> Snapshot::NewContextFromSnapshot(Isolate* isolate) {
-  if (context_size_ == 0) return Handle<Context>();
+  if (!HaveASnapshotToStartFrom()) return Handle<Context>();
 
-  SnapshotData snapshot_data(context_data_, context_size_);
+  SnapshotData snapshot_data(ContextSnapshot());
   Deserializer deserializer(&snapshot_data);
   Object* root;
   deserializer.DeserializePartial(isolate, &root);
@@ -49,15 +71,4 @@ Handle<Context> Snapshot::NewContextFromSnapshot(Isolate* isolate) {
   return Handle<Context>(Context::cast(root));
 }
 
-
-#ifdef V8_USE_EXTERNAL_STARTUP_DATA
-// Dummy implementations of Set*FromFile(..) APIs.
-//
-// These are meant for use with snapshot-external.cc. Should this file
-// be compiled with those options we just supply these dummy implementations
-// below. This happens when compiling the mksnapshot utility.
-void SetNativesFromFile(StartupData* data) { CHECK(false); }
-void SetSnapshotFromFile(StartupData* data) { CHECK(false); }
-#endif  // V8_USE_EXTERNAL_STARTUP_DATA
-
 } }  // namespace v8::internal

src/snapshot-empty.cc

@@ -11,9 +11,18 @@
 namespace v8 {
 namespace internal {
 
-const byte Snapshot::data_[] = { 0 };
-const int Snapshot::size_ = 0;
-const byte Snapshot::context_data_[] = { 0 };
-const int Snapshot::context_size_ = 0;
+#ifdef V8_USE_EXTERNAL_STARTUP_DATA
+// Dummy implementations of Set*FromFile(..) APIs.
+//
+// These are meant for use with snapshot-external.cc. Should this file
+// be compiled with those options we just supply these dummy implementations
+// below. This happens when compiling the mksnapshot utility.
+void SetNativesFromFile(StartupData* data) { CHECK(false); }
+void SetSnapshotFromFile(StartupData* data) { CHECK(false); }
+#endif  // V8_USE_EXTERNAL_STARTUP_DATA
 
+
+const v8::StartupData Snapshot::SnapshotBlob() {
+  return {NULL, 0};
+}
 } }  // namespace v8::internal

src/snapshot-external.cc

@@ -10,70 +10,29 @@
 #include "src/snapshot-source-sink.h"
 #include "src/v8.h"  // for V8::Initialize
 
-namespace v8 {
-namespace internal {
-
-
-struct SnapshotImpl {
- public:
-  const byte* data;
-  int size;
-  const byte* context_data;
-  int context_size;
-};
-
-
-static SnapshotImpl* snapshot_impl_ = NULL;
-
-
-bool Snapshot::HaveASnapshotToStartFrom() {
-  return snapshot_impl_ != NULL;
-}
 
+#ifndef V8_USE_EXTERNAL_STARTUP_DATA
+#error snapshot-external.cc is used only for the external snapshot build.
+#endif  // V8_USE_EXTERNAL_STARTUP_DATA
 
-bool Snapshot::Initialize(Isolate* isolate) {
-  if (!HaveASnapshotToStartFrom()) return false;
-  base::ElapsedTimer timer;
-  if (FLAG_profile_deserialization) timer.Start();
 
-  SnapshotData snapshot_data(snapshot_impl_->data, snapshot_impl_->size);
-  Deserializer deserializer(&snapshot_data);
-  bool success = isolate->Init(&deserializer);
-  if (FLAG_profile_deserialization) {
-    double ms = timer.Elapsed().InMillisecondsF();
-    PrintF("[Snapshot loading and deserialization took %0.3f ms]\n", ms);
-  }
-  return success;
-}
-
-
-Handle<Context> Snapshot::NewContextFromSnapshot(Isolate* isolate) {
-  if (!HaveASnapshotToStartFrom()) return Handle<Context>();
-
-  SnapshotData snapshot_data(snapshot_impl_->context_data,
-                             snapshot_impl_->context_size);
-  Deserializer deserializer(&snapshot_data);
-  Object* root;
-  deserializer.DeserializePartial(isolate, &root);
-  CHECK(root->IsContext());
-  return Handle<Context>(Context::cast(root));
-}
+namespace v8 {
+namespace internal {
 
+static v8::StartupData external_startup_blob = {NULL, 0};
 
 void SetSnapshotFromFile(StartupData* snapshot_blob) {
   DCHECK(snapshot_blob);
   DCHECK(snapshot_blob->data);
   DCHECK(snapshot_blob->raw_size > 0);
-  DCHECK(!snapshot_impl_);
+  DCHECK(!external_startup_blob.data);
+  external_startup_blob = *snapshot_blob;
 
-  snapshot_impl_ = new SnapshotImpl;
-  SnapshotByteSource source(reinterpret_cast<const byte*>(snapshot_blob->data),
-                            snapshot_blob->raw_size);
-  bool success = source.GetBlob(&snapshot_impl_->data,
-                                &snapshot_impl_->size);
-  success &= source.GetBlob(&snapshot_impl_->context_data,
-                            &snapshot_impl_->context_size);
-  DCHECK(success);
+  // Validate snapshot blob.
+  DCHECK(!Snapshot::StartupSnapshot().is_empty());
+  DCHECK(!Snapshot::ContextSnapshot().is_empty());
 }
 
+
+const v8::StartupData Snapshot::SnapshotBlob() { return external_startup_blob; }
 } }  // namespace v8::internal

src/snapshot-source-sink.h

@@ -19,8 +19,10 @@ namespace internal {
  */
 class SnapshotByteSource FINAL {
  public:
-  SnapshotByteSource(const byte* data, int length)
-      : data_(data), length_(length), position_(0) {}
+  SnapshotByteSource(const char* data, int length)
+      : data_(reinterpret_cast<const byte*>(data)),
+        length_(length),
+        position_(0) {}
 
   explicit SnapshotByteSource(Vector<const byte> payload)
       : data_(payload.start()), length_(payload.length()), position_(0) {}

src/snapshot.h

@@ -15,18 +15,16 @@ class Snapshot {
   // Initialize the Isolate from the internal snapshot. Returns false if no
   // snapshot could be found.
   static bool Initialize(Isolate* isolate);
+  // Create a new context using the internal partial snapshot.
+  static Handle<Context> NewContextFromSnapshot(Isolate* isolate);
 
+  static const Vector<const byte> StartupSnapshot();
+  static const Vector<const byte> ContextSnapshot();
   static bool HaveASnapshotToStartFrom();
 
-  // Create a new context using the internal partial snapshot.
-  static Handle<Context> NewContextFromSnapshot(Isolate* isolate);
+  static const v8::StartupData SnapshotBlob();
 
  private:
-  static const byte data_[];
-  static const int size_;
-  static const byte context_data_[];
-  static const int context_size_;
-
   DISALLOW_IMPLICIT_CONSTRUCTORS(Snapshot);
 };
 

test/cctest/test-serialize.cc

@@ -188,7 +188,7 @@ v8::Isolate* InitializeFromFile(const char* snapshot_file) {
   if (!str) return NULL;
   v8::Isolate* v8_isolate = NULL;
   {
-    SnapshotData snapshot_data(str, len);
+    SnapshotData snapshot_data(Vector<const byte>(str, len));
     Deserializer deserializer(&snapshot_data);
     Isolate* isolate = Isolate::NewForTesting();
     v8_isolate = reinterpret_cast<v8::Isolate*>(isolate);
@@ -393,7 +393,7 @@ UNINITIALIZED_DEPENDENT_TEST(PartialDeserialization, PartialSerialization) {
       Isolate* isolate = reinterpret_cast<Isolate*>(v8_isolate);
       Object* root;
       {
-        SnapshotData snapshot_data(snapshot, snapshot_size);
+        SnapshotData snapshot_data(Vector<const byte>(snapshot, snapshot_size));
         Deserializer deserializer(&snapshot_data);
         deserializer.DeserializePartial(isolate, &root);
         CHECK(root->IsString());
@@ -404,7 +404,7 @@ UNINITIALIZED_DEPENDENT_TEST(PartialDeserialization, PartialSerialization) {
 
       Object* root2;
       {
-        SnapshotData snapshot_data(snapshot, snapshot_size);
+        SnapshotData snapshot_data(Vector<const byte>(snapshot, snapshot_size));
         Deserializer deserializer(&snapshot_data);
         deserializer.DeserializePartial(isolate, &root2);
         CHECK(root2->IsString());
@@ -503,7 +503,7 @@ UNINITIALIZED_DEPENDENT_TEST(ContextDeserialization, ContextSerialization) {
       Isolate* isolate = reinterpret_cast<Isolate*>(v8_isolate);
       Object* root;
       {
-        SnapshotData snapshot_data(snapshot, snapshot_size);
+        SnapshotData snapshot_data(Vector<const byte>(snapshot, snapshot_size));
         Deserializer deserializer(&snapshot_data);
         deserializer.DeserializePartial(isolate, &root);
         CHECK(root->IsContext());
@@ -514,7 +514,7 @@ UNINITIALIZED_DEPENDENT_TEST(ContextDeserialization, ContextSerialization) {
 
       Object* root2;
       {
-        SnapshotData snapshot_data(snapshot, snapshot_size);
+        SnapshotData snapshot_data(Vector<const byte>(snapshot, snapshot_size));
         Deserializer deserializer(&snapshot_data);
         deserializer.DeserializePartial(isolate, &root2);
         CHECK(root2->IsContext());

tools/gyp/v8.gyp

@@ -164,7 +164,6 @@
         '<(SHARED_INTERMEDIATE_DIR)/libraries.cc',
         '<(SHARED_INTERMEDIATE_DIR)/experimental-libraries.cc',
         '<(INTERMEDIATE_DIR)/snapshot.cc',
-        '../../src/snapshot-common.cc',
       ],
       'actions': [
         {
@@ -206,7 +205,6 @@
       'sources': [
         '<(SHARED_INTERMEDIATE_DIR)/libraries.cc',
         '<(SHARED_INTERMEDIATE_DIR)/experimental-libraries.cc',
-        '../../src/snapshot-common.cc',
         '../../src/snapshot-empty.cc',
       ],
       'conditions': [
@@ -824,6 +822,7 @@
         '../../src/small-pointer-list.h',
         '../../src/smart-pointers.h',
         '../../src/snapshot.h',
+        '../../src/snapshot-common.cc',
         '../../src/snapshot-source-sink.cc',
         '../../src/snapshot-source-sink.h',
         '../../src/string-builder.cc',