From db59de3e59d82319e3bb64549400125609fc6d0d Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Wed, 26 Jun 2019 11:06:41 +0200 Subject: [PATCH] Add optimization to PacketRouter for large number of senders. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove expectation in PacketRouter tests for exact number const accessors are called Bug: None Change-Id: I79c08f0c802b0c863adb133819d32e0b9203e721 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/143799 Reviewed-by: Erik Språng Reviewed-by: Sebastian Jansson Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#28387} --- modules/pacing/packet_router.cc | 42 ++++++++++++++++-------- modules/pacing/packet_router.h | 7 ++++ modules/pacing/packet_router_unittest.cc | 40 +++++++++++----------- 3 files changed, 56 insertions(+), 33 deletions(-) diff --git a/modules/pacing/packet_router.cc b/modules/pacing/packet_router.cc index 6d2c7ff337..791f54e939 100644 --- a/modules/pacing/packet_router.cc +++ b/modules/pacing/packet_router.cc @@ -67,6 +67,7 @@ void PacketRouter::AddSendRtpModule(RtpRtcp* rtp_module, bool remb_candidate) { void PacketRouter::RemoveSendRtpModule(RtpRtcp* rtp_module) { rtc::CritScope cs(&modules_crit_); + rtp_module_cache_map_.clear(); MaybeRemoveRembModuleCandidate(rtp_module, /* media_sender = */ true); auto it = std::find(rtp_send_modules_.begin(), rtp_send_modules_.end(), rtp_module); @@ -108,23 +109,38 @@ RtpPacketSendResult PacketRouter::TimeToSendPacket( bool retransmission, const PacedPacketInfo& pacing_info) { rtc::CritScope cs(&modules_crit_); - for (auto* rtp_module : rtp_send_modules_) { - if (!rtp_module->SendingMedia()) { - continue; + RtpRtcp* rtp_module = FindRtpModule(ssrc); + if (rtp_module == nullptr || !rtp_module->SendingMedia()) { + return RtpPacketSendResult::kPacketNotFound; + } + + if ((rtp_module->RtxSendStatus() & kRtxRedundantPayloads) && + rtp_module->HasBweExtensions()) { + // This is now the last module to send media, and has the desired + // properties needed for payload based padding. Cache it for later use. + last_send_module_ = rtp_module; + } + return rtp_module->TimeToSendPacket(ssrc, sequence_number, capture_timestamp, + retransmission, pacing_info); +} + +RtpRtcp* PacketRouter::FindRtpModule(uint32_t ssrc) { + auto it = rtp_module_cache_map_.find(ssrc); + if (it != rtp_module_cache_map_.end()) { + if (ssrc == it->second->SSRC() || ssrc == it->second->FlexfecSsrc()) { + return it->second; } + // This entry is stale due to a changed ssrc - remove it. + rtp_module_cache_map_.erase(it); + } + // Slow path - find and cache matching module + for (RtpRtcp* rtp_module : rtp_send_modules_) { if (ssrc == rtp_module->SSRC() || ssrc == rtp_module->FlexfecSsrc()) { - if ((rtp_module->RtxSendStatus() & kRtxRedundantPayloads) && - rtp_module->HasBweExtensions()) { - // This is now the last module to send media, and has the desired - // properties needed for payload based padding. Cache it for later use. - last_send_module_ = rtp_module; - } - return rtp_module->TimeToSendPacket(ssrc, sequence_number, - capture_timestamp, retransmission, - pacing_info); + rtp_module_cache_map_[ssrc] = rtp_module; + return rtp_module; } } - return RtpPacketSendResult::kPacketNotFound; + return nullptr; } void PacketRouter::SendPacket(std::unique_ptr packet, diff --git a/modules/pacing/packet_router.h b/modules/pacing/packet_router.h index 9a51899438..0ca4607d61 100644 --- a/modules/pacing/packet_router.h +++ b/modules/pacing/packet_router.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include "api/transport/network_types.h" @@ -86,6 +87,9 @@ class PacketRouter : public TransportSequenceNumberAllocator, bool SendTransportFeedback(rtcp::TransportFeedback* packet) override; private: + RtpRtcp* FindRtpModule(uint32_t ssrc) + RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_crit_); + void AddRembModuleCandidate(RtcpFeedbackSenderInterface* candidate_module, bool media_sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_crit_); @@ -98,6 +102,9 @@ class PacketRouter : public TransportSequenceNumberAllocator, rtc::CriticalSection modules_crit_; // Rtp and Rtcp modules of the rtp senders. std::list rtp_send_modules_ RTC_GUARDED_BY(modules_crit_); + // Ssrc to RtpRtcp module cache. + std::unordered_map rtp_module_cache_map_ + RTC_GUARDED_BY(modules_crit_); // The last module used to send media. RtpRtcp* last_send_module_ RTC_GUARDED_BY(modules_crit_); // Rtcp modules of the rtp receivers. diff --git a/modules/pacing/packet_router_unittest.cc b/modules/pacing/packet_router_unittest.cc index df7619afad..1677b0ce3d 100644 --- a/modules/pacing/packet_router_unittest.cc +++ b/modules/pacing/packet_router_unittest.cc @@ -108,14 +108,14 @@ TEST(PacketRouterTest, TimeToSendPacket) { bool retransmission = false; // Send on the first module by letting rtp_1 be sending with correct ssrc. - EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true)); - EXPECT_CALL(rtp_1, SSRC()).Times(1).WillOnce(Return(kSsrc1)); + ON_CALL(rtp_1, SendingMedia).WillByDefault(Return(true)); + ON_CALL(rtp_1, SSRC).WillByDefault(Return(kSsrc1)); EXPECT_CALL(rtp_1, TimeToSendPacket( kSsrc1, sequence_number, timestamp, retransmission, Field(&PacedPacketInfo::probe_cluster_id, 1))) .Times(1) .WillOnce(Return(RtpPacketSendResult::kSuccess)); - EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _, _)).Times(0); + EXPECT_CALL(rtp_2, TimeToSendPacket).Times(0); EXPECT_EQ(RtpPacketSendResult::kSuccess, packet_router.TimeToSendPacket( kSsrc1, sequence_number, timestamp, retransmission, @@ -126,10 +126,10 @@ TEST(PacketRouterTest, TimeToSendPacket) { timestamp += 30; retransmission = true; const uint16_t kSsrc2 = 4567; - EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(false)); - EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true)); - EXPECT_CALL(rtp_2, SSRC()).Times(1).WillOnce(Return(kSsrc2)); - EXPECT_CALL(rtp_1, TimeToSendPacket(_, _, _, _, _)).Times(0); + ON_CALL(rtp_1, SendingMedia).WillByDefault(Return(false)); + ON_CALL(rtp_2, SendingMedia).WillByDefault(Return(true)); + ON_CALL(rtp_2, SSRC).WillByDefault(Return(kSsrc2)); + EXPECT_CALL(rtp_1, TimeToSendPacket).Times(0); EXPECT_CALL(rtp_2, TimeToSendPacket( kSsrc2, sequence_number, timestamp, retransmission, Field(&PacedPacketInfo::probe_cluster_id, 2))) @@ -141,22 +141,22 @@ TEST(PacketRouterTest, TimeToSendPacket) { PacedPacketInfo(2, kProbeMinProbes, kProbeMinBytes))); // No module is sending, hence no packet should be sent. - EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(false)); - EXPECT_CALL(rtp_1, TimeToSendPacket(_, _, _, _, _)).Times(0); - EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(false)); - EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _, _)).Times(0); + ON_CALL(rtp_1, SendingMedia).WillByDefault(Return(false)); + ON_CALL(rtp_2, SendingMedia).WillByDefault(Return(false)); + EXPECT_CALL(rtp_1, TimeToSendPacket).Times(0); + EXPECT_CALL(rtp_2, TimeToSendPacket).Times(0); EXPECT_EQ(RtpPacketSendResult::kPacketNotFound, packet_router.TimeToSendPacket( kSsrc1, sequence_number, timestamp, retransmission, PacedPacketInfo(1, kProbeMinProbes, kProbeMinBytes))); // Add a packet with incorrect ssrc and test it's dropped in the router. - EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true)); - EXPECT_CALL(rtp_1, SSRC()).Times(1).WillOnce(Return(kSsrc1)); - EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true)); - EXPECT_CALL(rtp_2, SSRC()).Times(1).WillOnce(Return(kSsrc2)); - EXPECT_CALL(rtp_1, TimeToSendPacket(_, _, _, _, _)).Times(0); - EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _, _)).Times(0); + ON_CALL(rtp_1, SendingMedia).WillByDefault(Return(true)); + ON_CALL(rtp_1, SSRC).WillByDefault(Return(kSsrc1)); + ON_CALL(rtp_2, SendingMedia).WillByDefault(Return(true)); + ON_CALL(rtp_2, SSRC).WillByDefault(Return(kSsrc2)); + EXPECT_CALL(rtp_1, TimeToSendPacket).Times(0); + EXPECT_CALL(rtp_2, TimeToSendPacket).Times(0); EXPECT_EQ(RtpPacketSendResult::kPacketNotFound, packet_router.TimeToSendPacket( kSsrc1 + kSsrc2, sequence_number, timestamp, retransmission, @@ -166,9 +166,9 @@ TEST(PacketRouterTest, TimeToSendPacket) { // rtp_1 has been removed, try sending a packet on that ssrc and make sure // it is dropped as expected by not expecting any calls to rtp_1. - EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true)); - EXPECT_CALL(rtp_2, SSRC()).Times(1).WillOnce(Return(kSsrc2)); - EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _, _)).Times(0); + ON_CALL(rtp_2, SendingMedia).WillByDefault(Return(true)); + ON_CALL(rtp_2, SSRC).WillByDefault(Return(kSsrc2)); + EXPECT_CALL(rtp_2, TimeToSendPacket).Times(0); EXPECT_EQ(RtpPacketSendResult::kPacketNotFound, packet_router.TimeToSendPacket( kSsrc1, sequence_number, timestamp, retransmission,