[tracing] Speed up SharedFunctionInfo::TraceID

Avoid the linear lookup of function literal id when getting the shared
function info TraceID, by optionally passing through a FunctionLiteral.
Additionally, use the FunctionLiteralId helper when a FunctionLiteral is
not available, since it can also fast-path in some cases.

As a drive-by, allow using a ScriptIterator without an Isolate pointer
(e.g. manually creating a handle) to allow calling FunctionLiteralId
without an Isolate pointer.

Bug: v8:9325
Change-Id: Ibfa053f300d6d5005485c67174a848264a5d1372
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1643429
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61983}

src/debug/liveedit.cc

@@ -1124,8 +1124,7 @@ void LiveEdit::PatchScript(Isolate* isolate, Handle<Script> script,
     }
     new_script->shared_function_infos().Set(
         mapping.second->function_literal_id(), HeapObjectReference::Weak(*sfi));
-    DCHECK_EQ(sfi->FunctionLiteralId(isolate),
-              mapping.second->function_literal_id());
+    DCHECK_EQ(sfi->FunctionLiteralId(), mapping.second->function_literal_id());
 
     // Save the new start_position -> id mapping, so that we can recover it when
     // iterating over changed functions' constant pools.
@@ -1222,7 +1221,7 @@ void LiveEdit::PatchScript(Isolate* isolate, Handle<Script> script,
     std::set<int> start_positions;
     for (SharedFunctionInfo sfi = it.Next(); !sfi.is_null(); sfi = it.Next()) {
       DCHECK_EQ(sfi.script(), *new_script);
-      DCHECK_EQ(sfi.FunctionLiteralId(isolate), it.CurrentIndex());
+      DCHECK_EQ(sfi.FunctionLiteralId(), it.CurrentIndex());
       // Don't check the start position of the top-level function, as it can
       // overlap with a function in the script.
       if (sfi.is_toplevel()) {
@@ -1242,7 +1241,7 @@ void LiveEdit::PatchScript(Isolate* isolate, Handle<Script> script,
             SharedFunctionInfo::cast(constants.get(i));
         DCHECK_EQ(inner_sfi.script(), *new_script);
         DCHECK_EQ(inner_sfi, new_script->shared_function_infos()
-                                 .Get(inner_sfi.FunctionLiteralId(isolate))
+                                 .Get(inner_sfi.FunctionLiteralId())
                                  ->GetHeapObject());
       }
     }

src/heap/factory.cc

@@ -3335,10 +3335,12 @@ Handle<SharedFunctionInfo> Factory::NewSharedFunctionInfoForLiteral(
                                 false);
   TRACE_EVENT_OBJECT_CREATED_WITH_ID(
       TRACE_DISABLED_BY_DEFAULT("v8.compile"), "SharedFunctionInfo",
-      TRACE_ID_WITH_SCOPE(SharedFunctionInfo::kTraceScope, shared->TraceID()));
+      TRACE_ID_WITH_SCOPE(SharedFunctionInfo::kTraceScope,
+                          shared->TraceID(literal)));
   TRACE_EVENT_OBJECT_SNAPSHOT_WITH_ID(
       TRACE_DISABLED_BY_DEFAULT("v8.compile"), "SharedFunctionInfo",
-      TRACE_ID_WITH_SCOPE(SharedFunctionInfo::kTraceScope, shared->TraceID()),
+      TRACE_ID_WITH_SCOPE(SharedFunctionInfo::kTraceScope,
+                          shared->TraceID(literal)),
       shared->ToTracedValue(literal));
   return shared;
 }

src/objects/objects.cc

@@ -4855,22 +4855,12 @@ std::unique_ptr<v8::tracing::TracedValue> SharedFunctionInfo::ToTracedValue(
 const char* SharedFunctionInfo::kTraceScope =
     "v8::internal::SharedFunctionInfo";
 
-uint64_t SharedFunctionInfo::TraceID() const {
-  // TODO(bmeurer): We use a combination of Script ID and function literal
-  // ID (within the Script) to uniquely identify SharedFunctionInfos. This
-  // can add significant overhead, and we should probably find a better way
-  // to uniquely identify SharedFunctionInfos over time.
+uint64_t SharedFunctionInfo::TraceID(FunctionLiteral* literal) const {
+  int literal_id =
+      literal ? literal->function_literal_id() : FunctionLiteralId();
   Script script = Script::cast(this->script());
-  WeakFixedArray script_functions = script.shared_function_infos();
-  for (int i = 0; i < script_functions.length(); ++i) {
-    HeapObject script_function;
-    if (script_functions.Get(i).GetHeapObjectIfWeak(&script_function) &&
-        script_function.address() == address()) {
-      return (static_cast<uint64_t>(script.id() + 1) << 32) |
-             (static_cast<uint64_t>(i));
-    }
-  }
-  UNREACHABLE();
+  return (static_cast<uint64_t>(script.id() + 1) << 32) |
+         (static_cast<uint64_t>(literal_id));
 }
 
 std::unique_ptr<v8::tracing::TracedValue> SharedFunctionInfo::TraceIDRef()
@@ -4946,21 +4936,17 @@ WasmCapiFunctionData SharedFunctionInfo::wasm_capi_function_data() const {
 
 SharedFunctionInfo::ScriptIterator::ScriptIterator(Isolate* isolate,
                                                    Script script)
-    : ScriptIterator(isolate, handle(script.shared_function_infos(), isolate)) {
-}
+    : ScriptIterator(handle(script.shared_function_infos(), isolate)) {}
 
 SharedFunctionInfo::ScriptIterator::ScriptIterator(
-    Isolate* isolate, Handle<WeakFixedArray> shared_function_infos)
-    : isolate_(isolate),
-      shared_function_infos_(shared_function_infos),
-      index_(0) {}
+    Handle<WeakFixedArray> shared_function_infos)
+    : shared_function_infos_(shared_function_infos), index_(0) {}
 
 SharedFunctionInfo SharedFunctionInfo::ScriptIterator::Next() {
   while (index_ < shared_function_infos_->length()) {
     MaybeObject raw = shared_function_infos_->Get(index_++);
     HeapObject heap_object;
-    if (!raw->GetHeapObject(&heap_object) ||
-        heap_object.IsUndefined(isolate_)) {
+    if (!raw->GetHeapObject(&heap_object) || heap_object.IsUndefined()) {
       continue;
     }
     return SharedFunctionInfo::cast(heap_object);
@@ -4968,13 +4954,15 @@ SharedFunctionInfo SharedFunctionInfo::ScriptIterator::Next() {
   return SharedFunctionInfo();
 }
 
-void SharedFunctionInfo::ScriptIterator::Reset(Script script) {
-  shared_function_infos_ = handle(script.shared_function_infos(), isolate_);
+void SharedFunctionInfo::ScriptIterator::Reset(Isolate* isolate,
+                                               Script script) {
+  shared_function_infos_ = handle(script.shared_function_infos(), isolate);
   index_ = 0;
 }
 
 SharedFunctionInfo::GlobalIterator::GlobalIterator(Isolate* isolate)
-    : script_iterator_(isolate),
+    : isolate_(isolate),
+      script_iterator_(isolate),
       noscript_sfi_iterator_(isolate->heap()->noscript_shared_function_infos()),
       sfi_iterator_(isolate, script_iterator_.Next()) {}
 
@@ -4986,7 +4974,7 @@ SharedFunctionInfo SharedFunctionInfo::GlobalIterator::Next() {
     if (!next.is_null()) return SharedFunctionInfo::cast(next);
     Script next_script = script_iterator_.Next();
     if (next_script.is_null()) return SharedFunctionInfo();
-    sfi_iterator_.Reset(next_script);
+    sfi_iterator_.Reset(isolate_, next_script);
   }
 }
 
@@ -5148,7 +5136,7 @@ void SharedFunctionInfo::DiscardCompiled(
       handle(shared_info->inferred_name(), isolate);
   int start_position = shared_info->StartPosition();
   int end_position = shared_info->EndPosition();
-  int function_literal_id = shared_info->FunctionLiteralId(isolate);
+  int function_literal_id = shared_info->FunctionLiteralId();
 
   shared_info->DiscardCompiledMetadata(isolate);
 
@@ -5273,7 +5261,7 @@ bool SharedFunctionInfo::IsInlineable() {
 
 int SharedFunctionInfo::SourceSize() { return EndPosition() - StartPosition(); }
 
-int SharedFunctionInfo::FindIndexInScript(Isolate* isolate) const {
+int SharedFunctionInfo::FindIndexInScript() const {
   DisallowHeapAllocation no_gc;
 
   Object script_obj = script();
@@ -5282,7 +5270,6 @@ int SharedFunctionInfo::FindIndexInScript(Isolate* isolate) const {
   WeakFixedArray shared_info_list =
       Script::cast(script_obj).shared_function_infos();
   SharedFunctionInfo::ScriptIterator iterator(
-      isolate,
       Handle<WeakFixedArray>(reinterpret_cast<Address*>(&shared_info_list)));
 
   for (SharedFunctionInfo shared = iterator.Next(); !shared.is_null();
@@ -5510,19 +5497,19 @@ int SharedFunctionInfo::EndPosition() const {
   return kNoSourcePosition;
 }
 
-int SharedFunctionInfo::FunctionLiteralId(Isolate* isolate) const {
+int SharedFunctionInfo::FunctionLiteralId() const {
   // Fast path for the common case when the SFI is uncompiled and so the
   // function literal id is already in the uncompiled data.
   if (HasUncompiledData() && uncompiled_data().has_function_literal_id()) {
     int id = uncompiled_data().function_literal_id();
     // Make sure the id is what we should have found with the slow path.
-    DCHECK_EQ(id, FindIndexInScript(isolate));
+    DCHECK_EQ(id, FindIndexInScript());
     return id;
   }
 
   // Otherwise, search for the function in the SFI's script's function list,
   // and return its index in that list.
-  return FindIndexInScript(isolate);
+  return FindIndexInScript();
 }
 
 void SharedFunctionInfo::SetPosition(int start_position, int end_position) {

src/objects/shared-function-info.h

@@ -386,7 +386,7 @@ class SharedFunctionInfo : public HeapObject {
   inline String inferred_name();
 
   // Get the function literal id associated with this function, for parsing.
-  V8_EXPORT_PRIVATE int FunctionLiteralId(Isolate* isolate) const;
+  V8_EXPORT_PRIVATE int FunctionLiteralId() const;
 
   // Break infos are contained in DebugInfo, this is a convenience method
   // to simplify access.
@@ -624,7 +624,7 @@ class SharedFunctionInfo : public HeapObject {
   // Returns the unique TraceID for this SharedFunctionInfo (within the
   // kTraceScope, works only for functions that have a Script and start/end
   // position).
-  uint64_t TraceID() const;
+  uint64_t TraceID(FunctionLiteral* literal = nullptr) const;
 
   // Returns the unique trace ID reference for this SharedFunctionInfo
   // (based on the |TraceID()| above).
@@ -634,16 +634,14 @@ class SharedFunctionInfo : public HeapObject {
   class ScriptIterator {
    public:
     V8_EXPORT_PRIVATE ScriptIterator(Isolate* isolate, Script script);
-    ScriptIterator(Isolate* isolate,
-                   Handle<WeakFixedArray> shared_function_infos);
+    explicit ScriptIterator(Handle<WeakFixedArray> shared_function_infos);
     V8_EXPORT_PRIVATE SharedFunctionInfo Next();
     int CurrentIndex() const { return index_ - 1; }
 
     // Reset the iterator to run on |script|.
-    void Reset(Script script);
+    void Reset(Isolate* isolate, Script script);
 
    private:
-    Isolate* isolate_;
     Handle<WeakFixedArray> shared_function_infos_;
     int index_;
     DISALLOW_COPY_AND_ASSIGN(ScriptIterator);
@@ -656,6 +654,7 @@ class SharedFunctionInfo : public HeapObject {
     V8_EXPORT_PRIVATE SharedFunctionInfo Next();
 
    private:
+    Isolate* isolate_;
     Script::Iterator script_iterator_;
     WeakArrayList::Iterator noscript_sfi_iterator_;
     SharedFunctionInfo::ScriptIterator sfi_iterator_;
@@ -746,7 +745,7 @@ class SharedFunctionInfo : public HeapObject {
 
   // Find the index of this function in the parent script. Slow path of
   // FunctionLiteralId.
-  int FindIndexInScript(Isolate* isolate) const;
+  int FindIndexInScript() const;
 
   OBJECT_CONSTRUCTORS(SharedFunctionInfo, HeapObject);
 };

src/parsing/parse-info.cc

@@ -97,7 +97,7 @@ ParseInfo::ParseInfo(Isolate* isolate, Handle<SharedFunctionInfo> shared)
 
   set_start_position(shared->StartPosition());
   set_end_position(shared->EndPosition());
-  function_literal_id_ = shared->FunctionLiteralId(isolate);
+  function_literal_id_ = shared->FunctionLiteralId();
   SetFunctionInfo(shared);
 
   Handle<Script> script(Script::cast(shared->script()), isolate);

test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc

@@ -94,7 +94,7 @@ class CompilerDispatcherTest : public TestWithNativeContext {
             FunctionLiteral::kNoDuplicateParameters,
             FunctionLiteral::kAnonymousExpression,
             FunctionLiteral::kShouldEagerCompile, shared->StartPosition(), true,
-            shared->FunctionLiteralId(isolate), nullptr);
+            shared->FunctionLiteralId(), nullptr);
 
     return dispatcher->Enqueue(outer_parse_info.get(), function_name,
                                function_literal);

test/unittests/tasks/background-compile-task-unittest.cc

@@ -73,7 +73,7 @@ class BackgroundCompileTaskTest : public TestWithNativeContext {
             FunctionLiteral::kNoDuplicateParameters,
             FunctionLiteral::kAnonymousExpression,
             FunctionLiteral::kShouldEagerCompile, shared->StartPosition(), true,
-            shared->FunctionLiteralId(isolate), nullptr);
+            shared->FunctionLiteralId(), nullptr);
 
     return new BackgroundCompileTask(
         allocator(), outer_parse_info.get(), function_name, function_literal,