[web snapshot] Deduplicate SFIs

The de-duplication happens when
1) we have a JSFunction for an outer function and a JSFunction for its
inner function in the snapshot and
2) we call the outer function again after deserializing

Expectation: the created JSFunction for the inner function uses the
SFI which was created when deserializing.

Bug: v8:11525
Change-Id: I80933514873e857452585317248fa34913d8d8e7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2794438Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Shu-yu Guo <syg@chromium.org>
Commit-Queue: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73900}

src/codegen/compiler.cc

@@ -611,8 +611,10 @@ void EnsureSharedFunctionInfosArrayOnScript(Handle<Script> script,
                                             ParseInfo* parse_info,
                                             LocalIsolate* isolate) {
   DCHECK(parse_info->flags().is_toplevel());
-  if (script->shared_function_infos().length() > 0) {
-    DCHECK_EQ(script->shared_function_infos().length(),
+  if (script->shared_function_info_count() > 0) {
+    DCHECK_LE(script->shared_function_info_count(),
+              script->shared_function_infos().length());
+    DCHECK_EQ(script->shared_function_info_count(),
               parse_info->max_function_literal_id() + 1);
     return;
   }
@@ -1389,8 +1391,7 @@ void FinalizeUnoptimizedScriptCompilation(
       FunctionLiteral* literal = it.first;
       CompilerDispatcher::JobId job_id = it.second;
       MaybeHandle<SharedFunctionInfo> maybe_shared_for_task =
-          script->FindSharedFunctionInfo(isolate,
-                                         literal->function_literal_id());
+          Script::FindSharedFunctionInfo(script, isolate, literal);
       Handle<SharedFunctionInfo> shared_for_task;
       if (maybe_shared_for_task.ToHandle(&shared_for_task)) {
         dispatcher->RegisterSharedFunctionInfo(job_id, *shared_for_task);
@@ -3170,8 +3171,7 @@ Handle<SharedFunctionInfo> Compiler::GetSharedFunctionInfo(
   MaybeHandle<SharedFunctionInfo> maybe_existing;
 
   // Find any previously allocated shared function info for the given literal.
-  maybe_existing =
-      script->FindSharedFunctionInfo(isolate, literal->function_literal_id());
+  maybe_existing = Script::FindSharedFunctionInfo(script, isolate, literal);
 
   // If we found an existing shared function info, return it.
   Handle<SharedFunctionInfo> existing;

src/debug/debug.cc

@@ -1608,7 +1608,9 @@ bool Debug::FindSharedFunctionInfosIntersectingRange(
     }
 
     if (!triedTopLevelCompile && !candidateSubsumesRange &&
-        script->shared_function_infos().length() > 0) {
+        script->shared_function_info_count() > 0) {
+      DCHECK_LE(script->shared_function_info_count(),
+                script->shared_function_infos().length());
       MaybeObject maybeToplevel = script->shared_function_infos().Get(0);
       HeapObject heap_object;
       const bool topLevelInfoExists =

src/diagnostics/objects-debug.cc

@@ -1685,7 +1685,13 @@ void AllocationSite::AllocationSiteVerify(Isolate* isolate) {
 
 void Script::ScriptVerify(Isolate* isolate) {
   TorqueGeneratedClassVerifiers::ScriptVerify(*this, isolate);
-  for (int i = 0; i < shared_function_infos().length(); ++i) {
+  if V8_UNLIKELY (type() == Script::TYPE_WEB_SNAPSHOT) {
+    CHECK_LE(shared_function_info_count(), shared_function_infos().length());
+  } else {
+    // No overallocating shared_function_infos.
+    CHECK_EQ(shared_function_info_count(), shared_function_infos().length());
+  }
+  for (int i = 0; i < shared_function_info_count(); ++i) {
     MaybeObject maybe_object = shared_function_infos().Get(i);
     HeapObject heap_object;
     CHECK(maybe_object->IsWeak() || maybe_object->IsCleared() ||

src/diagnostics/objects-printer.cc

@@ -2170,9 +2170,11 @@ void Script::ScriptPrint(std::ostream& os) {  // NOLINT
   if (!is_wasm) {
     if (has_eval_from_shared()) {
       os << "\n - eval from shared: " << Brief(eval_from_shared());
-    }
-    if (is_wrapped()) {
+    } else if (is_wrapped()) {
       os << "\n - wrapped arguments: " << Brief(wrapped_arguments());
+    } else if (type() == TYPE_WEB_SNAPSHOT) {
+      os << "\n - shared function info table: "
+         << Brief(shared_function_info_table());
     }
     os << "\n - eval from position: " << eval_from_position();
   }

src/heap/factory-base.cc

@@ -243,8 +243,8 @@ Handle<Script> FactoryBase<Impl>::NewScriptWithId(
     raw.set_context_data(roots.undefined_value(), SKIP_WRITE_BARRIER);
     raw.set_type(Script::TYPE_NORMAL);
     raw.set_line_ends(roots.undefined_value(), SKIP_WRITE_BARRIER);
-    raw.set_eval_from_shared_or_wrapped_arguments(roots.undefined_value(),
-                                                  SKIP_WRITE_BARRIER);
+    raw.set_eval_from_shared_or_wrapped_arguments_or_sfi_table(
+        roots.undefined_value(), SKIP_WRITE_BARRIER);
     raw.set_eval_from_position(0);
     raw.set_shared_function_infos(roots.empty_weak_fixed_array(),
                                   SKIP_WRITE_BARRIER);

src/heap/factory.cc

@@ -1311,8 +1311,8 @@ Handle<Script> Factory::CloneScript(Handle<Script> script) {
     new_script.set_context_data(old_script.context_data());
     new_script.set_type(old_script.type());
     new_script.set_line_ends(*undefined_value(), SKIP_WRITE_BARRIER);
-    new_script.set_eval_from_shared_or_wrapped_arguments(
-        script->eval_from_shared_or_wrapped_arguments());
+    new_script.set_eval_from_shared_or_wrapped_arguments_or_sfi_table(
+        script->eval_from_shared_or_wrapped_arguments_or_sfi_table());
     new_script.set_shared_function_infos(*empty_weak_fixed_array(),
                                          SKIP_WRITE_BARRIER);
     new_script.set_eval_from_position(old_script.eval_from_position());

src/objects/objects.cc

@@ -4963,14 +4963,21 @@ Object Script::GetNameOrSourceURL() {
 
 template <typename LocalIsolate>
 MaybeHandle<SharedFunctionInfo> Script::FindSharedFunctionInfo(
-    LocalIsolate* isolate, int function_literal_id) {
+    Handle<Script> script, LocalIsolate* isolate,
+    FunctionLiteral* function_literal) {
+  int function_literal_id = function_literal->function_literal_id();
+  if V8_UNLIKELY (script->type() == Script::TYPE_WEB_SNAPSHOT &&
+                  function_literal_id >= script->shared_function_info_count()) {
+    return FindWebSnapshotSharedFunctionInfo(script, isolate, function_literal);
+  }
+
   CHECK_NE(function_literal_id, kFunctionLiteralIdInvalid);
   // If this check fails, the problem is most probably the function id
   // renumbering done by AstFunctionLiteralIdReindexer; in particular, that
   // AstTraversalVisitor doesn't recurse properly in the construct which
   // triggers the mismatch.
-  CHECK_LT(function_literal_id, shared_function_infos().length());
-  MaybeObject shared = shared_function_infos().Get(function_literal_id);
+  CHECK_LT(function_literal_id, script->shared_function_info_count());
+  MaybeObject shared = script->shared_function_infos().Get(function_literal_id);
   HeapObject heap_object;
   if (!shared->GetHeapObject(&heap_object) ||
       heap_object.IsUndefined(isolate)) {
@@ -4979,9 +4986,81 @@ MaybeHandle<SharedFunctionInfo> Script::FindSharedFunctionInfo(
   return handle(SharedFunctionInfo::cast(heap_object), isolate);
 }
 template MaybeHandle<SharedFunctionInfo> Script::FindSharedFunctionInfo(
-    Isolate* isolate, int function_literal_id);
+    Handle<Script> script, Isolate* isolate, FunctionLiteral* function_literal);
 template MaybeHandle<SharedFunctionInfo> Script::FindSharedFunctionInfo(
-    LocalIsolate* isolate, int function_literal_id);
+    Handle<Script> script, LocalIsolate* isolate,
+    FunctionLiteral* function_literal);
+
+MaybeHandle<SharedFunctionInfo> Script::FindWebSnapshotSharedFunctionInfo(
+    Handle<Script> script, Isolate* isolate,
+    FunctionLiteral* function_literal) {
+  // We might be able to de-dupe the SFI against a SFI that was
+  // created when deserializing the snapshot (or when calling a function which
+  // was included in the snapshot). In that case, we can find it based on the
+  // start position in shared_function_info_table.
+  Handle<ObjectHashTable> shared_function_info_table = handle(
+      ObjectHashTable::cast(script->shared_function_info_table()), isolate);
+  {
+    DisallowHeapAllocation no_gc;
+    Object index_object = shared_function_info_table->Lookup(
+        handle(Smi::FromInt(function_literal->start_position()), isolate));
+    if (!index_object.IsTheHole()) {
+      int index = Smi::cast(index_object).value();
+      DCHECK_LT(index, script->shared_function_info_count());
+      MaybeObject maybe_shared = script->shared_function_infos().Get(index);
+      HeapObject heap_object;
+      if (!maybe_shared->GetHeapObject(&heap_object)) {
+        // We found the correct location but it's not filled in (e.g., the weak
+        // pointer to the SharedFunctionInfo has been cleared). Record the
+        // location in the FunctionLiteral, so that it will be refilled later.
+        // SharedFunctionInfo::SetScript will write the SharedFunctionInfo in
+        // the shared_function_infos.
+        function_literal->set_function_literal_id(index);
+        return MaybeHandle<SharedFunctionInfo>();
+      }
+      SharedFunctionInfo shared = SharedFunctionInfo::cast(heap_object);
+      DCHECK_EQ(shared.StartPosition(), function_literal->start_position());
+      DCHECK_EQ(shared.EndPosition(), function_literal->end_position());
+      return handle(shared, isolate);
+    }
+  }
+
+  // It's possible that FunctionLiterals which were processed before this one
+  // were deduplicated against existing ones. Decrease function_literal_id to
+  // avoid holes in shared_function_infos.
+  int old_length = script->shared_function_info_count();
+  int function_literal_id = old_length;
+  function_literal->set_function_literal_id(function_literal_id);
+
+  // Also add to shared_function_info_table.
+  shared_function_info_table = ObjectHashTable::Put(
+      shared_function_info_table,
+      handle(Smi::FromInt(function_literal->start_position()), isolate),
+      handle(Smi::FromInt(function_literal_id), isolate));
+  script->set_shared_function_info_table(*shared_function_info_table);
+
+  // Grow shared_function_infos if needed (we don't know the correct amount of
+  // space needed upfront).
+  int new_length = old_length + 1;
+  Handle<WeakFixedArray> old_infos =
+      handle(script->shared_function_infos(), isolate);
+  if (new_length > old_infos->length()) {
+    int capacity = WeakArrayList::CapacityForLength(new_length);
+    Handle<WeakFixedArray> new_infos(
+        isolate->factory()->NewWeakFixedArray(capacity, AllocationType::kOld));
+    new_infos->CopyElements(isolate, 0, *old_infos, 0, old_length,
+                            WriteBarrierMode::UPDATE_WRITE_BARRIER);
+    script->set_shared_function_infos(*new_infos);
+  }
+  return MaybeHandle<SharedFunctionInfo>();
+}
+
+MaybeHandle<SharedFunctionInfo> Script::FindWebSnapshotSharedFunctionInfo(
+    Handle<Script> script, LocalIsolate* isolate,
+    FunctionLiteral* function_literal) {
+  // Off-thread serialization of web snapshots is not implemented.
+  UNREACHABLE();
+}
 
 Script::Iterator::Iterator(Isolate* isolate)
     : iterator_(isolate->heap()->script_list()) {}

src/objects/script-inl.h

@@ -25,7 +25,7 @@ NEVER_READ_ONLY_SPACE_IMPL(Script)
 
 #if V8_ENABLE_WEBASSEMBLY
 ACCESSORS_CHECKED(Script, wasm_breakpoint_infos, FixedArray,
-                  kEvalFromSharedOrWrappedArgumentsOffset,
+                  kEvalFromSharedOrWrappedArgumentsOrSfiTableOffset,
                   this->type() == TYPE_WASM)
 ACCESSORS_CHECKED(Script, wasm_managed_native_module, Object,
                   kEvalFromPositionOffset, this->type() == TYPE_WASM)
@@ -37,40 +37,59 @@ ACCESSORS_CHECKED(Script, wasm_weak_instance_list, WeakArrayList,
 #endif  // V8_ENABLE_WEBASSEMBLY
 
 SMI_ACCESSORS(Script, type, kScriptTypeOffset)
-ACCESSORS_CHECKED(Script, eval_from_shared_or_wrapped_arguments, Object,
-                  kEvalFromSharedOrWrappedArgumentsOffset,
+ACCESSORS_CHECKED(Script, eval_from_shared_or_wrapped_arguments_or_sfi_table,
+                  Object, kEvalFromSharedOrWrappedArgumentsOrSfiTableOffset,
                   CHECK_SCRIPT_NOT_WASM)
 SMI_ACCESSORS_CHECKED(Script, eval_from_position, kEvalFromPositionOffset,
                       CHECK_SCRIPT_NOT_WASM)
 #undef CHECK_SCRIPT_NOT_WASM
 
 bool Script::is_wrapped() const {
-  return eval_from_shared_or_wrapped_arguments().IsFixedArray();
+  return eval_from_shared_or_wrapped_arguments_or_sfi_table().IsFixedArray() &&
+         type() != TYPE_WEB_SNAPSHOT;
 }
 
 bool Script::has_eval_from_shared() const {
-  return eval_from_shared_or_wrapped_arguments().IsSharedFunctionInfo();
+  return eval_from_shared_or_wrapped_arguments_or_sfi_table()
+      .IsSharedFunctionInfo();
 }
 
 void Script::set_eval_from_shared(SharedFunctionInfo shared,
                                   WriteBarrierMode mode) {
   DCHECK(!is_wrapped());
-  set_eval_from_shared_or_wrapped_arguments(shared, mode);
+  DCHECK_NE(type(), TYPE_WEB_SNAPSHOT);
+  set_eval_from_shared_or_wrapped_arguments_or_sfi_table(shared, mode);
 }
 
 SharedFunctionInfo Script::eval_from_shared() const {
   DCHECK(has_eval_from_shared());
-  return SharedFunctionInfo::cast(eval_from_shared_or_wrapped_arguments());
+  return SharedFunctionInfo::cast(
+      eval_from_shared_or_wrapped_arguments_or_sfi_table());
 }
 
 void Script::set_wrapped_arguments(FixedArray value, WriteBarrierMode mode) {
   DCHECK(!has_eval_from_shared());
-  set_eval_from_shared_or_wrapped_arguments(value, mode);
+  DCHECK_NE(type(), TYPE_WEB_SNAPSHOT);
+  set_eval_from_shared_or_wrapped_arguments_or_sfi_table(value, mode);
 }
 
 FixedArray Script::wrapped_arguments() const {
   DCHECK(is_wrapped());
-  return FixedArray::cast(eval_from_shared_or_wrapped_arguments());
+  return FixedArray::cast(eval_from_shared_or_wrapped_arguments_or_sfi_table());
+}
+
+void Script::set_shared_function_info_table(ObjectHashTable value,
+                                            WriteBarrierMode mode) {
+  DCHECK(!has_eval_from_shared());
+  DCHECK(!is_wrapped());
+  DCHECK_EQ(type(), TYPE_WEB_SNAPSHOT);
+  set_eval_from_shared_or_wrapped_arguments_or_sfi_table(value, mode);
+}
+
+ObjectHashTable Script::shared_function_info_table() const {
+  DCHECK_EQ(type(), TYPE_WEB_SNAPSHOT);
+  return ObjectHashTable::cast(
+      eval_from_shared_or_wrapped_arguments_or_sfi_table());
 }
 
 DEF_GETTER(Script, shared_function_infos, WeakFixedArray) {
@@ -91,6 +110,15 @@ void Script::set_shared_function_infos(WeakFixedArray value,
   CONDITIONAL_WRITE_BARRIER(*this, kSharedFunctionInfosOffset, value, mode);
 }
 
+int Script::shared_function_info_count() const {
+  if V8_UNLIKELY (type() == TYPE_WEB_SNAPSHOT) {
+    // +1 because the 0th element in shared_function_infos is reserved for the
+    // top-level SharedFunctionInfo which doesn't exist.
+    return shared_function_info_table().NumberOfElements() + 1;
+  }
+  return shared_function_infos().length();
+}
+
 #if V8_ENABLE_WEBASSEMBLY
 bool Script::has_wasm_breakpoint_infos() const {
   return type() == TYPE_WASM && wasm_breakpoint_infos().length() > 0;

src/objects/script.h

@@ -20,6 +20,8 @@ namespace v8 {
 
 namespace internal {
 
+class FunctionLiteral;
+
 #include "torque-generated/src/objects/script-tq.inc"
 
 // Script describes a script which has been added to the VM.
@@ -38,7 +40,8 @@ class Script : public TorqueGeneratedScript<Script, Struct> {
 #if V8_ENABLE_WEBASSEMBLY
     TYPE_WASM = 3,
 #endif  // V8_ENABLE_WEBASSEMBLY
-    TYPE_INSPECTOR = 4
+    TYPE_INSPECTOR = 4,
+    TYPE_WEB_SNAPSHOT = 5
   };
 
   // Script compilation types.
@@ -53,7 +56,7 @@ class Script : public TorqueGeneratedScript<Script, Struct> {
   // [type]: the script type.
   DECL_INT_ACCESSORS(type)
 
-  DECL_ACCESSORS(eval_from_shared_or_wrapped_arguments, Object)
+  DECL_ACCESSORS(eval_from_shared_or_wrapped_arguments_or_sfi_table, Object)
 
   // [eval_from_shared]: for eval scripts the shared function info for the
   // function from which eval was called.
@@ -62,6 +65,12 @@ class Script : public TorqueGeneratedScript<Script, Struct> {
   // [wrapped_arguments]: for the list of arguments in a wrapped script.
   DECL_ACCESSORS(wrapped_arguments, FixedArray)
 
+  // For web snapshots: a hash table mapping function positions to indices in
+  // shared_function_infos.
+  // TODO(v8:11525): Replace with a more efficient data structure mapping
+  // function positions to weak pointers to SharedFunctionInfos directly.
+  DECL_ACCESSORS(shared_function_info_table, ObjectHashTable)
+
   // Whether the script is implicitly wrapped in a function.
   inline bool is_wrapped() const;
 
@@ -78,6 +87,8 @@ class Script : public TorqueGeneratedScript<Script, Struct> {
   // function infos created from this script.
   DECL_ACCESSORS(shared_function_infos, WeakFixedArray)
 
+  inline int shared_function_info_count() const;
+
 #if V8_ENABLE_WEBASSEMBLY
   // [wasm_breakpoint_infos]: the list of {BreakPointInfo} objects describing
   // all WebAssembly breakpoints for modules/instances managed via this script.
@@ -176,10 +187,19 @@ class Script : public TorqueGeneratedScript<Script, Struct> {
   int GetLineNumber(int code_pos) const;
 
   // Look through the list of existing shared function infos to find one
-  // that matches the function literal.  Return empty handle if not found.
+  // that matches the function literal. Return empty handle if not found.
   template <typename LocalIsolate>
-  MaybeHandle<SharedFunctionInfo> FindSharedFunctionInfo(
-      LocalIsolate* isolate, int function_literal_id);
+  static MaybeHandle<SharedFunctionInfo> FindSharedFunctionInfo(
+      Handle<Script> script, LocalIsolate* isolate,
+      FunctionLiteral* function_literal);
+
+  static MaybeHandle<SharedFunctionInfo> FindWebSnapshotSharedFunctionInfo(
+      Handle<Script> script, Isolate* isolate,
+      FunctionLiteral* function_literal);
+
+  static MaybeHandle<SharedFunctionInfo> FindWebSnapshotSharedFunctionInfo(
+      Handle<Script> script, LocalIsolate* isolate,
+      FunctionLiteral* function_literal);
 
   // Iterate over all script objects on the heap.
   class V8_EXPORT_PRIVATE Iterator {

src/objects/script.tq

@@ -40,8 +40,12 @@ extern class Script extends Struct {
   // [id]: the script id.
   id: Smi;
 
-  eval_from_shared_or_wrapped_arguments: SharedFunctionInfo|FixedArray|
-      Undefined;
+  // For scripts originating from eval: the SharedFunctionInfo contains the SFI
+  // for the script. For scripts wrapped as functions: the FixedArray contains
+  // the arguments. For web snapshots: the ObjectHashTable maps function start
+  // position to SFI index in shared_function_infos.
+  eval_from_shared_or_wrapped_arguments_or_sfi_table: SharedFunctionInfo|
+      FixedArray|ObjectHashTable|Undefined;
   eval_from_position: Smi|Foreign;  // Smi or Managed<wasm::NativeModule>
   shared_function_infos: WeakFixedArray|WeakArrayList;
 

src/parsing/parser.cc

@@ -833,14 +833,23 @@ void Parser::ParseFunction(Isolate* isolate, ParseInfo* info,
                         Scope::DeserializationMode::kIncludingVariables);
   DCHECK_EQ(factory()->zone(), info->zone());
 
+  Handle<Script> script = handle(Script::cast(shared_info->script()), isolate);
   if (shared_info->is_wrapped()) {
-    maybe_wrapped_arguments_ = handle(
-        Script::cast(shared_info->script()).wrapped_arguments(), isolate);
+    maybe_wrapped_arguments_ = handle(script->wrapped_arguments(), isolate);
   }
 
   int start_position = shared_info->StartPosition();
   int end_position = shared_info->EndPosition();
   int function_literal_id = shared_info->function_literal_id();
+  if V8_UNLIKELY (script->type() == Script::TYPE_WEB_SNAPSHOT) {
+    // Function literal IDs for inner functions haven't been allocated when
+    // deserializing. Put the inner function SFIs to the end of the list;
+    // they'll be deduplicated later (if the corresponding SFIs exist already)
+    // in Script::FindSharedFunctionInfo. (-1 here because function_literal_id
+    // is the parent's id. The inner function will get ids starting from
+    // function_literal_id + 1.)
+    function_literal_id = script->shared_function_info_count() - 1;
+  }
 
   // Initialize parser state.
   Handle<String> name(shared_info->Name(), isolate);
@@ -865,9 +874,10 @@ void Parser::ParseFunction(Isolate* isolate, ParseInfo* info,
   if (result != nullptr) {
     Handle<String> inferred_name(shared_info->inferred_name(), isolate);
     result->set_inferred_name(inferred_name);
+    // Fix the function_literal_id in case we changed it earlier.
+    result->set_function_literal_id(shared_info->function_literal_id());
   }
   PostProcessParseResult(isolate, info, result);
-
   if (V8_UNLIKELY(FLAG_log_function_events) && result != nullptr) {
     double ms = timer.Elapsed().InMillisecondsF();
     // We should already be internalized by now, so the debug name will be

src/web-snapshot/web-snapshot.cc

@@ -238,7 +238,10 @@ void WebSnapshotSerializer::SerializeMap(Handle<Map> map, uint32_t& id) {
 
 // Format (serialized function):
 // - 0 if there's no context, 1 + context id otherwise
-// - String id (source string)
+// - String id (source snippet)
+// - Start position in the source snippet
+// - Length in the source snippet
+// TODO(v8:11525): Investigate whether the length is really needed.
 void WebSnapshotSerializer::SerializeFunction(Handle<JSFunction> function,
                                               uint32_t& id) {
   if (InsertIntoIndexMap(function_ids_, function, id)) {
@@ -251,7 +254,7 @@ void WebSnapshotSerializer::SerializeFunction(Handle<JSFunction> function,
   }
 
   Handle<Context> context(function->context(), isolate_);
-  if (context->IsNativeContext()) {
+  if (context->IsNativeContext() || context->IsScriptContext()) {
     function_serializer_.WriteUint32(0);
   } else {
     DCHECK(context->IsFunctionContext());
@@ -260,20 +263,19 @@ void WebSnapshotSerializer::SerializeFunction(Handle<JSFunction> function,
     function_serializer_.WriteUint32(context_id + 1);
   }
 
-  // TODO(v8:11525): For inner functions which occur inside a serialized
-  // function, create a "substring" type, so that we don't need to serialize the
-  // same content twice.
+  // TODO(v8:11525): Don't write the full source but instead, a set of minimal
+  // snippets which cover the serialized functions.
   Handle<String> full_source(
       String::cast(Script::cast(function->shared().script()).source()),
       isolate_);
-  int start = function->shared().StartPosition();
-  int end = function->shared().EndPosition();
-  Handle<String> source =
-      isolate_->factory()->NewSubString(full_source, start, end);
   uint32_t source_id = 0;
-  SerializeString(source, source_id);
+  SerializeString(full_source, source_id);
   function_serializer_.WriteUint32(source_id);
 
+  int start = function->shared().StartPosition();
+  function_serializer_.WriteUint32(start);
+  int end = function->shared().EndPosition();
+  function_serializer_.WriteUint32(end - start);
   // TODO(v8:11525): Serialize .prototype.
   // TODO(v8:11525): Support properties in functions.
 }
@@ -297,7 +299,8 @@ void WebSnapshotSerializer::SerializeContext(Handle<Context> context,
   }
 
   uint32_t parent_context_id = 0;
-  if (!context->previous().IsNativeContext()) {
+  if (!context->previous().IsNativeContext() &&
+      !context->previous().IsScriptContext()) {
     SerializeContext(handle(context->previous(), isolate_), parent_context_id);
     ++parent_context_id;
   }
@@ -669,8 +672,21 @@ void WebSnapshotDeserializer::DeserializeFunctions() {
     Throw("Web snapshot: Malformed function table");
     return;
   }
-  STATIC_ASSERT(kMaxItemCount <= FixedArray::kMaxLength);
+  STATIC_ASSERT(kMaxItemCount + 1 <= FixedArray::kMaxLength);
   functions_ = isolate_->factory()->NewFixedArray(function_count_);
+
+  Handle<Script> script =
+      isolate_->factory()->NewScript(isolate_->factory()->empty_string());
+  script->set_type(Script::TYPE_WEB_SNAPSHOT);
+  // Overallocate the array for SharedFunctionInfos; functions which we
+  // deserialize soon will create more SharedFunctionInfos when called.
+  Handle<WeakFixedArray> infos(isolate_->factory()->NewWeakFixedArray(
+      WeakArrayList::CapacityForLength(function_count_ + 1),
+      AllocationType::kOld));
+  script->set_shared_function_infos(*infos);
+  Handle<ObjectHashTable> shared_function_info_table =
+      ObjectHashTable::New(isolate_, function_count_);
+
   for (uint32_t i = 0; i < function_count_; ++i) {
     uint32_t context_id;
     // Note: > (not >= on purpose, we will subtract 1).
@@ -681,10 +697,24 @@ void WebSnapshotDeserializer::DeserializeFunctions() {
     }
 
     Handle<String> source = ReadString(false);
+    if (i == 0) {
+      script->set_source(*source);
+    } else {
+      // TODO(v8:11525): Support multiple source snippets.
+      DCHECK_EQ(script->source(), *source);
+    }
+
+    uint32_t start_position;
+    uint32_t length;
+    if (!deserializer_->ReadUint32(&start_position) ||
+        !deserializer_->ReadUint32(&length)) {
+      Throw("Web snapshot: Malformed function");
+      return;
+    }
 
     // TODO(v8:11525): Support other function kinds.
     // TODO(v8:11525): Support (exported) top level functions.
-    Handle<Script> script = isolate_->factory()->NewScript(source);
+
     // TODO(v8:11525): Deduplicate the SFIs for inner functions the user creates
     // post-deserialization (by calling the outer function, if it's also in the
     // snapshot) against the ones we create here.
@@ -692,18 +722,24 @@ void WebSnapshotDeserializer::DeserializeFunctions() {
         isolate_->factory()->NewSharedFunctionInfo(
             isolate_->factory()->empty_string(), MaybeHandle<Code>(),
             Builtins::kCompileLazy, FunctionKind::kNormalFunction);
-    shared->set_function_literal_id(1);
+    shared->set_script(*script);
+    // Index 0 is reserved for top-level shared function info (which web
+    // snapshot scripts don't have).
+    const int shared_function_info_index = i + 1;
+    shared->set_function_literal_id(shared_function_info_index);
     // TODO(v8:11525): Decide how to handle language modes.
     shared->set_language_mode(LanguageMode::kStrict);
     shared->set_uncompiled_data(
         *isolate_->factory()->NewUncompiledDataWithoutPreparseData(
-            ReadOnlyRoots(isolate_).empty_string_handle(), 0,
-            source->length()));
-    shared->set_script(*script);
-    Handle<WeakFixedArray> infos(
-        isolate_->factory()->NewWeakFixedArray(3, AllocationType::kOld));
-    infos->Set(1, HeapObjectReference::Weak(*shared));
-    script->set_shared_function_infos(*infos);
+            ReadOnlyRoots(isolate_).empty_string_handle(), start_position,
+            start_position + length));
+    shared->set_allows_lazy_compilation(true);
+    infos->Set(shared_function_info_index, HeapObjectReference::Weak(*shared));
+
+    shared_function_info_table = ObjectHashTable::Put(
+        shared_function_info_table,
+        handle(Smi::FromInt(start_position), isolate_),
+        handle(Smi::FromInt(shared_function_info_index), isolate_));
 
     Handle<JSFunction> function =
         Factory::JSFunctionBuilder(isolate_, shared, isolate_->native_context())
@@ -718,6 +754,7 @@ void WebSnapshotDeserializer::DeserializeFunctions() {
     }
     functions_->set(i, *function);
   }
+  script->set_shared_function_info_table(*shared_function_info_table);
 }
 
 void WebSnapshotDeserializer::DeserializeObjects() {

src/web-snapshot/web-snapshot.h

@@ -42,7 +42,7 @@ class WebSnapshotSerializerDeserializer {
 
   // The maximum count of items for each value type (strings, objects etc.)
   static constexpr uint32_t kMaxItemCount =
-      static_cast<uint32_t>(FixedArray::kMaxLength);
+      static_cast<uint32_t>(FixedArray::kMaxLength - 1);
   // This ensures indices and lengths can be converted between uint32_t and int
   // without problems:
   STATIC_ASSERT(kMaxItemCount < std::numeric_limits<int32_t>::max());

test/cctest/test-web-snapshots.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "src/api/api-inl.h"
 #include "src/web-snapshot/web-snapshot.h"
 #include "test/cctest/cctest-utils.h"
 #include "test/cctest/cctest.h"
@@ -17,16 +18,18 @@ void TestWebSnapshot(const char* snapshot_source, const char* test_source,
                      uint32_t function_count, uint32_t object_count) {
   CcTest::InitializeVM();
   v8::Isolate* isolate = CcTest::isolate();
-  v8::HandleScope scope(isolate);
-  v8::Local<v8::Context> context = isolate->GetCurrentContext();
 
-  CompileRun(snapshot_source);
   WebSnapshotData snapshot_data;
   {
+    v8::HandleScope scope(isolate);
+    v8::Local<v8::Context> new_context = CcTest::NewContext();
+    v8::Context::Scope context_scope(new_context);
+
+    CompileRun(snapshot_source);
     std::vector<std::string> exports;
     exports.push_back("foo");
     WebSnapshotSerializer serializer(isolate);
-    CHECK(serializer.TakeSnapshot(context, exports, snapshot_data));
+    CHECK(serializer.TakeSnapshot(new_context, exports, snapshot_data));
     CHECK(!serializer.has_error());
     CHECK_NOT_NULL(snapshot_data.buffer);
     CHECK_EQ(string_count, serializer.string_count());
@@ -37,6 +40,7 @@ void TestWebSnapshot(const char* snapshot_source, const char* test_source,
   }
 
   {
+    v8::HandleScope scope(isolate);
     v8::Local<v8::Context> new_context = CcTest::NewContext();
     v8::Context::Scope context_scope(new_context);
     WebSnapshotDeserializer deserializer(isolate);
@@ -127,5 +131,194 @@ TEST(InnerFunctionWithContextAndParentContext) {
                   kMapCount, kContextCount, kFunctionCount, kObjectCount);
 }
 
+TEST(SFIDeduplication) {
+  CcTest::InitializeVM();
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::HandleScope scope(isolate);
+
+  WebSnapshotData snapshot_data;
+  {
+    v8::Local<v8::Context> new_context = CcTest::NewContext();
+    v8::Context::Scope context_scope(new_context);
+    const char* snapshot_source =
+        "let foo = {};\n"
+        "foo.outer = function(a) {\n"
+        "  return function() {\n"
+        "   return a;\n"
+        "  }\n"
+        "}\n"
+        "foo.inner = foo.outer('hi');";
+
+    CompileRun(snapshot_source);
+    std::vector<std::string> exports;
+    exports.push_back("foo");
+    WebSnapshotSerializer serializer(isolate);
+    CHECK(serializer.TakeSnapshot(new_context, exports, snapshot_data));
+    CHECK(!serializer.has_error());
+    CHECK_NOT_NULL(snapshot_data.buffer);
+  }
+
+  {
+    v8::Local<v8::Context> new_context = CcTest::NewContext();
+    v8::Context::Scope context_scope(new_context);
+    WebSnapshotDeserializer deserializer(isolate);
+    CHECK(deserializer.UseWebSnapshot(snapshot_data.buffer,
+                                      snapshot_data.buffer_size));
+    CHECK(!deserializer.has_error());
+
+    const char* get_inner = "foo.inner";
+    const char* create_new_inner = "foo.outer()";
+
+    // Verify that foo.inner and the JSFunction which is the result of calling
+    // foo.outer() after deserialization share the SFI.
+    v8::Local<v8::Function> v8_inner1 =
+        CompileRun(get_inner).As<v8::Function>();
+    v8::Local<v8::Function> v8_inner2 =
+        CompileRun(create_new_inner).As<v8::Function>();
+
+    Handle<JSFunction> inner1 =
+        Handle<JSFunction>::cast(Utils::OpenHandle(*v8_inner1));
+    Handle<JSFunction> inner2 =
+        Handle<JSFunction>::cast(Utils::OpenHandle(*v8_inner2));
+
+    CHECK_EQ(inner1->shared(), inner2->shared());
+  }
+}
+
+TEST(SFIDeduplicationAfterBytecodeFlushing) {
+  FLAG_stress_flush_bytecode = true;
+  CcTest::InitializeVM();
+  v8::Isolate* isolate = CcTest::isolate();
+
+  WebSnapshotData snapshot_data;
+  {
+    v8::HandleScope scope(isolate);
+    v8::Local<v8::Context> new_context = CcTest::NewContext();
+    v8::Context::Scope context_scope(new_context);
+
+    const char* snapshot_source =
+        "let foo = {};\n"
+        "foo.outer = function() {\n"
+        "  let a = 'hello';\n"
+        "  return function() {\n"
+        "   return a;\n"
+        "  }\n"
+        "}\n"
+        "foo.inner = foo.outer();";
+
+    CompileRun(snapshot_source);
+
+    std::vector<std::string> exports;
+    exports.push_back("foo");
+    WebSnapshotSerializer serializer(isolate);
+    CHECK(serializer.TakeSnapshot(new_context, exports, snapshot_data));
+    CHECK(!serializer.has_error());
+    CHECK_NOT_NULL(snapshot_data.buffer);
+  }
+
+  CcTest::CollectAllGarbage();
+  CcTest::CollectAllGarbage();
+
+  {
+    v8::HandleScope scope(isolate);
+    v8::Local<v8::Context> new_context = CcTest::NewContext();
+    v8::Context::Scope context_scope(new_context);
+    WebSnapshotDeserializer deserializer(isolate);
+    CHECK(deserializer.UseWebSnapshot(snapshot_data.buffer,
+                                      snapshot_data.buffer_size));
+    CHECK(!deserializer.has_error());
+
+    const char* get_outer = "foo.outer";
+    const char* get_inner = "foo.inner";
+    const char* create_new_inner = "foo.outer()";
+
+    v8::Local<v8::Function> v8_outer = CompileRun(get_outer).As<v8::Function>();
+    Handle<JSFunction> outer =
+        Handle<JSFunction>::cast(Utils::OpenHandle(*v8_outer));
+    CHECK(!outer->shared().is_compiled());
+
+    v8::Local<v8::Function> v8_inner1 =
+        CompileRun(get_inner).As<v8::Function>();
+    v8::Local<v8::Function> v8_inner2 =
+        CompileRun(create_new_inner).As<v8::Function>();
+
+    Handle<JSFunction> inner1 =
+        Handle<JSFunction>::cast(Utils::OpenHandle(*v8_inner1));
+    Handle<JSFunction> inner2 =
+        Handle<JSFunction>::cast(Utils::OpenHandle(*v8_inner2));
+
+    CHECK(outer->shared().is_compiled());
+    CHECK_EQ(inner1->shared(), inner2->shared());
+
+    // Force bytecode flushing of "foo.outer".
+    CcTest::CollectAllGarbage();
+    CcTest::CollectAllGarbage();
+
+    CHECK(!outer->shared().is_compiled());
+
+    // Create another inner function.
+    v8::Local<v8::Function> v8_inner3 =
+        CompileRun(create_new_inner).As<v8::Function>();
+    Handle<JSFunction> inner3 =
+        Handle<JSFunction>::cast(Utils::OpenHandle(*v8_inner3));
+
+    // Check that it shares the SFI with the original inner function which is in
+    // the snapshot.
+    CHECK_EQ(inner1->shared(), inner3->shared());
+  }
+}
+
+TEST(SFIDeduplicationOfFunctionsNotInSnapshot) {
+  CcTest::InitializeVM();
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::HandleScope scope(isolate);
+
+  WebSnapshotData snapshot_data;
+  {
+    v8::Local<v8::Context> new_context = CcTest::NewContext();
+    v8::Context::Scope context_scope(new_context);
+    const char* snapshot_source =
+        "let foo = {};\n"
+        "foo.outer = function(a) {\n"
+        "  return function() {\n"
+        "   return a;\n"
+        "  }\n"
+        "}\n";
+
+    CompileRun(snapshot_source);
+    std::vector<std::string> exports;
+    exports.push_back("foo");
+    WebSnapshotSerializer serializer(isolate);
+    CHECK(serializer.TakeSnapshot(new_context, exports, snapshot_data));
+    CHECK(!serializer.has_error());
+    CHECK_NOT_NULL(snapshot_data.buffer);
+  }
+
+  {
+    v8::Local<v8::Context> new_context = CcTest::NewContext();
+    v8::Context::Scope context_scope(new_context);
+    WebSnapshotDeserializer deserializer(isolate);
+    CHECK(deserializer.UseWebSnapshot(snapshot_data.buffer,
+                                      snapshot_data.buffer_size));
+    CHECK(!deserializer.has_error());
+
+    const char* create_new_inner = "foo.outer()";
+
+    // Verify that repeated invocations of foo.outer() return functions which
+    // share the SFI.
+    v8::Local<v8::Function> v8_inner1 =
+        CompileRun(create_new_inner).As<v8::Function>();
+    v8::Local<v8::Function> v8_inner2 =
+        CompileRun(create_new_inner).As<v8::Function>();
+
+    Handle<JSFunction> inner1 =
+        Handle<JSFunction>::cast(Utils::OpenHandle(*v8_inner1));
+    Handle<JSFunction> inner2 =
+        Handle<JSFunction>::cast(Utils::OpenHandle(*v8_inner2));
+
+    CHECK_EQ(inner1->shared(), inner2->shared());
+  }
+}
+
 }  // namespace internal
 }  // namespace v8