[stack-traces] Speed up method name inference.

In JSStackFrame::GetMethodName() we try to infer a useful method name to
show for the closure to which the stack frame belongs. This is done by
first considering the functions name, and checking if the receiver has a
property with that name and if that property's value is the closure. In
case the function doesn't have a name or the property's value is not the
closure itself, we fall back to a reverse lookup of the closure within
the object (and its prototypes).

This CL speeds up this logic by attacking two problems:

1. The reverse lookup was performed by first using the KeyAccumulator to
   extract the names of all enumerable properties, and afterwards using
   the LookupIterator on each name, and testing the resulting property
   value against the closure. This is fairly slow and creates a lot of
   temporary objects and handles. We now look into the descriptor arrays
   or dictionary backing stores of the objects directly instead, which
   is easily 2-10x faster.
2. For the common case of `o.foo = function() { ... }` the parser already
   places an "inferred name" of `o.foo` onto the SharedFunctionInfo,
   which we can use as a hint to infer the name of the function instead
   of immediately falling back to the expensive reverse lookup.

This repairs the regression reported in http://crbug.com/1069425 and
recovers most of the slowdown reported in http://crbug.com/1077657
(there's still some overhead left from the async stack trace tracking).

Fixed: chromium:1069425
Bug: chromium:1077657
Change-Id: I88d23ccad123906df70c5217e815493106e03ccf
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2676635
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72545}

src/execution/messages.cc

@@ -18,7 +18,6 @@
 #include "src/objects/foreign-inl.h"
 #include "src/objects/frame-array-inl.h"
 #include "src/objects/js-array-inl.h"
-#include "src/objects/keys.h"
 #include "src/objects/stack-frame-info-inl.h"
 #include "src/objects/struct-inl.h"
 #include "src/parsing/parse-info.h"
@@ -389,21 +388,92 @@ Handle<PrimitiveHeapObject> JSStackFrame::GetFunctionName() {
 
 namespace {
 
-bool CheckMethodName(Isolate* isolate, Handle<JSReceiver> receiver,
-                     Handle<Name> name, Handle<JSFunction> fun,
-                     LookupIterator::Configuration config) {
-  LookupIterator::Key key(isolate, name);
-  LookupIterator iter(isolate, receiver, key, config);
-  if (iter.state() == LookupIterator::DATA) {
-    return iter.GetDataValue().is_identical_to(fun);
-  } else if (iter.state() == LookupIterator::ACCESSOR) {
-    Handle<Object> accessors = iter.GetAccessors();
-    if (accessors->IsAccessorPair()) {
-      Handle<AccessorPair> pair = Handle<AccessorPair>::cast(accessors);
-      return pair->getter() == *fun || pair->setter() == *fun;
+PrimitiveHeapObject InferMethodNameFromFastObject(Isolate* isolate,
+                                                  JSObject receiver,
+                                                  JSFunction fun,
+                                                  PrimitiveHeapObject name) {
+  ReadOnlyRoots roots(isolate);
+  Map map = receiver.map();
+  DescriptorArray descriptors = map.instance_descriptors(kRelaxedLoad);
+  for (auto i : map.IterateOwnDescriptors()) {
+    PrimitiveHeapObject key = descriptors.GetKey(i);
+    if (key.IsSymbol()) continue;
+    auto details = descriptors.GetDetails(i);
+    if (details.IsDontEnum()) continue;
+    Object value;
+    if (details.location() == kField) {
+      auto field_index = FieldIndex::ForPropertyIndex(
+          map, details.field_index(), details.representation());
+      if (field_index.is_double()) continue;
+      value = receiver.RawFastPropertyAt(isolate, field_index);
+    } else {
+      value = descriptors.GetStrongValue(i);
+    }
+    if (value != fun) {
+      if (!value.IsAccessorPair()) continue;
+      auto pair = AccessorPair::cast(value);
+      if (pair.getter() != fun && pair.setter() != fun) continue;
+    }
+    if (name != key) {
+      name = name.IsUndefined(isolate) ? key : roots.null_value();
     }
   }
-  return false;
+  return name;
+}
+
+template <typename Dictionary>
+PrimitiveHeapObject InferMethodNameFromDictionary(Isolate* isolate,
+                                                  Dictionary dictionary,
+                                                  JSFunction fun,
+                                                  PrimitiveHeapObject name) {
+  ReadOnlyRoots roots(isolate);
+  for (auto i : dictionary.IterateEntries()) {
+    Object key;
+    if (!dictionary.ToKey(roots, i, &key)) continue;
+    if (key.IsSymbol()) continue;
+    auto details = dictionary.DetailsAt(i);
+    if (details.IsDontEnum()) continue;
+    auto value = dictionary.ValueAt(i);
+    if (value != fun) {
+      if (!value.IsAccessorPair()) continue;
+      auto pair = AccessorPair::cast(value);
+      if (pair.getter() != fun && pair.setter() != fun) continue;
+    }
+    if (name != key) {
+      name = name.IsUndefined(isolate) ? PrimitiveHeapObject::cast(key)
+                                       : roots.null_value();
+    }
+  }
+  return name;
+}
+
+PrimitiveHeapObject InferMethodName(Isolate* isolate, JSReceiver receiver,
+                                    JSFunction fun) {
+  DisallowGarbageCollection no_gc;
+  ReadOnlyRoots roots(isolate);
+  PrimitiveHeapObject name = roots.undefined_value();
+  for (PrototypeIterator it(isolate, receiver, kStartAtReceiver); !it.IsAtEnd();
+       it.Advance()) {
+    auto current = it.GetCurrent();
+    if (!current.IsJSObject()) break;
+    auto object = JSObject::cast(current);
+    if (object.IsAccessCheckNeeded()) break;
+    if (object.HasFastProperties()) {
+      name = InferMethodNameFromFastObject(isolate, object, fun, name);
+    } else if (object.IsJSGlobalObject()) {
+      name = InferMethodNameFromDictionary(
+          isolate, JSGlobalObject::cast(object).global_dictionary(kAcquireLoad),
+          fun, name);
+    } else if (V8_DICT_MODE_PROTOTYPES_BOOL) {
+      name = InferMethodNameFromDictionary(
+          isolate, object.property_dictionary_ordered(), fun, name);
+    } else {
+      name = InferMethodNameFromDictionary(
+          isolate, object.property_dictionary(), fun, name);
+    }
+  }
+  if (name.IsUndefined(isolate)) return roots.null_value();
+  return name;
 }
 
 Handle<Object> ScriptNameOrSourceUrl(Handle<Script> script, Isolate* isolate) {
@@ -446,37 +516,41 @@ Handle<PrimitiveHeapObject> JSStackFrame::GetMethodName() {
   if (name->HasOneBytePrefix(CStrVector("get ")) ||
       name->HasOneBytePrefix(CStrVector("set "))) {
     name = isolate_->factory()->NewProperSubString(name, 4, name->length());
-  }
-  if (CheckMethodName(isolate_, receiver, name, function_,
-                      LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR)) {
-    return name;
+  } else if (name->length() == 0) {
+    // The function doesn't have a meaningful "name" property, however
+    // the parser does store an inferred name "o.foo" for the common
+    // case of `o.foo = function() {...}`, so see if we can derive a
+    // property name to guess from that.
+    name = handle(function_->shared().inferred_name(), isolate_);
+    for (int index = name->length(); --index >= 0;) {
+      if (name->Get(index, isolate_) == '.') {
+        name = isolate_->factory()->NewProperSubString(name, index + 1,
+                                                       name->length());
+        break;
+      }
+    }
   }
 
-  HandleScope outer_scope(isolate_);
-  Handle<PrimitiveHeapObject> result;
-  for (PrototypeIterator iter(isolate_, receiver, kStartAtReceiver);
-       !iter.IsAtEnd(); iter.Advance()) {
-    Handle<Object> current = PrototypeIterator::GetCurrent(iter);
-    if (!current->IsJSObject()) break;
-    Handle<JSObject> current_obj = Handle<JSObject>::cast(current);
-    if (current_obj->IsAccessCheckNeeded()) break;
-    Handle<FixedArray> keys =
-        KeyAccumulator::GetOwnEnumPropertyKeys(isolate_, current_obj);
-    for (int i = 0; i < keys->length(); i++) {
-      HandleScope inner_scope(isolate_);
-      if (!keys->get(i).IsName()) continue;
-      Handle<Name> name_key(Name::cast(keys->get(i)), isolate_);
-      if (!CheckMethodName(isolate_, current_obj, name_key, function_,
-                           LookupIterator::OWN_SKIP_INTERCEPTOR))
-        continue;
-      // Return null in case of duplicates to avoid confusion.
-      if (!result.is_null()) return isolate_->factory()->null_value();
-      result = inner_scope.CloseAndEscape(name_key);
+  if (name->length() != 0) {
+    LookupIterator::Key key(isolate_, Handle<Name>::cast(name));
+    LookupIterator it(isolate_, receiver, key,
+                      LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
+    if (it.state() == LookupIterator::DATA) {
+      if (it.GetDataValue().is_identical_to(function_)) {
+        return name;
+      }
+    } else if (it.state() == LookupIterator::ACCESSOR) {
+      Handle<Object> accessors = it.GetAccessors();
+      if (accessors->IsAccessorPair()) {
+        Handle<AccessorPair> pair = Handle<AccessorPair>::cast(accessors);
+        if (pair->getter() == *function_ || pair->setter() == *function_) {
+          return name;
+        }
+      }
     }
   }
 
-  if (!result.is_null()) return outer_scope.CloseAndEscape(result);
-  return isolate_->factory()->null_value();
+  return handle(InferMethodName(isolate_, *receiver, *function_), isolate_);
 }
 
 Handle<PrimitiveHeapObject> JSStackFrame::GetTypeName() {

test/mjsunit/stack-traces-custom.js

@@ -2,28 +2,43 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// Flags: --allow-natives-syntax
+
+function testMethodNames(o) {
+  try {
+    o.k = 42;
+  } catch (e) {
+    Error.prepareStackTrace = function(e, frames) { return frames; };
+    var frames = e.stack;
+    Error.prepareStackTrace = undefined;
+    assertEquals("f", frames[0].getMethodName());
+    assertEquals(null, frames[1].getMethodName());
+    assertEquals("h1", frames[2].getMethodName());
+    assertEquals("j", frames[3].getMethodName());
+    assertEquals("k", frames[4].getMethodName());
+    assertEquals("testMethodNames", frames[5].getMethodName());
+  }
+}
+
 var o = {
   f: function() { throw new Error(); },
-  get j() { o.h(); },
+  get j() { o.h1(); },
   set k(_) { o.j; },
 };
-o.g1 = function() { o.f() }
-o.g2 = o.g1;
-o.h = function() { o.g1() }
-
-try {
-  o.k = 42;
-} catch (e) {
-  Error.prepareStackTrace = function(e, frames) { return frames; };
-  var frames = e.stack;
-  Error.prepareStackTrace = undefined;
-  assertEquals("f", frames[0].getMethodName());
-  assertEquals(null, frames[1].getMethodName());
-  assertEquals("h", frames[2].getMethodName());
-  assertEquals("j", frames[3].getMethodName());
-  assertEquals("k", frames[4].getMethodName());
-  assertEquals(null, frames[5].getMethodName());
-}
+const duplicate = function() { o.f() }
+o.g1 = duplicate;
+o.g2 = duplicate;
+o.h1 = function() { o.g1() }
+o.h2 = o.h1;
+
+// Test in dictionary mode first.
+assertFalse(%HasFastProperties(o));
+testMethodNames(o);
+
+// Same test but with fast mode object.
+o = %ToFastProperties(o);
+assertTrue(%HasFastProperties(o));
+testMethodNames(o);
 
 function testMethodName(f, frameNumber, expectedName) {
   try {