Revert of Use same blob format for internal and external snapshots. (patchset...

Revert of Use same blob format for internal and external snapshots. (patchset #2 id:20001 of https://codereview.chromium.org/787033002/)

Reason for revert:
[sheriff] breaks http://build.chromium.org/p/client.v8/builders/V8-Blink%20Linux%2064%20%28dbg%29/builds/1293

Original issue's description:
> Use same blob format for internal and external snapshots.
>
> R=vogelheim@chromium.org

TBR=vogelheim@chromium.org,yangguo@chromium.org
NOTREECHECKS=true
NOTRY=true

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

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

BUILD.gn

@@ -357,6 +357,7 @@ 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" ]
@@ -378,6 +379,7 @@ 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" ]
@@ -892,7 +894,6 @@ 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,9 +82,6 @@ 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

@@ -714,6 +714,12 @@ 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,13 +24,22 @@ 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);
@@ -38,29 +47,34 @@ class SnapshotWriter {
 
   void WriteSnapshot(const i::SnapshotData& sd,
                      const i::SnapshotData& csd) const {
-    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);
+    WriteSnapshotFile(sd, csd);
+    MaybeWriteStartupBlob(sd, csd);
   }
 
  private:
-  void MaybeWriteStartupBlob(const i::Vector<const i::byte>& blob) const {
+  void MaybeWriteStartupBlob(const i::SnapshotData& sd,
+                             const i::SnapshotData& csd) const {
     if (!startup_blob_file_) return;
 
-    size_t written = fwrite(blob.begin(), 1, blob.length(), startup_blob_file_);
-    if (written != static_cast<size_t>(blob.length())) {
+    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())) {
       i::PrintF("Writing snapshot file failed.. Aborting.\n");
       exit(1);
     }
   }
 
-  void WriteSnapshotFile(const i::Vector<const i::byte>& blob) const {
+  void WriteSnapshotFile(const i::SnapshotData& snapshot_data,
+                         const i::SnapshotData& context_snapshot_data) const {
     WriteFilePrefix();
-    WriteData(blob);
+    WriteData("", snapshot_data.RawData(), raw_file_);
+    WriteData("context_", context_snapshot_data.RawData(), raw_context_file_);
     WriteFileSuffix();
   }
 
@@ -74,29 +88,47 @@ 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 i::Vector<const i::byte>& blob) const {
-    fprintf(fp_, "static const byte blob_data[] = {\n");
-    WriteSnapshotData(blob);
+  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);
     fprintf(fp_, "};\n");
-    fprintf(fp_, "static const int blob_size = %d;\n", blob.length());
+    fprintf(fp_, "const int Snapshot::%ssize_ = %d;\n", prefix,
+            source_data.length());
     fprintf(fp_, "\n");
   }
 
-  void WriteSnapshotData(const i::Vector<const i::byte>& blob) const {
-    for (int i = 0; i < blob.length(); i++) {
+  void WriteSnapshotData(const i::Vector<const i::byte>& data) const {
+    for (int i = 0; i < data.length(); i++) {
       if ((i & 0x1f) == 0x1f) fprintf(fp_, "\n");
       if (i > 0) fprintf(fp_, ",");
-      fprintf(fp_, "%u", static_cast<unsigned char>(blob.at(i)));
+      fprintf(fp_, "%u", static_cast<unsigned char>(data.at(i)));
     }
     fprintf(fp_, "\n");
   }
@@ -111,6 +143,8 @@ class SnapshotWriter {
   }
 
   FILE* fp_;
+  FILE* raw_file_;
+  FILE* raw_context_file_;
   FILE* startup_blob_file_;
 };
 
@@ -236,6 +270,8 @@ 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,11 +10,6 @@
 #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 {
 
@@ -147,7 +142,8 @@ void SetNativesFromFile(StartupData* natives_blob) {
   DCHECK(natives_blob->data);
   DCHECK(natives_blob->raw_size > 0);
 
-  SnapshotByteSource bytes(natives_blob->data, natives_blob->raw_size);
+  SnapshotByteSource bytes(reinterpret_cast<const byte*>(natives_blob->data),
+                           natives_blob->raw_size);
   NativesHolder<CORE>::set(NativesStore::MakeFromScriptsSource(&bytes));
   NativesHolder<EXPERIMENTAL>::set(NativesStore::MakeFromScriptsSource(&bytes));
   DCHECK(!bytes.HasMore());
@@ -192,7 +188,7 @@ Vector<const char> NativesCollection<type>::GetScriptsSource() {
 
 
 // The compiler can't 'see' all uses of the static methods and hence
-// my choice to elide them. This we'll explicitly instantiate these.
+// my chose 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 Vector<const byte> snapshot)
-      : SerializedData(const_cast<byte*>(snapshot.begin()), snapshot.length()) {
+  explicit SnapshotData(const byte* data, int size)
+      : SerializedData(const_cast<byte*>(data), size) {
     CHECK(IsSane());
   }
 

src/snapshot-common.cc

@@ -14,56 +14,34 @@
 namespace v8 {
 namespace internal {
 
-bool Snapshot::HaveASnapshotToStartFrom() {
-  return SnapshotBlob().data != NULL;
-}
-
-
-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);
-}
-
 
-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 (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;
 }
 
 
-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;
+bool Snapshot::HaveASnapshotToStartFrom() {
+  return size_ != 0;
 }
 
 
 Handle<Context> Snapshot::NewContextFromSnapshot(Isolate* isolate) {
-  if (!HaveASnapshotToStartFrom()) return Handle<Context>();
+  if (context_size_ == 0) return Handle<Context>();
 
-  SnapshotData snapshot_data(ContextSnapshot());
+  SnapshotData snapshot_data(context_data_, context_size_);
   Deserializer deserializer(&snapshot_data);
   Object* root;
   deserializer.DeserializePartial(isolate, &root);
@@ -71,4 +49,15 @@ 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,18 +11,9 @@
 namespace v8 {
 namespace internal {
 
-#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 byte Snapshot::data_[] = { 0 };
+const int Snapshot::size_ = 0;
+const byte Snapshot::context_data_[] = { 0 };
+const int Snapshot::context_size_ = 0;
 
-
-const v8::StartupData Snapshot::SnapshotBlob() {
-  return {NULL, 0};
-}
 } }  // namespace v8::internal

src/snapshot-external.cc

@@ -10,32 +10,70 @@
 #include "src/snapshot-source-sink.h"
 #include "src/v8.h"  // for V8::Initialize
 
+namespace v8 {
+namespace internal {
 
-#ifndef V8_USE_EXTERNAL_STARTUP_DATA
-#error snapshot-external.cc is used only for the external snapshot build.
-#endif  // V8_USE_EXTERNAL_STARTUP_DATA
 
+struct SnapshotImpl {
+ public:
+  const byte* data;
+  int size;
+  const byte* context_data;
+  int context_size;
+};
 
-namespace v8 {
-namespace internal {
 
-static v8::StartupData* external_startup_blob = NULL;
+static SnapshotImpl* snapshot_impl_ = NULL;
+
+
+bool Snapshot::HaveASnapshotToStartFrom() {
+  return snapshot_impl_ != NULL;
+}
+
+
+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));
+}
+
 
 void SetSnapshotFromFile(StartupData* snapshot_blob) {
   DCHECK(snapshot_blob);
   DCHECK(snapshot_blob->data);
   DCHECK(snapshot_blob->raw_size > 0);
-  DCHECK(!external_startup_blob);
-  external_startup_blob = snapshot_blob;
-
-  // Validate snapshot blob.
-  DCHECK(!Snapshot::StartupSnapshot().is_empty());
-  DCHECK(!Snapshot::ContextSnapshot().is_empty());
-}
-
+  DCHECK(!snapshot_impl_);
 
-const v8::StartupData Snapshot::SnapshotBlob() {
-  return *external_startup_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);
 }
 
 } }  // namespace v8::internal

src/snapshot-source-sink.h

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

src/snapshot.h

@@ -15,16 +15,18 @@ 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();
 
-  static const v8::StartupData SnapshotBlob();
+  // Create a new context using the internal partial snapshot.
+  static Handle<Context> NewContextFromSnapshot(Isolate* isolate);
 
  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(Vector<const byte>(str, len));
+    SnapshotData snapshot_data(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(Vector<const byte>(snapshot, snapshot_size));
+        SnapshotData snapshot_data(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(Vector<const byte>(snapshot, snapshot_size));
+        SnapshotData snapshot_data(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(Vector<const byte>(snapshot, snapshot_size));
+        SnapshotData snapshot_data(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(Vector<const byte>(snapshot, snapshot_size));
+        SnapshotData snapshot_data(snapshot, snapshot_size);
         Deserializer deserializer(&snapshot_data);
         deserializer.DeserializePartial(isolate, &root2);
         CHECK(root2->IsContext());

tools/gyp/v8.gyp

@@ -164,6 +164,7 @@
         '<(SHARED_INTERMEDIATE_DIR)/libraries.cc',
         '<(SHARED_INTERMEDIATE_DIR)/experimental-libraries.cc',
         '<(INTERMEDIATE_DIR)/snapshot.cc',
+        '../../src/snapshot-common.cc',
       ],
       'actions': [
         {
@@ -205,6 +206,7 @@
       'sources': [
         '<(SHARED_INTERMEDIATE_DIR)/libraries.cc',
         '<(SHARED_INTERMEDIATE_DIR)/experimental-libraries.cc',
+        '../../src/snapshot-common.cc',
         '../../src/snapshot-empty.cc',
       ],
       'conditions': [
@@ -822,7 +824,6 @@
         '../../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',