Bug 1369042 - Optimize @@toStringTag and @@toPrimitive property lookups in the VM. r=evilpie
authorJan de Mooij <jdemooij@mozilla.com>
Fri, 02 Jun 2017 09:06:30 +0200
changeset 361996 5ab80eaba78ccbfe4603899d78ffcd16395322cf
parent 361995 9b2fc8bf9cad366dbded5b56f0f309d3f86d7f81
child 361997 0b3be4de7ac365ed6e79efb4046d013f7d020711
push id31953
push usercbook@mozilla.com
push dateFri, 02 Jun 2017 12:22:33 +0000
treeherdermozilla-central@2a8478029a0c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersevilpie
bugs1369042
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1369042 - Optimize @@toStringTag and @@toPrimitive property lookups in the VM. r=evilpie
js/src/builtin/Object.cpp
js/src/jsobj.cpp
js/src/jsobj.h
js/src/jsobjinlines.h
js/src/vm/NativeObject-inl.h
js/src/vm/Shape.cpp
js/src/vm/Shape.h
js/src/vm/Symbol.h
--- a/js/src/builtin/Object.cpp
+++ b/js/src/builtin/Object.cpp
@@ -502,18 +502,17 @@ js::obj_toString(JSContext* cx, unsigned
             break;
         }
     }
     // Step 14.
     // Currently omitted for non-standard fallback.
 
     // Step 15.
     RootedValue tag(cx);
-    RootedId toStringTagId(cx, SYMBOL_TO_JSID(cx->wellKnownSymbols().toStringTag));
-    if (!GetProperty(cx, obj, obj, toStringTagId, &tag))
+    if (!GetInterestingSymbolProperty(cx, obj, cx->wellKnownSymbols().toStringTag, &tag))
         return false;
 
     // Step 16.
     if (!tag.isString()) {
         // Non-standard (bug 1277801): Use ClassName as a fallback in the interim
         if (!builtinTag) {
             const char* className = GetObjectClassName(cx, obj);
             // "[object Object]" is by far the most common case at this point,
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -3195,19 +3195,18 @@ js::ToPrimitiveSlow(JSContext* cx, JSTyp
     // Step numbers refer to the first algorithm listed in ES6 draft rev 36
     // (2015 Mar 17) 7.1.1 ToPrimitive.
     MOZ_ASSERT(preferredType == JSTYPE_UNDEFINED ||
                preferredType == JSTYPE_STRING ||
                preferredType == JSTYPE_NUMBER);
     RootedObject obj(cx, &vp.toObject());
 
     // Steps 4-5.
-    RootedId id(cx, SYMBOL_TO_JSID(cx->wellKnownSymbols().toPrimitive));
     RootedValue method(cx);
-    if (!GetProperty(cx, obj, obj, id, &method))
+    if (!GetInterestingSymbolProperty(cx, obj, cx->wellKnownSymbols().toPrimitive, &method))
         return false;
 
     // Step 6.
     if (!method.isUndefined()) {
         // Step 6 of GetMethod. js::Call() below would do this check and throw a
         // TypeError anyway, but this produces a better error message.
         if (!IsCallable(method))
             return ReportCantConvert(cx, JSMSG_TOPRIMITIVE_NOT_CALLABLE, obj, preferredType);
@@ -3559,16 +3558,17 @@ JSObject::dump(FILE* fp) const
         const ObjectGroup* group = obj->group();
         fprintf(fp, "group %p\n", (const void*)group);
     }
 
     fprintf(fp, "flags:");
     if (obj->isDelegate()) fprintf(fp, " delegate");
     if (!obj->is<ProxyObject>() && !obj->nonProxyIsExtensible()) fprintf(fp, " not_extensible");
     if (obj->isIndexed()) fprintf(fp, " indexed");
+    if (obj->maybeHasInterestingSymbolProperty()) fprintf(fp, " maybe_has_interesting_symbol");
     if (obj->isBoundFunction()) fprintf(fp, " bound_function");
     if (obj->isQualifiedVarObj()) fprintf(fp, " varobj");
     if (obj->isUnqualifiedVarObj()) fprintf(fp, " unqualified_varobj");
     if (obj->watched()) fprintf(fp, " watched");
     if (obj->isIteratedSingleton()) fprintf(fp, " iterated_singleton");
     if (obj->isNewGroupUnknown()) fprintf(fp, " new_type_unknown");
     if (obj->hasUncacheableProto()) fprintf(fp, " has_uncacheable_proto");
     if (obj->hadElementsAccess()) fprintf(fp, " had_elements_access");
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -270,16 +270,23 @@ class JSObject : public js::gc::Cell
 
     /*
      * Whether there may be indexed properties on this object, excluding any in
      * the object's elements.
      */
     inline bool isIndexed() const;
 
     /*
+     * Whether there may be "interesting symbol" properties on this object. An
+     * interesting symbol is a symbol for which symbol->isInterestingSymbol()
+     * returns true.
+     */
+    MOZ_ALWAYS_INLINE bool maybeHasInterestingSymbolProperty() const;
+
+    /*
      * If this object was instantiated with `new Ctor`, return the constructor's
      * display atom. Otherwise, return nullptr.
      */
     static bool constructorDisplayAtom(JSContext* cx, js::HandleObject obj,
                                        js::MutableHandleAtom name);
 
     /*
      * The same as constructorDisplayAtom above, however if this object has a
@@ -892,16 +899,28 @@ GetPropertyNoGC(JSContext* cx, JSObject*
 }
 
 inline bool
 GetElementNoGC(JSContext* cx, JSObject* obj, const Value& receiver, uint32_t index, Value* vp);
 
 inline bool
 GetElementNoGC(JSContext* cx, JSObject* obj, JSObject* receiver, uint32_t index, Value* vp);
 
+// Returns whether |obj| or an object on its proto chain may have an interesting
+// symbol property (see JSObject::hasInterestingSymbolProperty). If it returns
+// true, *holder is set to the object that may have this property.
+MOZ_ALWAYS_INLINE bool
+MaybeHasInterestingSymbolProperty(JSContext* cx, JSObject* obj, Symbol* symbol,
+                                  JSObject** holder = nullptr);
+
+// Like GetProperty but optimized for interesting symbol properties like
+// @@toStringTag.
+MOZ_ALWAYS_INLINE bool
+GetInterestingSymbolProperty(JSContext* cx, HandleObject obj, Symbol* sym, MutableHandleValue vp);
+
 /*
  * ES6 [[Set]]. Carry out the assignment `obj[id] = v`.
  *
  * The `receiver` argument has to do with how [[Set]] interacts with the
  * prototype chain and proxies. It's hard to explain and ES6 doesn't really
  * try. Long story short, if you just want bog-standard assignment, pass
  * `ObjectValue(*obj)` as receiver. Or better, use one of the signatures that
  * doesn't have a receiver parameter.
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -40,16 +40,38 @@ MaybeConvertUnboxedObjectToNative(JSCont
 {
     if (obj->is<UnboxedPlainObject>())
         return UnboxedPlainObject::convertToNative(cx, obj);
     if (obj->is<UnboxedArrayObject>())
         return UnboxedArrayObject::convertToNative(cx, obj);
     return true;
 }
 
+static MOZ_ALWAYS_INLINE bool
+ClassMayResolveId(const JSAtomState& names, const Class* clasp, jsid id, JSObject* maybeObj)
+{
+    MOZ_ASSERT_IF(maybeObj, maybeObj->getClass() == clasp);
+
+    if (!clasp->getResolve()) {
+        // Sanity check: we should only have a mayResolve hook if we have a
+        // resolve hook.
+        MOZ_ASSERT(!clasp->getMayResolve(), "Class with mayResolve hook but no resolve hook");
+        return false;
+    }
+
+    if (JSMayResolveOp mayResolve = clasp->getMayResolve()) {
+        // Tell the analysis our mayResolve hooks won't trigger GC.
+        JS::AutoSuppressGCAnalysis nogc;
+        if (!mayResolve(names, id, maybeObj))
+            return false;
+    }
+
+    return true;
+}
+
 } // namespace js
 
 inline js::Shape*
 JSObject::maybeShape() const
 {
     if (!is<js::ShapedObject>())
         return nullptr;
 
@@ -237,16 +259,60 @@ inline bool
 js::DeleteElement(JSContext* cx, HandleObject obj, uint32_t index, ObjectOpResult& result)
 {
     RootedId id(cx);
     if (!IndexToId(cx, index, &id))
         return false;
     return DeleteProperty(cx, obj, id, result);
 }
 
+MOZ_ALWAYS_INLINE bool
+js::MaybeHasInterestingSymbolProperty(JSContext* cx, JSObject* obj, Symbol* symbol,
+                                      JSObject** holder)
+{
+    MOZ_ASSERT(symbol->isInterestingSymbol());
+
+    jsid id = SYMBOL_TO_JSID(symbol);
+    do {
+        if (obj->maybeHasInterestingSymbolProperty() ||
+            obj->hasDynamicPrototype() ||
+            MOZ_UNLIKELY(ClassMayResolveId(cx->names(), obj->getClass(), id, obj) ||
+                         obj->getClass()->getGetProperty()))
+        {
+            if (holder)
+                *holder = obj;
+            return true;
+        }
+        obj = obj->staticPrototype();
+    } while (obj);
+
+    return false;
+}
+
+MOZ_ALWAYS_INLINE bool
+js::GetInterestingSymbolProperty(JSContext* cx, HandleObject obj, Symbol* sym, MutableHandleValue vp)
+{
+    JSObject* holder;
+    if (!MaybeHasInterestingSymbolProperty(cx, obj, sym, &holder)) {
+#ifdef DEBUG
+        RootedValue receiver(cx, ObjectValue(*obj));
+        RootedId id(cx, SYMBOL_TO_JSID(sym));
+        if (!GetProperty(cx, obj, receiver, id, vp))
+            return false;
+        MOZ_ASSERT(vp.isUndefined());
+#endif
+        vp.setUndefined();
+        return true;
+    }
+
+    RootedObject holderRoot(cx, holder);
+    RootedValue receiver(cx, ObjectValue(*obj));
+    RootedId id(cx, SYMBOL_TO_JSID(sym));
+    return GetProperty(cx, holderRoot, receiver, id, vp);
+}
 
 /* * */
 
 inline bool
 JSObject::isQualifiedVarObj() const
 {
     if (is<js::DebugEnvironmentProxy>())
         return as<js::DebugEnvironmentProxy>().environment().isQualifiedVarObj();
@@ -406,16 +472,33 @@ JSObject::hadElementsAccess() const
 }
 
 inline bool
 JSObject::isIndexed() const
 {
     return hasAllFlags(js::BaseShape::INDEXED);
 }
 
+MOZ_ALWAYS_INLINE bool
+JSObject::maybeHasInterestingSymbolProperty() const
+{
+    const js::NativeObject* nobj;
+    if (isNative()) {
+        nobj = &as<js::NativeObject>();
+    } else if (is<js::UnboxedPlainObject>()) {
+        nobj = as<js::UnboxedPlainObject>().maybeExpando();
+        if (!nobj)
+            return false;
+    } else {
+        return true;
+    }
+
+    return nobj->hasAllFlags(js::BaseShape::HAS_INTERESTING_SYMBOL);
+}
+
 inline bool
 JSObject::staticPrototypeIsImmutable() const
 {
     MOZ_ASSERT(hasStaticPrototype());
     return hasAllFlags(js::BaseShape::IMMUTABLE_PROTOTYPE);
 }
 
 inline bool
--- a/js/src/vm/NativeObject-inl.h
+++ b/js/src/vm/NativeObject-inl.h
@@ -667,38 +667,16 @@ CallResolveOp(JSContext* cx, HandleNativ
     if (shape)
         propp.setNativeProperty(shape);
     else
         propp.setNotFound();
 
     return true;
 }
 
-static MOZ_ALWAYS_INLINE bool
-ClassMayResolveId(const JSAtomState& names, const Class* clasp, jsid id, JSObject* maybeObj)
-{
-    MOZ_ASSERT_IF(maybeObj, maybeObj->getClass() == clasp);
-
-    if (!clasp->getResolve()) {
-        // Sanity check: we should only have a mayResolve hook if we have a
-        // resolve hook.
-        MOZ_ASSERT(!clasp->getMayResolve(), "Class with mayResolve hook but no resolve hook");
-        return false;
-    }
-
-    if (JSMayResolveOp mayResolve = clasp->getMayResolve()) {
-        // Tell the analysis our mayResolve hooks won't trigger GC.
-        JS::AutoSuppressGCAnalysis nogc;
-        if (!mayResolve(names, id, maybeObj))
-            return false;
-    }
-
-    return true;
-}
-
 template <AllowGC allowGC>
 static MOZ_ALWAYS_INLINE bool
 LookupOwnPropertyInline(JSContext* cx,
                         typename MaybeRooted<NativeObject*, allowGC>::HandleType obj,
                         typename MaybeRooted<jsid, allowGC>::HandleType id,
                         typename MaybeRooted<PropertyResult, allowGC>::MutableHandleType propp,
                         bool* donep)
 {
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -440,16 +440,34 @@ ShouldConvertToDictionary(NativeObject* 
      * Use a lower limit if this object is likely a hashmap (SETELEM was used
      * to set properties).
      */
     if (obj->hadElementsAccess())
         return obj->lastProperty()->entryCount() >= PropertyTree::MAX_HEIGHT_WITH_ELEMENTS_ACCESS;
     return obj->lastProperty()->entryCount() >= PropertyTree::MAX_HEIGHT;
 }
 
+static MOZ_ALWAYS_INLINE UnownedBaseShape*
+GetBaseShapeForNewShape(JSContext* cx, HandleShape last, HandleId id)
+{
+    uint32_t index;
+    bool indexed = IdIsIndex(id, &index);
+    bool interestingSymbol = JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id)->isInterestingSymbol();
+
+    if (MOZ_LIKELY(!indexed && !interestingSymbol))
+        return last->base()->unowned();
+
+    StackBaseShape base(last->base());
+    if (indexed)
+        base.flags |= BaseShape::INDEXED;
+    else if (interestingSymbol)
+        base.flags |= BaseShape::HAS_INTERESTING_SYMBOL;
+    return BaseShape::getUnowned(cx, base);
+}
+
 /* static */ Shape*
 NativeObject::addPropertyInternal(JSContext* cx,
                                   HandleNativeObject obj, HandleId id,
                                   GetterOp getter, SetterOp setter,
                                   uint32_t slot, unsigned attrs,
                                   unsigned flags, ShapeTable::Entry* entry,
                                   bool allowDictionary, const AutoKeepShapeTables& keep)
 {
@@ -492,30 +510,19 @@ NativeObject::addPropertyInternal(JSCont
     }
 
     MOZ_ASSERT(!!table == !!entry);
 
     /* Find or create a property tree node labeled by our arguments. */
     RootedShape shape(cx);
     {
         RootedShape last(cx, obj->lastProperty());
-
-        uint32_t index;
-        bool indexed = IdIsIndex(id, &index);
-
-        Rooted<UnownedBaseShape*> nbase(cx);
-        if (!indexed) {
-            nbase = last->base()->unowned();
-        } else {
-            StackBaseShape base(last->base());
-            base.flags |= BaseShape::INDEXED;
-            nbase = BaseShape::getUnowned(cx, base);
-            if (!nbase)
-                return nullptr;
-        }
+        Rooted<UnownedBaseShape*> nbase(cx, GetBaseShapeForNewShape(cx, last, id));
+        if (!nbase)
+            return nullptr;
 
         Rooted<StackShape> child(cx, StackShape(nbase, id, slot, attrs, flags));
         child.updateGetterSetter(getter, setter);
         shape = getChildProperty(cx, obj, last, &child);
     }
 
     if (shape) {
         MOZ_ASSERT(shape == obj->lastProperty());
@@ -564,27 +571,19 @@ js::ReshapeForAllocKind(JSContext* cx, S
     RootedShape newShape(cx, EmptyShape::getInitialShape(cx, shape->getObjectClass(),
                                                          proto, nfixed, shape->getObjectFlags()));
     if (!newShape)
         return nullptr;
 
     for (unsigned i = 0; i < ids.length(); i++) {
         id = ids[i];
 
-        uint32_t index;
-        bool indexed = IdIsIndex(id, &index);
-
-        Rooted<UnownedBaseShape*> nbase(cx, newShape->base()->unowned());
-        if (indexed) {
-            StackBaseShape base(nbase);
-            base.flags |= BaseShape::INDEXED;
-            nbase = BaseShape::getUnowned(cx, base);
-            if (!nbase)
-                return nullptr;
-        }
+        Rooted<UnownedBaseShape*> nbase(cx, GetBaseShapeForNewShape(cx, newShape, id));
+        if (!nbase)
+            return nullptr;
 
         Rooted<StackShape> child(cx, StackShape(nbase, id, i, JSPROP_ENUMERATE, 0));
         newShape = cx->zone()->propertyTree().getChild(cx, newShape, child);
         if (!newShape)
             return nullptr;
     }
 
     return newShape;
@@ -679,22 +678,18 @@ NativeObject::putProperty(JSContext* cx,
      */
     bool hadSlot = shape->hasSlot();
     uint32_t oldSlot = shape->maybeSlot();
     if (!(attrs & JSPROP_SHARED) && slot == SHAPE_INVALID_SLOT && hadSlot)
         slot = oldSlot;
 
     Rooted<UnownedBaseShape*> nbase(cx);
     {
-        uint32_t index;
-        bool indexed = IdIsIndex(id, &index);
-        StackBaseShape base(obj->lastProperty()->base());
-        if (indexed)
-            base.flags |= BaseShape::INDEXED;
-        nbase = BaseShape::getUnowned(cx, base);
+        RootedShape shape(cx, obj->lastProperty());
+        nbase = GetBaseShapeForNewShape(cx, shape, id);
         if (!nbase)
             return nullptr;
     }
 
     /*
      * Now that we've possibly preserved slot, check whether all members match.
      * If so, this is a redundant "put" and we can return without more work.
      */
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@@ -384,17 +384,17 @@ class BaseShape : public gc::TenuredCell
          *
          * If you add a new flag here, please add appropriate code to
          * JSObject::dump to dump it as part of object representation.
          */
 
         DELEGATE            =    0x8,
         NOT_EXTENSIBLE      =   0x10,
         INDEXED             =   0x20,
-        /* (0x40 is unused) */
+        HAS_INTERESTING_SYMBOL = 0x40,
         HAD_ELEMENTS_ACCESS =   0x80,
         WATCHED             =  0x100,
         ITERATED_SINGLETON  =  0x200,
         NEW_GROUP_UNKNOWN   =  0x400,
         UNCACHEABLE_PROTO   =  0x800,
         IMMUTABLE_PROTOTYPE = 0x1000,
 
         // See JSObject::isQualifiedVarObj().
--- a/js/src/vm/Symbol.h
+++ b/js/src/vm/Symbol.h
@@ -63,16 +63,25 @@ class Symbol : public js::gc::TenuredCel
     static Symbol* for_(JSContext* cx, js::HandleString description);
 
     JSAtom* description() const { return description_; }
     SymbolCode code() const { return code_; }
     js::HashNumber hash() const { return hash_; }
 
     bool isWellKnownSymbol() const { return uint32_t(code_) < WellKnownSymbolLimit; }
 
+    // An "interesting symbol" is a well-known symbol, like @@toStringTag,
+    // that's often looked up on random objects but is usually not present. We
+    // optimize this by setting a flag on the object's BaseShape when such
+    // symbol properties are added, so we can optimize lookups on objects that
+    // don't have the BaseShape flag.
+    bool isInterestingSymbol() const {
+        return code_ == SymbolCode::toStringTag || code_ == SymbolCode::toPrimitive;
+    }
+
     static const JS::TraceKind TraceKind = JS::TraceKind::Symbol;
     inline void traceChildren(JSTracer* trc) {
         if (description_)
             js::TraceManuallyBarrieredEdge(trc, &description_, "description");
     }
     inline void finalize(js::FreeOp*) {}
 
     static MOZ_ALWAYS_INLINE void writeBarrierPre(Symbol* thing) {