zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Clarify thread/TaskQueue requirements for internal::CallStats

Browse Source 
Bug: webrtc:11581
Change-Id: Idec96b14e61d9f9c53dd81fa4325b5ed63da448e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/197424
Reviewed-by: Tommi <tommi@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32835}
This commit is contained in:
Niels Möller
2020-12-14 12:12:37 +01:00
committed by Commit Bot
parent 16dd6b0ce4
commit c72733dc95
2 changed files with 17 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
video/call_stats2.cc
View File

@ -76,7 +76,7 @@ CallStats::CallStats(Clock* clock, TaskQueueBase* task_queue)
time_of_first_rtt_ms_(-1),
task_queue_(task_queue) {
RTC_DCHECK(task_queue_);
process_thread_checker_.Detach();
RTC_DCHECK_RUN_ON(task_queue_);
repeating_task_ =
RepeatingTaskHandle::DelayedStart(task_queue_, kUpdateInterval, [this]() {
UpdateAndReport();
@ -85,7 +85,7 @@ CallStats::CallStats(Clock* clock, TaskQueueBase* task_queue)
}
CallStats::~CallStats() {
RTC_DCHECK_RUN_ON(&construction_thread_checker_);
RTC_DCHECK_RUN_ON(task_queue_);
RTC_DCHECK(observers_.empty());
repeating_task_.Stop();
@ -94,7 +94,7 @@ CallStats::~CallStats() {
}
void CallStats::UpdateAndReport() {
RTC_DCHECK_RUN_ON(&construction_thread_checker_);
RTC_DCHECK_RUN_ON(task_queue_);
RemoveOldReports(clock_->CurrentTime().ms(), &reports_);
max_rtt_ms_ = GetMaxRttMs(reports_);
@ -112,18 +112,18 @@ void CallStats::UpdateAndReport() {
}
void CallStats::RegisterStatsObserver(CallStatsObserver* observer) {
RTC_DCHECK_RUN_ON(&construction_thread_checker_);
RTC_DCHECK_RUN_ON(task_queue_);
if (!absl::c_linear_search(observers_, observer))
observers_.push_back(observer);
}
void CallStats::DeregisterStatsObserver(CallStatsObserver* observer) {
RTC_DCHECK_RUN_ON(&construction_thread_checker_);
RTC_DCHECK_RUN_ON(task_queue_);
observers_.remove(observer);
}
int64_t CallStats::LastProcessedRtt() const {
RTC_DCHECK_RUN_ON(&construction_thread_checker_);
RTC_DCHECK_RUN_ON(task_queue_);
// No need for locking since we're on the construction thread.
return avg_rtt_ms_;
}
@ -134,7 +134,7 @@ void CallStats::OnRttUpdate(int64_t rtt) {
// on the correct TQ.
int64_t now_ms = clock_->TimeInMilliseconds();
auto update = [this, rtt, now_ms]() {
RTC_DCHECK_RUN_ON(&construction_thread_checker_);
RTC_DCHECK_RUN_ON(task_queue_);
reports_.push_back(RttTime(rtt, now_ms));
if (time_of_first_rtt_ms_ == -1)
time_of_first_rtt_ms_ = now_ms;
@ -149,7 +149,7 @@ void CallStats::OnRttUpdate(int64_t rtt) {
}
void CallStats::UpdateHistograms() {
RTC_DCHECK_RUN_ON(&construction_thread_checker_);
RTC_DCHECK_RUN_ON(task_queue_);
if (time_of_first_rtt_ms_ == -1 || num_avg_rtt_ < 1)
return;

35
video/call_stats2.h
View File

@ -18,8 +18,6 @@
#include "modules/include/module_common_types.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "rtc_base/constructor_magic.h"
#include "rtc_base/synchronization/sequence_checker.h"
#include "rtc_base/system/no_unique_address.h"
#include "rtc_base/task_queue.h"
#include "rtc_base/task_utils/pending_task_safety_flag.h"
#include "rtc_base/task_utils/repeating_task.h"
@ -33,6 +31,7 @@ class CallStats {
// Time interval for updating the observers.
static constexpr TimeDelta kUpdateInterval = TimeDelta::Millis(1000);
// Must be created and destroyed on the same task_queue.
CallStats(Clock* clock, TaskQueueBase* task_queue);
~CallStats();
@ -50,11 +49,6 @@ class CallStats {
// Expose |LastProcessedRtt()| from RtcpRttStats to the public interface, as
// it is the part of the API that is needed by direct users of CallStats.
// TODO(tommi): Threading or lifetime guarantees are not explicit in how
// CallStats is used as RtcpRttStats or how pointers are cached in a
// few different places (distributed via Call). It would be good to clarify
// from what thread/TQ calls to OnRttUpdate and LastProcessedRtt need to be
// allowed.
int64_t LastProcessedRtt() const;
// Exposed for tests to test histogram support.
@ -106,35 +100,24 @@ class CallStats {
Clock* const clock_;
// Used to regularly call UpdateAndReport().
RepeatingTaskHandle repeating_task_
RTC_GUARDED_BY(construction_thread_checker_);
RepeatingTaskHandle repeating_task_ RTC_GUARDED_BY(task_queue_);
// The last RTT in the statistics update (zero if there is no valid estimate).
int64_t max_rtt_ms_ RTC_GUARDED_BY(construction_thread_checker_);
int64_t max_rtt_ms_ RTC_GUARDED_BY(task_queue_);
// Last reported average RTT value.
int64_t avg_rtt_ms_ RTC_GUARDED_BY(construction_thread_checker_);
int64_t avg_rtt_ms_ RTC_GUARDED_BY(task_queue_);
// |sum_avg_rtt_ms_|, |num_avg_rtt_| and |time_of_first_rtt_ms_| are only used
// on the ProcessThread when running. When the Process Thread is not running,
// (and only then) they can be used in UpdateHistograms(), usually called from
// the dtor.
int64_t sum_avg_rtt_ms_ RTC_GUARDED_BY(construction_thread_checker_);
int64_t num_avg_rtt_ RTC_GUARDED_BY(construction_thread_checker_);
int64_t time_of_first_rtt_ms_ RTC_GUARDED_BY(construction_thread_checker_);
int64_t sum_avg_rtt_ms_ RTC_GUARDED_BY(task_queue_);
int64_t num_avg_rtt_ RTC_GUARDED_BY(task_queue_);
int64_t time_of_first_rtt_ms_ RTC_GUARDED_BY(task_queue_);
// All Rtt reports within valid time interval, oldest first.
std::list<RttTime> reports_ RTC_GUARDED_BY(construction_thread_checker_);
std::list<RttTime> reports_ RTC_GUARDED_BY(task_queue_);
// Observers getting stats reports.
// When attached to ProcessThread, this is read-only. In order to allow
// modification, we detach from the process thread while the observer
// list is updated, to avoid races. This allows us to not require a lock
// for the observers_ list, which makes the most common case lock free.
std::list<CallStatsObserver*> observers_;
std::list<CallStatsObserver*> observers_ RTC_GUARDED_BY(task_queue_);
RTC_NO_UNIQUE_ADDRESS SequenceChecker construction_thread_checker_;
RTC_NO_UNIQUE_ADDRESS SequenceChecker process_thread_checker_;
TaskQueueBase* const task_queue_;
// Used to signal destruction to potentially pending tasks.