And remove the non-pointer bits from Return<T>. Having Return<T> as an actual struct results in a reliable 3-5% performance hit on Kraken on all platforms in bug 811168 (and presumably in all the other places we've introduced Return<T>). I verified this by manually removing all of the Return<T> from the patch in Bug 811168 and seeing the performance decrease disappear.

If we make our return types a struct in debug and and a pointer in opt, then NullPtr is not going to work as a return type. What we really, really need here is nullptr_t.

This is almost entirely mechanical after the gc/Root.h changes.

Comment on attachment 688040 [details] [diff] [review] v0 Review of attachment 688040 [details] [diff] [review]: ----------------------------------------------------------------- This seems like a nice simplification without losing too much safety. ::: js/src/gc/Root.h @@ +112,4 @@ > * scope. This can be used to force |v| out of scope before its C++ scope > * would end naturally. The usage of braces C++ syntactical scopes |{...}| > * is strongly perferred to this, but sometimes will not work because of > * awkwardly overlapping lifetimes. I think you need to update this comment to reflect the absence of Return<T>. @@ +121,5 @@ > + * The following diagram explains the list of supported, implicit type > + * conversions between classes of this family: > + * > + * RawT <- - - - - - -+ > + * v Currently we can convert from Unrooted to Raw. That should be reflected in the diagram. @@ +131,5 @@ > + * +--------> Rooted<T> <----> Handle<T> > + * ^ ^ > + * | | > + * | | > + * +---& MutableHandle<T> Using the ampersand as the arrowhead is a little confusing. Maybe use a normal arrowhead but put the ampersand below the horizontal part of the arrow? Or, more explicitly, put (via &) below? Also, is there any significance to the fact that some lines are dashed (between RawT and Rooted<T>) and the rest are not? ::: js/src/methodjit/PolyIC.cpp @@ +1499,5 @@ > } > > // Mark as not idempotent to avoid recompilation in Ion Monkey > // GetPropertyCache. > if (!obj->hasIdempotentProtoChain()) { You can drop the braces. @@ +1505,5 @@ > } > > // The property is missing, Mark as not idempotent to avoid > // recompilation in Ion Monkey GetPropertyCache. > if (!getprop.holder) { Same here.

Green try run at: https://tbpl.mozilla.org/?tree=Try&rev=31d9f28f0d87 Pushed at: https://hg.mozilla.org/integration/mozilla-inbound/rev/5acd87d0cf33