Add utility to count the number of blocking thread invokes.
This is useful to understand how often we block in certain parts of the
api and track improvements/regressions.
There are two macros, both are only active for RTC_DCHECK_IS_ON builds:
* RTC_LOG_THREAD_BLOCK_COUNT()
Example:
void MyClass::MyFunction() {
RTC_LOG_THREAD_BLOCK_COUNT();
thread_->Invoke<void>([this](){ DoStuff(); });
}
When executing this function during a test, the output could be:
(my_file.cc:2): Blocking MyFunction: total=1 (actual=1, would=0)
The words 'actual' and 'would' reflect whether an actual thread switch
was made, or if in the case of a test using the same thread for more
than one role (e.g. signaling, worker, network are all the same thread)
that an actual thread switch did not occur but it would have occurred
in the case of having dedicated threads. The 'total' count is the sum.
* RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(x)
Example:
void MyClass::MyFunction() {
RTC_LOG_THREAD_BLOCK_COUNT();
thread_->Invoke<void>([this](){ DoStuff(); });
thread_->Invoke<void>([this](){ MoreStuff(); });
RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(1);
}
When a function is known to have blocking calls and we want to not
regress from the currently known number of blocking calls, we can use
this macro to state that at a certain point in a function, below
where RTC_LOG_THREAD_BLOCK_COUNT() is called, there must have occurred
no more than |x| (total) blocking calls. If more occur, a DCHECK will
hit and print out what the actual number of calls was:
# Fatal error in: my_file.cc, line 5
# last system error: 60
# Check failed: blocked_call_count_printer.GetTotalBlockedCallCount() <= 1 (2 vs. 1)
Bug: webrtc:12649
Change-Id: Ibac4f85f00b89680601dba54a651eac95a0f45d3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/213782
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33632}
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 1381bf9..2d9f9c8 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -4700,11 +4700,29 @@
rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>
transceiver) {
RTC_DCHECK(transceiver);
+ RTC_LOG_THREAD_BLOCK_COUNT();
+
+ // TODO(tommi): We're currently on the signaling thread.
+ // There are multiple hops to the worker ahead.
+ // Consider if we can make the call to SetChannel() on the worker thread
+ // (and require that to be the context it's always called in) and also
+ // call DestroyChannelInterface there, since it also needs to hop to the
+ // worker.
cricket::ChannelInterface* channel = transceiver->internal()->channel();
+ RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(0);
if (channel) {
+ // TODO(tommi): VideoRtpReceiver::SetMediaChannel blocks and jumps to the
+ // worker thread. When being set to nullptr, there are additional blocking
+ // calls to e.g. ClearRecordableEncodedFrameCallback which triggers another
+ // blocking call or Stop() for video channels.
transceiver->internal()->SetChannel(nullptr);
+ RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(2);
+ // TODO(tommi): All channel objects end up getting deleted on the
+ // worker thread. Can DestroyTransceiverChannel be purely posted to the
+ // worker?
DestroyChannelInterface(channel);
+ RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(3);
}
}
@@ -4734,6 +4752,9 @@
// DestroyChannelInterface to either be called on the worker thread, or do
// this asynchronously on the worker.
RTC_DCHECK(channel);
+
+ RTC_LOG_THREAD_BLOCK_COUNT();
+
switch (channel->media_type()) {
case cricket::MEDIA_TYPE_AUDIO:
channel_manager()->DestroyVoiceChannel(
@@ -4751,6 +4772,10 @@
RTC_NOTREACHED() << "Unknown media type: " << channel->media_type();
break;
}
+
+ // TODO(tommi): Figure out why we can get 2 blocking calls when running
+ // PeerConnectionCryptoTest.CreateAnswerWithDifferentSslRoles.
+ RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(2);
}
void SdpOfferAnswerHandler::DestroyAllChannels() {
@@ -4758,18 +4783,25 @@
if (!transceivers()) {
return;
}
+
+ RTC_LOG_THREAD_BLOCK_COUNT();
+
// Destroy video channels first since they may have a pointer to a voice
// channel.
- for (const auto& transceiver : transceivers()->List()) {
+ auto list = transceivers()->List();
+ RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(0);
+
+ for (const auto& transceiver : list) {
if (transceiver->media_type() == cricket::MEDIA_TYPE_VIDEO) {
DestroyTransceiverChannel(transceiver);
}
}
- for (const auto& transceiver : transceivers()->List()) {
+ for (const auto& transceiver : list) {
if (transceiver->media_type() == cricket::MEDIA_TYPE_AUDIO) {
DestroyTransceiverChannel(transceiver);
}
}
+
DestroyDataChannelTransport();
}
