Bug 1451363 - part 1 - move Shmem-related IProtocol interfaces into an intermediate State class; r=mccr8
authorNathan Froyd <froydnj@mozilla.com>
Mon, 23 Apr 2018 14:13:37 -0400
changeset 468679 5b5ce8fcfdffe3631186be9e1d293f86fa68e51c
parent 468678 5f5f5d37a32ebc395b4e65c442763a18089b95f0
child 468680 78353bf75968371ecbdcb447b5452f28dd757e85
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs1451363
milestone61.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 1451363 - part 1 - move Shmem-related IProtocol interfaces into an intermediate State class; r=mccr8 IProtocol, which is inherited by every generated IPDL protocol and every concrete protocol implementation in-tree, has a number of virtual methods that are only relevant when distinguishing between top-level protocols (IToplevelProtocol) and managed protocols (everything else). These virtual methods require pointers in every protocol's vtable, which is wasteful, and it's also somewhat confusing that many methods exist but don't really need to be overridable in any useful way. Let's clean this up, by creating a ProtocolState class to hold methods that solely differ between top-level protocols and everything else. This commit does that work and moves Shmem-related methods into this class as a proof that this can be done in a reasonable way.
ipc/glue/ProtocolUtils.cpp
ipc/glue/ProtocolUtils.h
--- a/ipc/glue/ProtocolUtils.cpp
+++ b/ipc/glue/ProtocolUtils.cpp
@@ -420,40 +420,40 @@ IProtocol::Unregister(int32_t aId)
 {
   if (mId == aId) {
     mId = kFreedActorId;
   }
   Manager()->Unregister(aId);
 }
 
 Shmem::SharedMemory*
-IProtocol::CreateSharedMemory(size_t aSize,
-                              SharedMemory::SharedMemoryType aType,
-                              bool aUnsafe,
-                              int32_t* aId)
+IProtocol::ManagedState::CreateSharedMemory(size_t aSize,
+                                            SharedMemory::SharedMemoryType aType,
+                                            bool aUnsafe,
+                                            int32_t* aId)
 {
-  return Manager()->CreateSharedMemory(aSize, aType, aUnsafe, aId);
+  return mProtocol->Manager()->CreateSharedMemory(aSize, aType, aUnsafe, aId);
 }
 
 Shmem::SharedMemory*
-IProtocol::LookupSharedMemory(int32_t aId)
+IProtocol::ManagedState::LookupSharedMemory(int32_t aId)
 {
-  return Manager()->LookupSharedMemory(aId);
+  return mProtocol->Manager()->LookupSharedMemory(aId);
 }
 
 bool
-IProtocol::IsTrackingSharedMemory(Shmem::SharedMemory* aSegment)
+IProtocol::ManagedState::IsTrackingSharedMemory(Shmem::SharedMemory* aSegment)
 {
-  return Manager()->IsTrackingSharedMemory(aSegment);
+  return mProtocol->Manager()->IsTrackingSharedMemory(aSegment);
 }
 
 bool
-IProtocol::DestroySharedMemory(Shmem& aShmem)
+IProtocol::ManagedState::DestroySharedMemory(Shmem& aShmem)
 {
-  return Manager()->DestroySharedMemory(aShmem);
+  return mProtocol->Manager()->DestroySharedMemory(aShmem);
 }
 
 ProcessId
 IProtocol::OtherPid() const
 {
   return Manager()->OtherPid();
 }
 
@@ -572,23 +572,22 @@ IProtocol::GetActorEventTarget()
 
 already_AddRefed<nsIEventTarget>
 IProtocol::GetActorEventTargetInternal(IProtocol* aActor)
 {
   return Manager()->GetActorEventTargetInternal(aActor);
 }
 
 IToplevelProtocol::IToplevelProtocol(ProtocolId aProtoId, Side aSide)
- : IProtocol(aSide),
+ : IProtocol(aSide, MakeUnique<ToplevelState>(this, aSide)),
    mMonitor("mozilla.ipc.IToplevelProtocol.mMonitor"),
    mProtocolId(aProtoId),
    mOtherPid(mozilla::ipc::kInvalidProcessId),
    mOtherPidState(ProcessIdState::eUnstarted),
    mLastRouteId(aSide == ParentSide ? kFreedActorId : kNullActorId),
-   mLastShmemId(aSide == ParentSide ? kFreedActorId : kNullActorId),
    mEventTargetMutex("ProtocolEventTargetMutex")
 {
 }
 
 IToplevelProtocol::~IToplevelProtocol()
 {
   if (mTrans) {
     RefPtr<DeleteTask<Transport>> task = new DeleteTask<Transport>(mTrans.release());
@@ -735,114 +734,122 @@ void
 IToplevelProtocol::Unregister(int32_t aId)
 {
   mActorMap.Remove(aId);
 
   MutexAutoLock lock(mEventTargetMutex);
   mEventTargetMap.RemoveIfPresent(aId);
 }
 
+IToplevelProtocol::ToplevelState::ToplevelState(IToplevelProtocol* aProtocol, Side aSide)
+  : mProtocol(aProtocol)
+  , mLastShmemId(aSide == ParentSide ? kFreedActorId : kNullActorId)
+{
+}
+
 Shmem::SharedMemory*
-IToplevelProtocol::CreateSharedMemory(size_t aSize,
-                                      Shmem::SharedMemory::SharedMemoryType aType,
-                                      bool aUnsafe,
-                                      Shmem::id_t* aId)
+IToplevelProtocol::ToplevelState::CreateSharedMemory(size_t aSize,
+                                                     Shmem::SharedMemory::SharedMemoryType aType,
+                                                     bool aUnsafe,
+                                                     Shmem::id_t* aId)
 {
+  // XXX the mProtocol uses here should go away!
   RefPtr<Shmem::SharedMemory> segment(
     Shmem::Alloc(Shmem::PrivateIPDLCaller(), aSize, aType, aUnsafe));
   if (!segment) {
     return nullptr;
   }
-  int32_t id = GetSide() == ParentSide ? ++mLastShmemId : --mLastShmemId;
+  int32_t id = mProtocol->GetSide() == ParentSide ? ++mLastShmemId : --mLastShmemId;
   Shmem shmem(
     Shmem::PrivateIPDLCaller(),
     segment.get(),
     id);
 
   base::ProcessId pid =
 #ifdef ANDROID
     // We use OtherPidMaybeInvalid() because on Android this method is actually
     // called on an unconnected protocol, but Android's shared memory
     // implementation doesn't actually use the PID.
-    OtherPidMaybeInvalid();
+    mProtocol->OtherPidMaybeInvalid();
 #else
-    OtherPid();
+    mProtocol->OtherPid();
 #endif
 
   Message* descriptor = shmem.ShareTo(
     Shmem::PrivateIPDLCaller(), pid, MSG_ROUTING_CONTROL);
   if (!descriptor) {
     return nullptr;
   }
-  Unused << GetIPCChannel()->Send(descriptor);
+  Unused << mProtocol->GetIPCChannel()->Send(descriptor);
 
   *aId = shmem.Id(Shmem::PrivateIPDLCaller());
   Shmem::SharedMemory* rawSegment = segment.get();
   mShmemMap.AddWithID(segment.forget().take(), *aId);
   return rawSegment;
 }
 
 Shmem::SharedMemory*
-IToplevelProtocol::LookupSharedMemory(Shmem::id_t aId)
+IToplevelProtocol::ToplevelState::LookupSharedMemory(Shmem::id_t aId)
 {
   return mShmemMap.Lookup(aId);
 }
 
 bool
-IToplevelProtocol::IsTrackingSharedMemory(Shmem::SharedMemory* segment)
+IToplevelProtocol::ToplevelState::IsTrackingSharedMemory(Shmem::SharedMemory* segment)
 {
   return mShmemMap.HasData(segment);
 }
 
 bool
-IToplevelProtocol::DestroySharedMemory(Shmem& shmem)
+IToplevelProtocol::ToplevelState::DestroySharedMemory(Shmem& shmem)
 {
   Shmem::id_t aId = shmem.Id(Shmem::PrivateIPDLCaller());
   Shmem::SharedMemory* segment = LookupSharedMemory(aId);
   if (!segment) {
     return false;
   }
 
   Message* descriptor = shmem.UnshareFrom(
     Shmem::PrivateIPDLCaller(), MSG_ROUTING_CONTROL);
 
   mShmemMap.Remove(aId);
   Shmem::Dealloc(Shmem::PrivateIPDLCaller(), segment);
 
-  if (!GetIPCChannel()->CanSend()) {
+  MessageChannel* channel = mProtocol->GetIPCChannel();
+  if (!channel->CanSend()) {
     delete descriptor;
     return true;
   }
 
-  return descriptor && GetIPCChannel()->Send(descriptor);
+  return descriptor && channel->Send(descriptor);
 }
 
 void
-IToplevelProtocol::DeallocShmems()
+IToplevelProtocol::ToplevelState::DeallocShmems()
 {
   for (IDMap<SharedMemory*>::const_iterator cit = mShmemMap.begin(); cit != mShmemMap.end(); ++cit) {
     Shmem::Dealloc(Shmem::PrivateIPDLCaller(), cit->second);
   }
   mShmemMap.Clear();
 }
 
 bool
-IToplevelProtocol::ShmemCreated(const Message& aMsg)
+IToplevelProtocol::ToplevelState::ShmemCreated(const Message& aMsg)
 {
   Shmem::id_t id;
   RefPtr<Shmem::SharedMemory> rawmem(Shmem::OpenExisting(Shmem::PrivateIPDLCaller(), aMsg, &id, true));
   if (!rawmem) {
     return false;
   }
   mShmemMap.AddWithID(rawmem.forget().take(), id);
   return true;
 }
 
 bool
-IToplevelProtocol::ShmemDestroyed(const Message& aMsg)
+IToplevelProtocol::ToplevelState::ShmemDestroyed(const Message& aMsg)
 {
   Shmem::id_t id;
   PickleIterator iter = PickleIterator(aMsg);
   if (!IPC::ReadParam(&aMsg, &iter, &id)) {
     return false;
   }
   aMsg.EndRead(iter);
 
--- a/ipc/glue/ProtocolUtils.h
+++ b/ipc/glue/ProtocolUtils.h
@@ -138,33 +138,90 @@ public:
     enum ActorDestroyReason {
         FailedConstructor,
         Deletion,
         AncestorDeletion,
         NormalShutdown,
         AbnormalShutdown
     };
 
+    // A lot of the functionality of IProtocol only differs between toplevel
+    // protocols (IToplevelProtocol) and managed protocols (everything else).
+    // If we put such functionality in IProtocol via virtual methods, that
+    // means that *every* protocol inherits that functionality through said
+    // virtual methods, then every protocol needs a (largely redundant)
+    // entry in its vtable.  That redundancy adds up quickly with several
+    // hundred protocols.
+    //
+    // This class (and its two subclasses) ensure that we don't have a bunch
+    // of redundant entries in protocol vtables: we have a single vtable per
+    // subclass, and then each protocol has its own instance of one of the
+    // subclasses.  This setup makes things a bit slower, but the space
+    // savings are worth it.
+    class ProtocolState
+    {
+    public:
+        virtual Shmem::SharedMemory* CreateSharedMemory(
+            size_t, SharedMemory::SharedMemoryType, bool, int32_t*) = 0;
+        virtual Shmem::SharedMemory* LookupSharedMemory(int32_t) = 0;
+        virtual bool IsTrackingSharedMemory(Shmem::SharedMemory*) = 0;
+        virtual bool DestroySharedMemory(Shmem&) = 0;
+        virtual ~ProtocolState() = default;
+    };
+
+    // Managed protocols just forward all of their operations to the topmost
+    // managing protocol.
+    class ManagedState final : public ProtocolState
+    {
+    public:
+        explicit ManagedState(IProtocol* aProtocol)
+            : mProtocol(aProtocol)
+        {}
+
+        Shmem::SharedMemory* CreateSharedMemory(
+            size_t, SharedMemory::SharedMemoryType, bool, int32_t*) override;
+        Shmem::SharedMemory* LookupSharedMemory(int32_t) override;
+        bool IsTrackingSharedMemory(Shmem::SharedMemory*) override;
+        bool DestroySharedMemory(Shmem&) override;
+
+    private:
+        IProtocol* const mProtocol;
+    };
+
     typedef base::ProcessId ProcessId;
     typedef IPC::Message Message;
     typedef IPC::MessageInfo MessageInfo;
 
-    IProtocol(Side aSide) : mId(0), mSide(aSide), mManager(nullptr), mChannel(nullptr) {}
+    explicit IProtocol(Side aSide)
+        : IProtocol(aSide, MakeUnique<ManagedState>(this))
+    {}
 
     virtual int32_t Register(IProtocol*);
     virtual int32_t RegisterID(IProtocol*, int32_t);
     virtual IProtocol* Lookup(int32_t);
     virtual void Unregister(int32_t);
     virtual void RemoveManagee(int32_t, IProtocol*) = 0;
 
-    virtual Shmem::SharedMemory* CreateSharedMemory(
-        size_t, SharedMemory::SharedMemoryType, bool, int32_t*);
-    virtual Shmem::SharedMemory* LookupSharedMemory(int32_t);
-    virtual bool IsTrackingSharedMemory(Shmem::SharedMemory*);
-    virtual bool DestroySharedMemory(Shmem&);
+    Shmem::SharedMemory* CreateSharedMemory(
+        size_t aSize, SharedMemory::SharedMemoryType aType, bool aUnsafe, int32_t* aId)
+    {
+        return mState->CreateSharedMemory(aSize, aType, aUnsafe, aId);
+    }
+    Shmem::SharedMemory* LookupSharedMemory(int32_t aId)
+    {
+        return mState->LookupSharedMemory(aId);
+    }
+    bool IsTrackingSharedMemory(Shmem::SharedMemory* aSegment)
+    {
+        return mState->IsTrackingSharedMemory(aSegment);
+    }
+    bool DestroySharedMemory(Shmem& aShmem)
+    {
+        return mState->DestroySharedMemory(aShmem);
+    }
 
     // XXX odd ducks, acknowledged
     virtual ProcessId OtherPid() const;
     Side GetSide() const { return mSide; }
 
     virtual const char* ProtocolName() const = 0;
     void FatalError(const char* const aErrorMsg) const;
     virtual void HandleFatalError(const char* aProtocolName, const char* aErrorMsg) const;
@@ -200,16 +257,24 @@ public:
     // unexpected behavior.
     void ReplaceEventTargetForActor(IProtocol* aActor,
                                     nsIEventTarget* aEventTarget);
 
     // Returns the event target set by SetEventTargetForActor() if available.
     virtual nsIEventTarget* GetActorEventTarget();
 
 protected:
+    IProtocol(Side aSide, UniquePtr<ProtocolState> aState)
+        : mId(0)
+        , mSide(aSide)
+        , mManager(nullptr)
+        , mChannel(nullptr)
+        , mState(Move(aState))
+    {}
+
     friend class IToplevelProtocol;
 
     void SetId(int32_t aId) { mId = aId; }
     void ResetManager() { mManager = nullptr; }
     void SetManager(IProtocol* aManager);
     void SetIPCChannel(MessageChannel* aChannel) { mChannel = aChannel; }
 
     virtual void SetEventTargetForActorInternal(IProtocol* aActor, nsIEventTarget* aEventTarget);
@@ -223,16 +288,17 @@ protected:
     static const int32_t kNullActorId = 0;
     static const int32_t kFreedActorId = 1;
 
 private:
     int32_t mId;
     Side mSide;
     IProtocol* mManager;
     MessageChannel* mChannel;
+    UniquePtr<ProtocolState> mState;
 };
 
 typedef IPCMessageStart ProtocolId;
 
 #define IPC_OK() mozilla::ipc::IPCResult::Ok()
 #define IPC_FAIL(actor, why) mozilla::ipc::IPCResult::Fail(WrapNotNull(actor), __func__, (why))
 #define IPC_FAIL_NO_REASON(actor) mozilla::ipc::IPCResult::Fail(WrapNotNull(actor), __func__)
 
@@ -272,16 +338,38 @@ protected:
 public:
     enum ProcessIdState {
         eUnstarted,
         ePending,
         eReady,
         eError
     };
 
+    class ToplevelState final : public ProtocolState
+    {
+    public:
+        ToplevelState(IToplevelProtocol* aProtocol, Side aSide);
+
+        Shmem::SharedMemory* CreateSharedMemory(
+            size_t, SharedMemory::SharedMemoryType, bool, int32_t*) override;
+        Shmem::SharedMemory* LookupSharedMemory(int32_t) override;
+        bool IsTrackingSharedMemory(Shmem::SharedMemory*) override;
+        bool DestroySharedMemory(Shmem&) override;
+
+        void DeallocShmems();
+
+        bool ShmemCreated(const Message& aMsg);
+        bool ShmemDestroyed(const Message& aMsg);
+
+    private:
+        IToplevelProtocol* const mProtocol;
+        IDMap<Shmem::SharedMemory*> mShmemMap;
+        Shmem::id_t mLastShmemId;
+    };
+
     using SchedulerGroupSet = nsILabelableRunnable::SchedulerGroupSet;
 
     void SetTransport(UniquePtr<Transport> aTrans)
     {
         mTrans = Move(aTrans);
     }
 
     Transport* GetTransport() const { return mTrans.get(); }
@@ -320,26 +408,20 @@ public:
 
     void SetReplyTimeoutMs(int32_t aTimeoutMs);
 
     virtual int32_t Register(IProtocol*) override;
     virtual int32_t RegisterID(IProtocol*, int32_t) override;
     virtual IProtocol* Lookup(int32_t) override;
     virtual void Unregister(int32_t) override;
 
-    virtual Shmem::SharedMemory* CreateSharedMemory(
-        size_t, SharedMemory::SharedMemoryType, bool, int32_t*) override;
-    virtual Shmem::SharedMemory* LookupSharedMemory(int32_t) override;
-    virtual bool IsTrackingSharedMemory(Shmem::SharedMemory*) override;
-    virtual bool DestroySharedMemory(Shmem&) override;
+    void DeallocShmems() { DowncastState()->DeallocShmems(); }
 
-    void DeallocShmems();
-
-    bool ShmemCreated(const Message& aMsg);
-    bool ShmemDestroyed(const Message& aMsg);
+    bool ShmemCreated(const Message& aMsg) { return DowncastState()->ShmemCreated(aMsg); }
+    bool ShmemDestroyed(const Message& aMsg) { return DowncastState()->ShmemDestroyed(aMsg); }
 
     virtual bool ShouldContinueFromReplyTimeout() {
         return false;
     }
 
     // WARNING: This function is called with the MessageChannel monitor held.
     virtual void IntentionalCrash() {
         MOZ_CRASH("Intentional IPDL crash");
@@ -419,16 +501,21 @@ public:
     GetActorEventTarget() override;
 
     virtual void OnChannelReceivedMessage(const Message& aMsg) {}
 
     bool IsMainThreadProtocol() const { return mIsMainThreadProtocol; }
     void SetIsMainThreadProtocol() { mIsMainThreadProtocol = NS_IsMainThread(); }
 
 protected:
+    ToplevelState* DowncastState() const
+    {
+        return static_cast<ToplevelState*>(mState.get());
+    }
+
     // Override this method in top-level protocols to change the event target
     // for a new actor (and its sub-actors).
     virtual already_AddRefed<nsIEventTarget>
     GetConstructedEventTarget(const Message& aMsg) { return nullptr; }
 
     // Override this method in top-level protocols to change the event target
     // for specific messages.
     virtual already_AddRefed<nsIEventTarget>
@@ -450,18 +537,16 @@ protected:
     base::ProcessId OtherPidMaybeInvalid() const;
 
     ProtocolId mProtocolId;
     UniquePtr<Transport> mTrans;
     base::ProcessId mOtherPid;
     ProcessIdState mOtherPidState;
     IDMap<IProtocol*> mActorMap;
     int32_t mLastRouteId;
-    IDMap<Shmem::SharedMemory*> mShmemMap;
-    Shmem::id_t mLastShmemId;
     bool mIsMainThreadProtocol;
 
     Mutex mEventTargetMutex;
     IDMap<nsCOMPtr<nsIEventTarget>> mEventTargetMap;
 };
 
 class IShmemAllocator
 {