Bug 1580524 - Free conduits when transceiver is stopped; r=bwc
authorDan Minor <dminor@mozilla.com>
Fri, 13 Sep 2019 17:44:06 +0000
changeset 493107 598d441e4ebaa93ab098d266035a396057c82129
parent 493106 f5dd266a1d0460abf9d2df433ad08e590edbd941
child 493108 f8d05174d092ffc02cd3b0690c04ed44b06f6760
child 493217 3d5a0f11a22275b85ac1724b9b2797c7fda500a8
push id114082
push userdvarga@mozilla.com
push dateFri, 13 Sep 2019 21:51:00 +0000
treeherdermozilla-inbound@598d441e4eba [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc
bugs1580524
milestone71.0a1
first release with
nightly linux32
598d441e4eba / 71.0a1 / 20190913214459 / files
nightly linux64
598d441e4eba / 71.0a1 / 20190913214459 / files
nightly mac
598d441e4eba / 71.0a1 / 20190913214459 / files
nightly win32
598d441e4eba / 71.0a1 / 20190913214459 / files
nightly win64
598d441e4eba / 71.0a1 / 20190913214459 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1580524 - Free conduits when transceiver is stopped; r=bwc Freeing the conduits when the transceiver is stopped releases all of the associated webrtc.org objects which results in substantial memory savings. On my system, with an opt+debug build and 200 stopped transceivers, I see 373.17 MB of memory use in the content process without this patch, and 158.93 MB of memory use with this patch applied. Going further and calling Shutdown_m as part of Stop() reduces the memory use to 157.98 MB, which seems like a marginal improvement at the cost of a much larger risk of introducing bugs. Differential Revision: https://phabricator.services.mozilla.com/D45845
media/webrtc/signaling/src/peerconnection/TransceiverImpl.cpp
--- a/media/webrtc/signaling/src/peerconnection/TransceiverImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/TransceiverImpl.cpp
@@ -261,17 +261,21 @@ nsresult TransceiverImpl::UpdatePrincipa
   // accessible to the script principal.
   static_cast<RemoteTrackSource&>(mReceiveTrack->GetSource())
       .SetPrincipal(aPrincipal);
 
   mReceivePipeline->SetPrincipalHandle_m(MakePrincipalHandle(aPrincipal));
   return NS_OK;
 }
 
-void TransceiverImpl::ResetSync() { mConduit->SetSyncGroup(""); }
+void TransceiverImpl::ResetSync() {
+  if (mConduit) {
+    mConduit->SetSyncGroup("");
+  }
+}
 
 nsresult TransceiverImpl::SyncWithMatchingVideoConduits(
     std::vector<RefPtr<TransceiverImpl>>& transceivers) {
   if (mJsepTransceiver->IsStopped()) {
     return NS_OK;
   }
 
   if (IsVideo()) {
@@ -282,16 +286,20 @@ nsresult TransceiverImpl::SyncWithMatchi
     return NS_ERROR_UNEXPECTED;
   }
 
   std::set<std::string> myReceiveStreamIds;
   myReceiveStreamIds.insert(mJsepTransceiver->mRecvTrack.GetStreamIds().begin(),
                             mJsepTransceiver->mRecvTrack.GetStreamIds().end());
 
   for (RefPtr<TransceiverImpl>& transceiver : transceivers) {
+    if (!transceiver->IsValid()) {
+      continue;
+    }
+
     if (!transceiver->IsVideo()) {
       // |this| is an audio transceiver, so we skip other audio transceivers
       continue;
     }
 
     // Maybe could make this more efficient by cacheing this set, but probably
     // not worth it.
     for (const std::string& streamId :
@@ -313,17 +321,17 @@ nsresult TransceiverImpl::SyncWithMatchi
       }
     }
   }
 
   return NS_OK;
 }
 
 bool TransceiverImpl::ConduitHasPluginID(uint64_t aPluginID) {
-  return mConduit->CodecPluginID() == aPluginID;
+  return mConduit ? mConduit->CodecPluginID() == aPluginID : false;
 }
 
 bool TransceiverImpl::HasSendTrack(
     const dom::MediaStreamTrack* aSendTrack) const {
   if (!mSendTrack) {
     return false;
   }
 
@@ -624,16 +632,18 @@ static nsresult NegotiatedDetailsToAudio
     MOZ_MTLOG(ML_ERROR, "Can't set up a conduit with 0 codecs");
     return NS_ERROR_FAILURE;
   }
 
   return NS_OK;
 }
 
 nsresult TransceiverImpl::UpdateAudioConduit() {
+  MOZ_ASSERT(IsValid());
+
   RefPtr<AudioSessionConduit> conduit =
       static_cast<AudioSessionConduit*>(mConduit.get());
 
   if (!mJsepTransceiver->mRecvTrack.GetSsrcs().empty()) {
     MOZ_MTLOG(ML_DEBUG, mPCHandle
                             << "[" << mMid << "]: " << __FUNCTION__
                             << " Setting remote SSRC "
                             << mJsepTransceiver->mRecvTrack.GetSsrcs().front());
@@ -775,16 +785,18 @@ static nsresult NegotiatedDetailsToVideo
       aConfigs->push_back(std::move(config));
     }
   }
 
   return NS_OK;
 }
 
 nsresult TransceiverImpl::UpdateVideoConduit() {
+  MOZ_ASSERT(IsValid());
+
   RefPtr<VideoSessionConduit> conduit =
       static_cast<VideoSessionConduit*>(mConduit.get());
 
   // NOTE(pkerr) - this is new behavior. Needed because the
   // CreateVideoReceiveStream method of the Call API will assert (in debug) and
   // fail if a value is not provided for the remote_ssrc that will be used by
   // the far-end sender.
   if (!mJsepTransceiver->mRecvTrack.GetSsrcs().empty()) {
@@ -914,16 +926,20 @@ nsresult TransceiverImpl::ConfigureVideo
   }
 
   return NS_OK;
 }
 
 void TransceiverImpl::UpdateConduitRtpExtmap(
     const JsepTrackNegotiatedDetails& aDetails,
     const LocalDirection aDirection) {
+  if (!IsValid()) {
+    return;
+  }
+
   std::vector<webrtc::RtpExtension> extmaps;
   // @@NG read extmap from track
   aDetails.ForEachRTPHeaderExtension(
       [&extmaps](const SdpExtmapAttributeList::Extmap& extmap) {
         extmaps.emplace_back(extmap.extensionname, extmap.entry);
       });
 
   RefPtr<VideoSessionConduit> conduit =
@@ -935,38 +951,44 @@ void TransceiverImpl::UpdateConduitRtpEx
 }
 
 void TransceiverImpl::Stop() {
   mTransmitPipeline->Shutdown_m();
   mReceivePipeline->Shutdown_m();
   // Make sure that stats queries stop working on this transceiver.
   UpdateSendTrack(nullptr);
   mHaveStartedReceiving = false;
+
+  if (mConduit) {
+    mConduit->DeleteStreams();
+  }
+  mConduit = nullptr;
 }
 
 bool TransceiverImpl::IsVideo() const {
   return mJsepTransceiver->GetMediaType() == SdpMediaSection::MediaType::kVideo;
 }
 
 void TransceiverImpl::GetRtpSources(
     const int64_t aTimeNow,
     nsTArray<dom::RTCRtpSourceEntry>& outSources) const {
-  if (IsVideo()) {
+  if (!IsValid() || IsVideo()) {
     return;
   }
+
   WebrtcAudioConduit* audio_conduit =
       static_cast<WebrtcAudioConduit*>(mConduit.get());
   audio_conduit->GetRtpSources(aTimeNow, outSources);
 }
 
 void TransceiverImpl::InsertAudioLevelForContributingSource(uint32_t aSource,
                                                             int64_t aTimestamp,
                                                             bool aHasLevel,
                                                             uint8_t aLevel) {
-  if (IsVideo()) {
+  if (!IsValid() || IsVideo()) {
     return;
   }
   WebrtcAudioConduit* audio_conduit =
       static_cast<WebrtcAudioConduit*>(mConduit.get());
   audio_conduit->InsertAudioLevelForContributingSource(aSource, aTimestamp,
                                                        aHasLevel, aLevel);
 }