[Stats] Optimization: Minimize number of thread-invokes in getStats().
TrackMediaInfoMap was previously constructed on the signaling thread. It
would iterate all the senders and receivers and perform GetParameters(),
which internally would invoke on the worker thread. This resulted in as
many thread-invokes as number of receivers.
With this CL we piggyback on an existing thread-invoke, performing a
single blocking invoke for all transceivers. This is good for
performance when there is a lot of thread contention.
The code is already exercised by unit tests and integration tests.
rtc::Thread::ScopedDisallowBlockingCalls is added to DCHECK that we
don't accidentally do any other blocking invokes.
A couple of unnecessary DCHECKs had to be removed to avoid PROXY
invokes back to the signaling thread (deadlock). These DCHECKs won't be
missed.
Bug: webrtc:11716
Change-Id: I139c7434682ff627bb88351b5752320dd322d9eb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178816
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31666}
diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc
index 07e40cb..e1527d5 100644
--- a/pc/rtc_stats_collector.cc
+++ b/pc/rtc_stats_collector.cc
@@ -1868,9 +1868,12 @@
}
}
- // Call GetStats for all media channels together on the worker thread in one
- // hop.
+ // We jump to the worker thread and call GetStats() on each media channel. At
+ // the same time we construct the TrackMediaInfoMaps, which also needs info
+ // from the worker thread. This minimizes the number of thread jumps.
worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
for (const auto& entry : voice_stats) {
if (!entry.first->GetStats(entry.second.get())) {
RTC_LOG(LS_WARNING) << "Failed to get voice stats.";
@@ -1881,41 +1884,41 @@
RTC_LOG(LS_WARNING) << "Failed to get video stats.";
}
}
- });
- // Create the TrackMediaInfoMap for each transceiver stats object.
- for (auto& stats : transceiver_stats_infos) {
- auto transceiver = stats.transceiver;
- std::unique_ptr<cricket::VoiceMediaInfo> voice_media_info;
- std::unique_ptr<cricket::VideoMediaInfo> video_media_info;
- if (transceiver->channel()) {
- cricket::MediaType media_type = transceiver->media_type();
- if (media_type == cricket::MEDIA_TYPE_AUDIO) {
- auto* voice_channel =
- static_cast<cricket::VoiceChannel*>(transceiver->channel());
- RTC_DCHECK(voice_stats[voice_channel->media_channel()]);
- voice_media_info =
- std::move(voice_stats[voice_channel->media_channel()]);
- } else if (media_type == cricket::MEDIA_TYPE_VIDEO) {
- auto* video_channel =
- static_cast<cricket::VideoChannel*>(transceiver->channel());
- RTC_DCHECK(video_stats[video_channel->media_channel()]);
- video_media_info =
- std::move(video_stats[video_channel->media_channel()]);
+ // Create the TrackMediaInfoMap for each transceiver stats object.
+ for (auto& stats : transceiver_stats_infos) {
+ auto transceiver = stats.transceiver;
+ std::unique_ptr<cricket::VoiceMediaInfo> voice_media_info;
+ std::unique_ptr<cricket::VideoMediaInfo> video_media_info;
+ if (transceiver->channel()) {
+ cricket::MediaType media_type = transceiver->media_type();
+ if (media_type == cricket::MEDIA_TYPE_AUDIO) {
+ auto* voice_channel =
+ static_cast<cricket::VoiceChannel*>(transceiver->channel());
+ RTC_DCHECK(voice_stats[voice_channel->media_channel()]);
+ voice_media_info =
+ std::move(voice_stats[voice_channel->media_channel()]);
+ } else if (media_type == cricket::MEDIA_TYPE_VIDEO) {
+ auto* video_channel =
+ static_cast<cricket::VideoChannel*>(transceiver->channel());
+ RTC_DCHECK(video_stats[video_channel->media_channel()]);
+ video_media_info =
+ std::move(video_stats[video_channel->media_channel()]);
+ }
}
+ std::vector<rtc::scoped_refptr<RtpSenderInternal>> senders;
+ for (const auto& sender : transceiver->senders()) {
+ senders.push_back(sender->internal());
+ }
+ std::vector<rtc::scoped_refptr<RtpReceiverInternal>> receivers;
+ for (const auto& receiver : transceiver->receivers()) {
+ receivers.push_back(receiver->internal());
+ }
+ stats.track_media_info_map = std::make_unique<TrackMediaInfoMap>(
+ std::move(voice_media_info), std::move(video_media_info), senders,
+ receivers);
}
- std::vector<rtc::scoped_refptr<RtpSenderInternal>> senders;
- for (const auto& sender : transceiver->senders()) {
- senders.push_back(sender->internal());
- }
- std::vector<rtc::scoped_refptr<RtpReceiverInternal>> receivers;
- for (const auto& receiver : transceiver->receivers()) {
- receivers.push_back(receiver->internal());
- }
- stats.track_media_info_map = std::make_unique<TrackMediaInfoMap>(
- std::move(voice_media_info), std::move(video_media_info), senders,
- receivers);
- }
+ });
return transceiver_stats_infos;
}
diff --git a/pc/track_media_info_map.cc b/pc/track_media_info_map.cc
index ca923a0..b3ec68b 100644
--- a/pc/track_media_info_map.cc
+++ b/pc/track_media_info_map.cc
@@ -14,6 +14,8 @@
#include <string>
#include <utility>
+#include "rtc_base/thread.h"
+
namespace webrtc {
namespace {
@@ -43,20 +45,12 @@
RTC_DCHECK(local_video_track_by_ssrc->empty());
RTC_DCHECK(remote_audio_track_by_ssrc->empty());
RTC_DCHECK(remote_video_track_by_ssrc->empty());
- // TODO(hbos): RTP senders/receivers uses a proxy to the signaling thread, and
- // our sender/receiver implementations invokes on the worker thread. (This
- // means one thread jump if on signaling thread and two thread jumps if on any
- // other threads). Is there a way to avoid thread jump(s) on a per
- // sender/receiver, per method basis?
for (const auto& rtp_sender : rtp_senders) {
cricket::MediaType media_type = rtp_sender->media_type();
MediaStreamTrackInterface* track = rtp_sender->track();
if (!track) {
continue;
}
- RTC_DCHECK_EQ(track->kind(), media_type == cricket::MEDIA_TYPE_AUDIO
- ? MediaStreamTrackInterface::kAudioKind
- : MediaStreamTrackInterface::kVideoKind);
// TODO(deadbeef): |ssrc| should be removed in favor of |GetParameters|.
uint32_t ssrc = rtp_sender->ssrc();
if (ssrc != 0) {
@@ -77,9 +71,6 @@
cricket::MediaType media_type = rtp_receiver->media_type();
MediaStreamTrackInterface* track = rtp_receiver->track();
RTC_DCHECK(track);
- RTC_DCHECK_EQ(track->kind(), media_type == cricket::MEDIA_TYPE_AUDIO
- ? MediaStreamTrackInterface::kAudioKind
- : MediaStreamTrackInterface::kVideoKind);
RtpParameters params = rtp_receiver->GetParameters();
for (const RtpEncodingParameters& encoding : params.encodings) {
if (!encoding.ssrc) {
@@ -115,6 +106,8 @@
const std::vector<rtc::scoped_refptr<RtpReceiverInternal>>& rtp_receivers)
: voice_media_info_(std::move(voice_media_info)),
video_media_info_(std::move(video_media_info)) {
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
std::map<uint32_t, AudioTrackInterface*> local_audio_track_by_ssrc;
std::map<uint32_t, VideoTrackInterface*> local_video_track_by_ssrc;
std::map<uint32_t, AudioTrackInterface*> remote_audio_track_by_ssrc;
