Bug 1442304 - make already_AddRefed returnable in registers in non-DEBUG Unix builds; r=glandium,tjr
authorNathan Froyd <froydnj@mozilla.com>
Wed, 07 Mar 2018 14:27:28 -0500
changeset 462107 32c5d61d51ed33a32cd8e7407e60def0108fac8d
parent 462106 fcabcde9cdae95c70c448f56f6bc8d6aaf245da8
child 462108 7939ec78670514e4fb273cc77af89f28eadbcbfa
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglandium, tjr
bugs1442304
milestone60.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 1442304 - make already_AddRefed returnable in registers in non-DEBUG Unix builds; r=glandium,tjr This change saves ~150k (!) of binary size on x86-64 Linux.
mfbt/AlreadyAddRefed.h
--- a/mfbt/AlreadyAddRefed.h
+++ b/mfbt/AlreadyAddRefed.h
@@ -70,17 +70,59 @@ struct MOZ_TEMPORARY_CLASS MOZ_MUST_USE_
   MOZ_IMPLICIT already_AddRefed(decltype(nullptr)) : mRawPtr(nullptr) {}
 
   explicit already_AddRefed(T* aRawPtr) : mRawPtr(aRawPtr) {}
 
   // Disallow copy constructor and copy assignment operator: move semantics used instead.
   already_AddRefed(const already_AddRefed<T>& aOther) = delete;
   already_AddRefed<T>& operator=(const already_AddRefed<T>& aOther) = delete;
 
-  already_AddRefed(already_AddRefed<T>&& aOther) : mRawPtr(aOther.take()) {}
+  // WARNING: sketchiness ahead.
+  //
+  // The x86-64 ABI for Unix-like operating systems requires structures to be
+  // returned via invisible reference if they are non-trivial for the purposes
+  // of calls according to the C++ ABI[1].  For our consideration here, that
+  // means that if we have a non-trivial move constructor or destructor,
+  // already_AddRefed must be returned by invisible reference.  But
+  // already_AddRefed is small enough and so commonly used that it would be
+  // beneficial to return it via registers instead.  So we need to figure out
+  // a way to make the move constructor and the destructor trivial.
+  //
+  // Our destructor is normally non-trivial, because it asserts that the
+  // stored pointer has been taken by somebody else prior to destruction.
+  // However, since the assert in question is compiled only for DEBUG builds,
+  // we can make the destructor trivial in non-DEBUG builds by simply defining
+  // it with `= default`.
+  //
+  // We now have to make the move constructor trivial as well.  It is normally
+  // non-trivial, because the incoming object has its pointer null-ed during
+  // the move. This null-ing is done to satisfy the assert in the destructor.
+  // But since that destructor has no assert in non-DEBUG builds, the clearing
+  // is unnecessary in such builds; all we really need to perform is a copy of
+  // the pointer from the incoming object.  So we can let the compiler define
+  // a trivial move constructor for us, and already_AddRefed can now be
+  // returned in registers rather than needing to allocate a stack slot for
+  // an invisible reference.
+  //
+  // The above considerations apply to Unix-like operating systems only; the
+  // conditions for the same optimization to apply on x86-64 Windows are much
+  // more strigent and are basically impossible for already_AddRefed to
+  // satisfy[2].  But we do get some benefit from this optimization on Windows
+  // because we removed the nulling of the pointer during the move, so that's
+  // a codesize win.
+  //
+  // [1] https://itanium-cxx-abi.github.io/cxx-abi/abi.html#non-trivial
+  // [2] https://docs.microsoft.com/en-us/cpp/build/return-values-cpp
+
+  already_AddRefed(already_AddRefed<T>&& aOther)
+#ifdef DEBUG
+    : mRawPtr(aOther.take()) {}
+#else
+    = default;
+#endif
 
   already_AddRefed<T>& operator=(already_AddRefed<T>&& aOther)
   {
     mRawPtr = aOther.take();
     return *this;
   }
 
   /**
@@ -98,17 +140,22 @@ struct MOZ_TEMPORARY_CLASS MOZ_MUST_USE_
    *    RefPtr<BaseClass> y = x.forget();
    *    return y.forget();
    *
    * Note that nsRefPtr is the XPCOM reference counting smart pointer class.
    */
   template <typename U>
   MOZ_IMPLICIT already_AddRefed(already_AddRefed<U>&& aOther) : mRawPtr(aOther.take()) {}
 
-  ~already_AddRefed() { MOZ_ASSERT(!mRawPtr); }
+  ~already_AddRefed()
+#ifdef DEBUG
+     { MOZ_ASSERT(!mRawPtr); }
+#else
+     = default;
+#endif
 
   // Specialize the unused operator<< for already_AddRefed, to allow
   // nsCOMPtr<nsIFoo> foo;
   // Unused << foo.forget();
   // Note that nsCOMPtr is the XPCOM reference counting smart pointer class.
   friend void operator<<(const mozilla::unused_t& aUnused,
                          const already_AddRefed<T>& aRhs)
   {