From da6dbe78bc1052de49dd47935cc2a74a3433130e Mon Sep 17 00:00:00 2001 From: machenbach Date: Tue, 9 Dec 2014 08:19:40 -0800 Subject: [PATCH] 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 | 3 +- Makefile | 3 -- src/flag-definitions.h | 6 +++ src/mksnapshot.cc | 86 ++++++++++++++++++++++++++++++------------- src/natives-external.cc | 10 ++--- src/serialize.h | 4 +- src/snapshot-common.cc | 71 +++++++++++++++-------------------- src/snapshot-empty.cc | 17 ++------- src/snapshot-external.cc | 70 +++++++++++++++++++++++++++-------- src/snapshot-source-sink.h | 6 +-- src/snapshot.h | 12 +++--- test/cctest/test-serialize.cc | 10 ++--- tools/gyp/v8.gyp | 3 +- 13 files changed, 178 insertions(+), 123 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index 0200dc0..5ba15f8 100644 --- a/BUILD.gn +++ b/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", diff --git a/Makefile b/Makefile index 606b5d7..6a34a72 100644 --- a/Makefile +++ b/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 diff --git a/src/flag-definitions.h b/src/flag-definitions.h index 368f931..5cdad43 100644 --- a/src/flag-definitions.h +++ b/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)") diff --git a/src/mksnapshot.cc b/src/mksnapshot.cc index 7194a61..77015a2 100644 --- a/src/mksnapshot.cc +++ b/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& blob = sink.data().ToConstVector(); - - WriteSnapshotFile(blob); - MaybeWriteStartupBlob(blob); + WriteSnapshotFile(sd, csd); + MaybeWriteStartupBlob(sd, csd); } private: - void MaybeWriteStartupBlob(const i::Vector& 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(blob.length())) { + i::SnapshotByteSink sink; + + sink.PutBlob(sd.RawData(), "snapshot"); + sink.PutBlob(csd.RawData(), "context"); + + const i::List& startup_blob = sink.data(); + size_t written = fwrite(startup_blob.begin(), 1, startup_blob.length(), + startup_blob_file_); + if (written != static_cast(startup_blob.length())) { i::PrintF("Writing snapshot file failed.. Aborting.\n"); exit(1); } } - void WriteSnapshotFile(const i::Vector& 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(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& blob) const { - fprintf(fp_, "static const byte blob_data[] = {\n"); - WriteSnapshotData(blob); + void WriteData(const char* prefix, + const i::Vector& source_data, + FILE* raw_file) const { + MaybeWriteRawFile(&source_data, raw_file); + WriteData(prefix, source_data); + } + + void MaybeWriteRawFile(const i::Vector* 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(data->length())) { + i::PrintF("Writing raw file failed.. Aborting.\n"); + exit(1); + } + } + + void WriteData(const char* prefix, + const i::Vector& 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& blob) const { - for (int i = 0; i < blob.length(); i++) { + void WriteSnapshotData(const i::Vector& 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(blob.at(i))); + fprintf(fp_, "%u", static_cast(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); diff --git a/src/natives-external.cc b/src/natives-external.cc index 4312091..cfa00bc 100644 --- a/src/natives-external.cc +++ b/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(natives_blob->data), + natives_blob->raw_size); NativesHolder::set(NativesStore::MakeFromScriptsSource(&bytes)); NativesHolder::set(NativesStore::MakeFromScriptsSource(&bytes)); DCHECK(!bytes.HasMore()); @@ -192,7 +188,7 @@ Vector NativesCollection::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; template class NativesCollection; diff --git a/src/serialize.h b/src/serialize.h index c524fcf..0c34650 100644 --- a/src/serialize.h +++ b/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 snapshot) - : SerializedData(const_cast(snapshot.begin()), snapshot.length()) { + explicit SnapshotData(const byte* data, int size) + : SerializedData(const_cast(data), size) { CHECK(IsSane()); } diff --git a/src/snapshot-common.cc b/src/snapshot-common.cc index 8046354..f06f38a 100644 --- a/src/snapshot-common.cc +++ b/src/snapshot-common.cc @@ -14,56 +14,34 @@ namespace v8 { namespace internal { -bool Snapshot::HaveASnapshotToStartFrom() { - return SnapshotBlob().data != NULL; -} - - -const Vector 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(data, length); -} - -const Vector 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(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 Snapshot::NewContextFromSnapshot(Isolate* isolate) { - if (!HaveASnapshotToStartFrom()) return Handle(); + if (context_size_ == 0) return Handle(); - 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 Snapshot::NewContextFromSnapshot(Isolate* isolate) { return Handle(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 diff --git a/src/snapshot-empty.cc b/src/snapshot-empty.cc index 020d1cb..749c587 100644 --- a/src/snapshot-empty.cc +++ b/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 diff --git a/src/snapshot-external.cc b/src/snapshot-external.cc index c232b2e..17f3d71 100644 --- a/src/snapshot-external.cc +++ b/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 Snapshot::NewContextFromSnapshot(Isolate* isolate) { + if (!HaveASnapshotToStartFrom()) return Handle(); + + 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::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(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 diff --git a/src/snapshot-source-sink.h b/src/snapshot-source-sink.h index 306813c..f0cf1bb 100644 --- a/src/snapshot-source-sink.h +++ b/src/snapshot-source-sink.h @@ -19,10 +19,8 @@ namespace internal { */ class SnapshotByteSource FINAL { public: - SnapshotByteSource(const char* data, int length) - : data_(reinterpret_cast(data)), - length_(length), - position_(0) {} + SnapshotByteSource(const byte* data, int length) + : data_(data), length_(length), position_(0) {} explicit SnapshotByteSource(Vector payload) : data_(payload.start()), length_(payload.length()), position_(0) {} diff --git a/src/snapshot.h b/src/snapshot.h index d37b50a..384e655 100644 --- a/src/snapshot.h +++ b/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 NewContextFromSnapshot(Isolate* isolate); - static const Vector StartupSnapshot(); - static const Vector ContextSnapshot(); static bool HaveASnapshotToStartFrom(); - static const v8::StartupData SnapshotBlob(); + // Create a new context using the internal partial snapshot. + static Handle 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); }; diff --git a/test/cctest/test-serialize.cc b/test/cctest/test-serialize.cc index acd904f..9c23212 100644 --- a/test/cctest/test-serialize.cc +++ b/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(str, len)); + SnapshotData snapshot_data(str, len); Deserializer deserializer(&snapshot_data); Isolate* isolate = Isolate::NewForTesting(); v8_isolate = reinterpret_cast(isolate); @@ -393,7 +393,7 @@ UNINITIALIZED_DEPENDENT_TEST(PartialDeserialization, PartialSerialization) { Isolate* isolate = reinterpret_cast(v8_isolate); Object* root; { - SnapshotData snapshot_data(Vector(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(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(v8_isolate); Object* root; { - SnapshotData snapshot_data(Vector(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(snapshot, snapshot_size)); + SnapshotData snapshot_data(snapshot, snapshot_size); Deserializer deserializer(&snapshot_data); deserializer.DeserializePartial(isolate, &root2); CHECK(root2->IsContext()); diff --git a/tools/gyp/v8.gyp b/tools/gyp/v8.gyp index 016cfc5..58857d3 100644 --- a/tools/gyp/v8.gyp +++ b/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', -- 2.7.4