Bug 1367885: Add a fast path to mscom Interceptor construction; r=jimm
authorAaron Klotz <aklotz@mozilla.com>
Tue, 06 Jun 2017 18:30:19 -0600
changeset 413089 09894b9ce473f681e86dfaf59ef4286c131ae669
parent 413088 80924717bcc66bce4cc1de04b61c6ea18a283126
child 413090 780118d9c79c2a8e8f12724ac7d04b0170f330a7
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm
bugs1367885
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 1367885: Add a fast path to mscom Interceptor construction; r=jimm MozReview-Commit-ID: AmS5oBNV7Po When creating a new interceptor, we already have the correct target interface. The interceptor ignores this and does a redundant inter-thread QI to resolve an interface that we already have! This patch adds a fast path to interceptor initialization that skips all of that stuff and directly initializes itself using the given target interceptor.
ipc/mscom/Interceptor.cpp
ipc/mscom/Interceptor.h
--- a/ipc/mscom/Interceptor.cpp
+++ b/ipc/mscom/Interceptor.cpp
@@ -20,42 +20,41 @@
 #include "nsDirectoryServiceUtils.h"
 #include "nsThreadUtils.h"
 
 namespace mozilla {
 namespace mscom {
 
 /* static */ HRESULT
 Interceptor::Create(STAUniquePtr<IUnknown> aTarget, IInterceptorSink* aSink,
-                    REFIID aIid, void** aOutput)
+                    REFIID aInitialIid, void** aOutInterface)
 {
-  MOZ_ASSERT(aOutput && aTarget && aSink);
-  if (!aOutput) {
+  MOZ_ASSERT(aOutInterface && aTarget && aSink);
+  if (!aOutInterface) {
     return E_INVALIDARG;
   }
 
-  *aOutput = nullptr;
+  *aOutInterface = nullptr;
 
   if (!aTarget || !aSink) {
     return E_INVALIDARG;
   }
 
-  RefPtr<WeakReferenceSupport> intcpt(new Interceptor(Move(aTarget), aSink));
-  return intcpt->QueryInterface(aIid, aOutput);
+  RefPtr<Interceptor> intcpt(new Interceptor(aSink));
+  return intcpt->GetInitialInterceptorForIID(aInitialIid, Move(aTarget),
+                                             aOutInterface);
 }
 
-Interceptor::Interceptor(STAUniquePtr<IUnknown> aTarget, IInterceptorSink* aSink)
+Interceptor::Interceptor(IInterceptorSink* aSink)
   : WeakReferenceSupport(WeakReferenceSupport::Flags::eDestroyOnMainThread)
-  , mTarget(Move(aTarget))
   , mEventSink(aSink)
   , mMutex("mozilla::mscom::Interceptor::mMutex")
   , mStdMarshal(nullptr)
 {
   MOZ_ASSERT(aSink);
-  MOZ_ASSERT(!IsProxy(mTarget.get()));
   RefPtr<IWeakReference> weakRef;
   if (SUCCEEDED(GetWeakReference(getter_AddRefs(weakRef)))) {
     aSink->SetInterceptor(weakRef);
   }
 }
 
 Interceptor::~Interceptor()
 {
@@ -200,16 +199,65 @@ Interceptor::CreateInterceptor(REFIID aI
                                       (void**)aOutput);
   // If this assert fires then the interceptor doesn't like something about
   // the format of the typelib. One thing in particular that it doesn't like
   // is complex types that contain unions.
   MOZ_ASSERT(SUCCEEDED(hr));
   return hr;
 }
 
+HRESULT
+Interceptor::GetInitialInterceptorForIID(REFIID aTargetIid,
+                                         STAUniquePtr<IUnknown> aTarget,
+                                         void** aOutInterceptor)
+{
+  MOZ_ASSERT(aOutInterceptor);
+  MOZ_ASSERT(aTargetIid != IID_IUnknown && aTargetIid != IID_IMarshal);
+  MOZ_ASSERT(!IsProxy(aTarget.get()));
+
+  // Raise the refcount for stabilization purposes during aggregation
+  RefPtr<IUnknown> kungFuDeathGrip(static_cast<IUnknown*>(
+        static_cast<WeakReferenceSupport*>(this)));
+
+  RefPtr<IUnknown> unkInterceptor;
+  HRESULT hr = CreateInterceptor(aTargetIid, kungFuDeathGrip,
+                                 getter_AddRefs(unkInterceptor));
+  if (FAILED(hr)) {
+    return hr;
+  }
+
+  RefPtr<ICallInterceptor> interceptor;
+  hr = unkInterceptor->QueryInterface(IID_ICallInterceptor,
+                                      getter_AddRefs(interceptor));
+  if (FAILED(hr)) {
+    return hr;
+  }
+
+  hr = interceptor->RegisterSink(mEventSink);
+  if (FAILED(hr)) {
+    return hr;
+  }
+
+  // mTarget is a weak reference to aTarget. This is safe because we transfer
+  // ownership of aTarget into mInterceptorMap which remains live for the
+  // lifetime of this Interceptor.
+  mTarget = ToInterceptorTargetPtr(aTarget);
+
+  // Now we transfer aTarget's ownership into mInterceptorMap.
+  mInterceptorMap.AppendElement(MapEntry(aTargetIid,
+                                         unkInterceptor,
+                                         aTarget.release()));
+
+  if (mEventSink->MarshalAs(aTargetIid) == aTargetIid) {
+    return unkInterceptor->QueryInterface(aTargetIid, aOutInterceptor);
+  }
+
+  return GetInterceptorForIID(aTargetIid, aOutInterceptor);
+}
+
 /**
  * This method contains the core guts of the handling of QueryInterface calls
  * that are delegated to us from the ICallInterceptor.
  *
  * @param aIid ID of the desired interface
  * @param aOutInterceptor The resulting emulated vtable that corresponds to
  * the interface specified by aIid.
  */
--- a/ipc/mscom/Interceptor.h
+++ b/ipc/mscom/Interceptor.h
@@ -62,17 +62,17 @@ struct IInterceptor : public IUnknown
  */
 class Interceptor final : public WeakReferenceSupport
                         , public IStdMarshalInfo
                         , public IMarshal
                         , public IInterceptor
 {
 public:
   static HRESULT Create(STAUniquePtr<IUnknown> aTarget, IInterceptorSink* aSink,
-                        REFIID aIid, void** aOutput);
+                        REFIID aInitialIid, void** aOutInterface);
 
   // IUnknown
   STDMETHODIMP QueryInterface(REFIID riid, void** ppv) override;
   STDMETHODIMP_(ULONG) AddRef() override;
   STDMETHODIMP_(ULONG) Release() override;
 
   // IStdMarshalInfo
   STDMETHODIMP GetClassForHandler(DWORD aDestContext, void* aDestContextPtr,
@@ -101,32 +101,54 @@ public:
 private:
   struct MapEntry
   {
     MapEntry(REFIID aIid, IUnknown* aInterceptor, IUnknown* aTargetInterface)
       : mIID(aIid)
       , mInterceptor(aInterceptor)
       , mTargetInterface(aTargetInterface)
     {}
-    IID               mIID;
+
+    MapEntry(const MapEntry& aOther)
+      : mIID(aOther.mIID)
+      , mInterceptor(aOther.mInterceptor)
+      , mTargetInterface(aOther.mTargetInterface)
+    {
+    }
+
+    MapEntry(MapEntry&& aOther)
+      : mIID(aOther.mIID)
+      , mInterceptor(Move(aOther.mInterceptor))
+      , mTargetInterface(aOther.mTargetInterface)
+    {
+      aOther.mTargetInterface = nullptr;
+    }
+
+    MapEntry& operator=(const MapEntry& aOther) = delete;
+    MapEntry& operator=(MapEntry&& aOther) = delete;
+
+    REFIID            mIID;
     RefPtr<IUnknown>  mInterceptor;
     IUnknown*         mTargetInterface;
   };
 
 private:
-  Interceptor(STAUniquePtr<IUnknown> aTarget, IInterceptorSink* aSink);
+  explicit Interceptor(IInterceptorSink* aSink);
   ~Interceptor();
+  HRESULT GetInitialInterceptorForIID(REFIID aTargetIid,
+                                      STAUniquePtr<IUnknown> aTarget,
+                                      void** aOutInterface);
   MapEntry* Lookup(REFIID aIid);
   HRESULT QueryInterfaceTarget(REFIID aIid, void** aOutput);
   HRESULT ThreadSafeQueryInterface(REFIID aIid,
                                    IUnknown** aOutInterface) override;
   HRESULT CreateInterceptor(REFIID aIid, IUnknown* aOuter, IUnknown** aOutput);
 
 private:
-  STAUniquePtr<IUnknown>    mTarget;
+  InterceptorTargetPtr<IUnknown>  mTarget;
   RefPtr<IInterceptorSink>  mEventSink;
   mozilla::Mutex            mMutex; // Guards mInterceptorMap
   // Using a nsTArray since the # of interfaces is not going to be very high
   nsTArray<MapEntry>        mInterceptorMap;
   RefPtr<IUnknown>          mStdMarshalUnk;
   IMarshal*                 mStdMarshal; // WEAK
 };