Deduplicate various snapshot blob helper functions

Helper functions to create and warm-up the snapshot blob were
duplicated in various spots (mksnapshot, inspector tests, serializer
cctests). This merges all of these into a single helper function
family declared in snapshot.h.

Bug: v8:9189, chromium:957029
Change-Id: I2d8d6fd8e955ffffd7d805c199d4a858500af588
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1598695
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61576}

src/snapshot/mksnapshot.cc

@@ -144,100 +144,37 @@ char* GetExtraCode(char* filename, const char* description) {
   return chars;
 }
 
-bool RunExtraCode(v8::Isolate* isolate, v8::Local<v8::Context> context,
-                  const char* utf8_source, const char* name) {
+v8::StartupData CreateSnapshotDataBlob(v8::Isolate* isolate,
+                                       const char* embedded_source) {
   v8::base::ElapsedTimer timer;
   timer.Start();
-  v8::Context::Scope context_scope(context);
-  v8::TryCatch try_catch(isolate);
-  v8::Local<v8::String> source_string;
-  if (!v8::String::NewFromUtf8(isolate, utf8_source, v8::NewStringType::kNormal)
-           .ToLocal(&source_string)) {
-    return false;
-  }
-  v8::Local<v8::String> resource_name =
-      v8::String::NewFromUtf8(isolate, name, v8::NewStringType::kNormal)
-          .ToLocalChecked();
-  v8::ScriptOrigin origin(resource_name);
-  v8::ScriptCompiler::Source source(source_string, origin);
-  v8::Local<v8::Script> script;
-  if (!v8::ScriptCompiler::Compile(context, &source).ToLocal(&script))
-    return false;
-  if (script->Run(context).IsEmpty()) return false;
-  if (i::FLAG_profile_deserialization) {
-    i::PrintF("Executing custom snapshot script %s took %0.3f ms\n", name,
-              timer.Elapsed().InMillisecondsF());
-  }
-  timer.Stop();
-  CHECK(!try_catch.HasCaught());
-  return true;
-}
 
-v8::StartupData CreateSnapshotDataBlob(v8::SnapshotCreator* snapshot_creator,
-                                       const char* script_source = nullptr) {
-  // Create a new isolate and a new context from scratch, optionally run
-  // a script to embed, and serialize to create a snapshot blob.
-  v8::StartupData result = {nullptr, 0};
-  v8::base::ElapsedTimer timer;
-  timer.Start();
-  {
-    v8::Isolate* isolate = snapshot_creator->GetIsolate();
-    {
-      v8::HandleScope scope(isolate);
-      v8::Local<v8::Context> context = v8::Context::New(isolate);
-      if (script_source != nullptr &&
-          !RunExtraCode(isolate, context, script_source, "<embedded>")) {
-        return result;
-      }
-      snapshot_creator->SetDefaultContext(context);
-    }
-    result = snapshot_creator->CreateBlob(
-        v8::SnapshotCreator::FunctionCodeHandling::kClear);
-  }
+  v8::StartupData result = i::CreateSnapshotDataBlobInternal(
+      v8::SnapshotCreator::FunctionCodeHandling::kClear, embedded_source,
+      isolate);
 
   if (i::FLAG_profile_deserialization) {
     i::PrintF("Creating snapshot took %0.3f ms\n",
               timer.Elapsed().InMillisecondsF());
   }
+
   timer.Stop();
   return result;
 }
 
-v8::StartupData WarmUpSnapshotDataBlob(v8::SnapshotCreator* snapshot_creator,
+v8::StartupData WarmUpSnapshotDataBlob(v8::StartupData cold_snapshot_blob,
                                        const char* warmup_source) {
-  CHECK_NOT_NULL(warmup_source);
-  // Use following steps to create a warmed up snapshot blob from a cold one:
-  //  - Create a new isolate from the cold snapshot.
-  //  - Create a new context to run the warmup script. This will trigger
-  //    compilation of executed functions.
-  //  - Create a new context. This context will be unpolluted.
-  //  - Serialize the isolate and the second context into a new snapshot blob.
-  v8::StartupData result = {nullptr, 0};
   v8::base::ElapsedTimer timer;
   timer.Start();
-  {
-    v8::Isolate* isolate = snapshot_creator->GetIsolate();
-    {
-      v8::HandleScope scope(isolate);
-      v8::Local<v8::Context> context = v8::Context::New(isolate);
-      if (!RunExtraCode(isolate, context, warmup_source, "<warm-up>")) {
-        return result;
-      }
-    }
-    {
-      v8::HandleScope handle_scope(isolate);
-      isolate->ContextDisposedNotification(false);
-      v8::Local<v8::Context> context = v8::Context::New(isolate);
-      snapshot_creator->SetDefaultContext(context);
-    }
-    result = snapshot_creator->CreateBlob(
-        v8::SnapshotCreator::FunctionCodeHandling::kKeep);
-  }
+
+  v8::StartupData result =
+      i::WarmUpSnapshotDataBlobInternal(cold_snapshot_blob, warmup_source);
 
   if (i::FLAG_profile_deserialization) {
     i::PrintF("Warming up snapshot took %0.3f ms\n",
               timer.Elapsed().InMillisecondsF());
   }
+
   timer.Stop();
   return result;
 }
@@ -332,18 +269,18 @@ int main(int argc, char** argv) {
         // to be written out if builtins are embedded.
         i_isolate->RegisterEmbeddedFileWriter(&embedded_writer);
       }
-      v8::SnapshotCreator snapshot_creator(isolate);
+      blob = CreateSnapshotDataBlob(isolate, embed_script.get());
       if (i::FLAG_embedded_builtins) {
+        // At this point, the Isolate has been torn down but the embedded blob
+        // is still alive (we called DisableEmbeddedBlobRefcounting above).
+        // That's fine as far as the embedded file writer is concerned.
         WriteEmbeddedFile(&embedded_writer);
       }
-      blob = CreateSnapshotDataBlob(&snapshot_creator, embed_script.get());
     }
 
     if (warmup_script) {
-      CHECK(blob.raw_size > 0 && blob.data != nullptr);
       v8::StartupData cold = blob;
-      v8::SnapshotCreator snapshot_creator(nullptr, &cold);
-      blob = WarmUpSnapshotDataBlob(&snapshot_creator, warmup_script.get());
+      blob = WarmUpSnapshotDataBlob(cold, warmup_script.get());
       delete[] cold.data;
     }
 

src/snapshot/snapshot-common.cc

@@ -380,29 +380,55 @@ bool RunExtraCode(v8::Isolate* isolate, v8::Local<v8::Context> context,
 
 }  // namespace
 
-// TODO(jgruber): Merge with related code in mksnapshot.cc and
-// inspector-test.cc.
 v8::StartupData CreateSnapshotDataBlobInternal(
     v8::SnapshotCreator::FunctionCodeHandling function_code_handling,
-    const char* embedded_source) {
-  // Create a new isolate and a new context from scratch, optionally run
-  // a script to embed, and serialize to create a snapshot blob.
-  v8::StartupData result = {nullptr, 0};
+    const char* embedded_source, v8::Isolate* isolate) {
+  // If no isolate is passed in, create it (and a new context) from scratch.
+  if (isolate == nullptr) isolate = v8::Isolate::Allocate();
+
+  // Optionally run a script to embed, and serialize to create a snapshot blob.
+  v8::SnapshotCreator snapshot_creator(isolate);
   {
-    v8::SnapshotCreator snapshot_creator;
-    v8::Isolate* isolate = snapshot_creator.GetIsolate();
-    {
-      v8::HandleScope scope(isolate);
-      v8::Local<v8::Context> context = v8::Context::New(isolate);
-      if (embedded_source != nullptr &&
-          !RunExtraCode(isolate, context, embedded_source, "<embedded>")) {
-        return result;
-      }
-      snapshot_creator.SetDefaultContext(context);
+    v8::HandleScope scope(isolate);
+    v8::Local<v8::Context> context = v8::Context::New(isolate);
+    if (embedded_source != nullptr &&
+        !RunExtraCode(isolate, context, embedded_source, "<embedded>")) {
+      return {};
     }
-    result = snapshot_creator.CreateBlob(function_code_handling);
+    snapshot_creator.SetDefaultContext(context);
   }
-  return result;
+  return snapshot_creator.CreateBlob(function_code_handling);
+}
+
+v8::StartupData WarmUpSnapshotDataBlobInternal(
+    v8::StartupData cold_snapshot_blob, const char* warmup_source) {
+  CHECK(cold_snapshot_blob.raw_size > 0 && cold_snapshot_blob.data != nullptr);
+  CHECK_NOT_NULL(warmup_source);
+
+  // Use following steps to create a warmed up snapshot blob from a cold one:
+  //  - Create a new isolate from the cold snapshot.
+  //  - Create a new context to run the warmup script. This will trigger
+  //    compilation of executed functions.
+  //  - Create a new context. This context will be unpolluted.
+  //  - Serialize the isolate and the second context into a new snapshot blob.
+  v8::SnapshotCreator snapshot_creator(nullptr, &cold_snapshot_blob);
+  v8::Isolate* isolate = snapshot_creator.GetIsolate();
+  {
+    v8::HandleScope scope(isolate);
+    v8::Local<v8::Context> context = v8::Context::New(isolate);
+    if (!RunExtraCode(isolate, context, warmup_source, "<warm-up>")) {
+      return {};
+    }
+  }
+  {
+    v8::HandleScope handle_scope(isolate);
+    isolate->ContextDisposedNotification(false);
+    v8::Local<v8::Context> context = v8::Context::New(isolate);
+    snapshot_creator.SetDefaultContext(context);
+  }
+
+  return snapshot_creator.CreateBlob(
+      v8::SnapshotCreator::FunctionCodeHandling::kKeep);
 }
 
 }  // namespace internal

src/snapshot/snapshot.h

@@ -162,7 +162,12 @@ class Snapshot : public AllStatic {
 // mksnapshot.
 V8_EXPORT_PRIVATE v8::StartupData CreateSnapshotDataBlobInternal(
     v8::SnapshotCreator::FunctionCodeHandling function_code_handling,
-    const char* embedded_source);
+    const char* embedded_source, v8::Isolate* isolate = nullptr);
+
+// Convenience wrapper around snapshot data blob warmup used e.g. by tests and
+// mksnapshot.
+V8_EXPORT_PRIVATE v8::StartupData WarmUpSnapshotDataBlobInternal(
+    v8::StartupData cold_snapshot_blob, const char* warmup_source);
 
 #ifdef V8_USE_EXTERNAL_STARTUP_DATA
 void SetSnapshotFromFile(StartupData* snapshot_blob);

test/cctest/test-serialize.cc

@@ -146,67 +146,12 @@ static Vector<const byte> WritePayload(const Vector<const byte>& payload) {
 
 namespace {
 
-bool RunExtraCode(v8::Isolate* isolate, v8::Local<v8::Context> context,
-                  const char* utf8_source, const char* name) {
-  v8::Context::Scope context_scope(context);
-  v8::TryCatch try_catch(isolate);
-  v8::Local<v8::String> source_string;
-  if (!v8::String::NewFromUtf8(isolate, utf8_source, v8::NewStringType::kNormal)
-           .ToLocal(&source_string)) {
-    return false;
-  }
-  v8::Local<v8::String> resource_name =
-      v8::String::NewFromUtf8(isolate, name, v8::NewStringType::kNormal)
-          .ToLocalChecked();
-  v8::ScriptOrigin origin(resource_name);
-  v8::ScriptCompiler::Source source(source_string, origin);
-  v8::Local<v8::Script> script;
-  if (!v8::ScriptCompiler::Compile(context, &source).ToLocal(&script))
-    return false;
-  if (script->Run(context).IsEmpty()) return false;
-  CHECK(!try_catch.HasCaught());
-  return true;
-}
-
-v8::StartupData CreateSnapshotDataBlob(const char* embedded_source = nullptr) {
+// Convenience wrapper around the convenience wrapper.
+v8::StartupData CreateSnapshotDataBlob(const char* embedded_source) {
   return CreateSnapshotDataBlobInternal(
       v8::SnapshotCreator::FunctionCodeHandling::kClear, embedded_source);
 }
 
-v8::StartupData WarmUpSnapshotDataBlob(v8::StartupData cold_snapshot_blob,
-                                       const char* warmup_source) {
-  CHECK(cold_snapshot_blob.raw_size > 0 && cold_snapshot_blob.data != nullptr);
-  CHECK_NOT_NULL(warmup_source);
-  // Use following steps to create a warmed up snapshot blob from a cold one:
-  //  - Create a new isolate from the cold snapshot.
-  //  - Create a new context to run the warmup script. This will trigger
-  //    compilation of executed functions.
-  //  - Create a new context. This context will be unpolluted.
-  //  - Serialize the isolate and the second context into a new snapshot blob.
-  v8::StartupData result = {nullptr, 0};
-  {
-    v8::SnapshotCreator snapshot_creator(nullptr, &cold_snapshot_blob);
-    v8::Isolate* isolate = snapshot_creator.GetIsolate();
-    {
-      v8::HandleScope scope(isolate);
-      v8::Local<v8::Context> context = v8::Context::New(isolate);
-      if (!RunExtraCode(isolate, context, warmup_source, "<warm-up>")) {
-        return result;
-      }
-    }
-    {
-      v8::HandleScope handle_scope(isolate);
-      isolate->ContextDisposedNotification(false);
-      v8::Local<v8::Context> context = v8::Context::New(isolate);
-      snapshot_creator.SetDefaultContext(context);
-    }
-    result = snapshot_creator.CreateBlob(
-        v8::SnapshotCreator::FunctionCodeHandling::kKeep);
-  }
-  ReadOnlyHeap::ClearSharedHeapForTest();
-  return result;
-}
-
 }  // namespace
 
 static StartupBlobs Serialize(v8::Isolate* isolate) {
@@ -1463,8 +1408,9 @@ UNINITIALIZED_TEST(SnapshotDataBlobWithWarmup) {
   const char* warmup = "Math.abs(1); Math.random = 1;";
 
   DisableEmbeddedBlobRefcounting();
-  v8::StartupData cold = CreateSnapshotDataBlob();
-  v8::StartupData warm = WarmUpSnapshotDataBlob(cold, warmup);
+  v8::StartupData cold = CreateSnapshotDataBlob(nullptr);
+  v8::StartupData warm = WarmUpSnapshotDataBlobInternal(cold, warmup);
+  ReadOnlyHeap::ClearSharedHeapForTest();
   delete[] cold.data;
 
   v8::Isolate::CreateParams params;
@@ -1500,7 +1446,8 @@ UNINITIALIZED_TEST(CustomSnapshotDataBlobWithWarmup) {
 
   DisableEmbeddedBlobRefcounting();
   v8::StartupData cold = CreateSnapshotDataBlob(source);
-  v8::StartupData warm = WarmUpSnapshotDataBlob(cold, warmup);
+  v8::StartupData warm = WarmUpSnapshotDataBlobInternal(cold, warmup);
+  ReadOnlyHeap::ClearSharedHeapForTest();
   delete[] cold.data;
 
   v8::Isolate::CreateParams params;

test/inspector/DEPS

@@ -8,4 +8,4 @@ include_rules = [
   "+src/locked-queue-inl.h",
   "+src/utils.h",
   "+src/vector.h",
-]
\ No newline at end of file
+]

test/inspector/inspector-test.cc

@@ -27,6 +27,12 @@ namespace internal {
 extern void DisableEmbeddedBlobRefcounting();
 extern void FreeCurrentEmbeddedBlob();
 
+extern v8::StartupData CreateSnapshotDataBlobInternal(
+    v8::SnapshotCreator::FunctionCodeHandling function_code_handling,
+    const char* embedded_source, v8::Isolate* isolate);
+extern v8::StartupData WarmUpSnapshotDataBlobInternal(
+    v8::StartupData cold_snapshot_blob, const char* warmup_source);
+
 }  // namespace internal
 }  // namespace v8
 
@@ -1044,50 +1050,6 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
   }
 };
 
-bool RunExtraCode(v8::Isolate* isolate, v8::Local<v8::Context> context,
-                  const char* utf8_source, const char* name) {
-  v8::Context::Scope context_scope(context);
-  v8::TryCatch try_catch(isolate);
-  v8::Local<v8::String> source_string;
-  if (!v8::String::NewFromUtf8(isolate, utf8_source, v8::NewStringType::kNormal)
-           .ToLocal(&source_string)) {
-    return false;
-  }
-  v8::Local<v8::String> resource_name =
-      v8::String::NewFromUtf8(isolate, name, v8::NewStringType::kNormal)
-          .ToLocalChecked();
-  v8::ScriptOrigin origin(resource_name);
-  v8::ScriptCompiler::Source source(source_string, origin);
-  v8::Local<v8::Script> script;
-  if (!v8::ScriptCompiler::Compile(context, &source).ToLocal(&script))
-    return false;
-  if (script->Run(context).IsEmpty()) return false;
-  CHECK(!try_catch.HasCaught());
-  return true;
-}
-
-v8::StartupData CreateSnapshotDataBlob(const char* embedded_source = nullptr) {
-  // Create a new isolate and a new context from scratch, optionally run
-  // a script to embed, and serialize to create a snapshot blob.
-  v8::StartupData result = {nullptr, 0};
-  {
-    v8::SnapshotCreator snapshot_creator;
-    v8::Isolate* isolate = snapshot_creator.GetIsolate();
-    {
-      v8::HandleScope scope(isolate);
-      v8::Local<v8::Context> context = v8::Context::New(isolate);
-      if (embedded_source != nullptr &&
-          !RunExtraCode(isolate, context, embedded_source, "<embedded>")) {
-        return result;
-      }
-      snapshot_creator.SetDefaultContext(context);
-    }
-    result = snapshot_creator.CreateBlob(
-        v8::SnapshotCreator::FunctionCodeHandling::kClear);
-  }
-  return result;
-}
-
 }  //  namespace
 
 int main(int argc, char* argv[]) {
@@ -1106,7 +1068,8 @@ int main(int argc, char* argv[]) {
     if (strcmp(argv[i], "--embed") == 0) {
       argv[i++] = nullptr;
       printf("Embedding script '%s'\n", argv[i]);
-      startup_data = CreateSnapshotDataBlob(argv[i]);
+      startup_data = i::CreateSnapshotDataBlobInternal(
+          v8::SnapshotCreator::FunctionCodeHandling::kClear, argv[i], nullptr);
       argv[i] = nullptr;
     }
   }