Object.prototype.toString and OrdinaryToPrimitive are pretty hot functions, but they have to do slow symbol lookups, even though in practice these symbols are almost never defined on the object. The attached patch optimizes this as follows: * When we add an "interesting symbol" property (currently @@toStringTag or @@toPrimitive) to an object, we set a flag on the BaseShape. * When we look up these symbol properties, we can check this flag and avoid the slow lookup if it's not set. The toString micro-benchmark below improves from 250 ms to 120 ms with this: -- function f() { var res = ""; var a = [1, 2, 3]; var toString = Object.prototype.toString; var t = new Date; for (var i = 0; i < 5000000; i++) res = toString.call(a); print(new Date - t); return res; } f(); --

Forgot to qref.

Comment on attachment 8873067 [details] [diff] [review] Patch Review of attachment 8873067 [details] [diff] [review]: ----------------------------------------------------------------- Great results for such a relatively simple change! ::: js/src/jsobjinlines.h @@ +290,5 @@ > +js::GetInterestingSymbolProperty(JSContext* cx, HandleObject obj, Symbol* sym, MutableHandleValue vp) > +{ > + JSObject* holder; > + if (!MaybeHasInterestingSymbolProperty(cx, obj, sym, &holder)) { > + vp.setUndefined(); I would assert that GetProperty really returns undefined here. @@ +469,5 @@ > return hasAllFlags(js::BaseShape::INDEXED); > } > > +MOZ_ALWAYS_INLINE bool > +JSObject::hasInterestingSymbolProperty() const Add maybe maybe? (For the non-native/unboxed cases) ::: js/src/vm/Shape.cpp @@ +683,5 @@ > > Rooted<UnownedBaseShape*> nbase(cx); > { > + RootedShape shape(cx, obj->lastProperty()); > + nbase = GetBaseShapeForNewShape(cx, shape, id); Seems like before we would never use the last unowned baseshape here?

Pushed by jandemooij@gmail.com: http://hg.mozilla.org/integration/mozilla-inbound/rev/5ab80eaba78c Optimize @@toStringTag and @@toPrimitive property lookups in the VM. r=evilpie

(In reply to Tom Schuster [:evilpie] from comment #2) > I would assert that GetProperty really returns undefined here. Done. > Add maybe maybe? (For the non-native/unboxed cases) Done. > Seems like before we would never use the last unowned baseshape here? Yeah we would do a lookup and usually get the same BaseShape, so this is a small optimization. This code path (changing attributes of an existing property) is not very hot AFAIK but it can't hurt.

Looks like V8 borrowed this idea :) http://v8project.blogspot.com/2017/09/v8-release-62.html