From 0e6758d7efc64a93d27f45b278d867d7f13f2131 Mon Sep 17 00:00:00 2001 From: hbos Date: Wed, 31 Aug 2016 07:57:36 -0700 Subject: [PATCH] RTCStatsCollector: timestamps updated. Using a timestamp based on a timer that is monotonically increasing for the cache, so that cache's freshness can be checked regardless of if system clock is modified. Using a system clock for the stats' timestamp, which needs to be relative to UNIX epoch (Jan 1, 1970, UTC). This CL removes the dependency on faketiming.h. BUG=chromium:627816 NOTRY=True Review-Url: https://codereview.webrtc.org/2299643002 Cr-Commit-Position: refs/heads/master@{#13997} --- webrtc/api/rtcstats.h | 18 +++--- webrtc/api/rtcstats_objects.h | 4 +- webrtc/stats/rtcstats.cc | 2 +- webrtc/stats/rtcstats_objects.cc | 8 +-- webrtc/stats/rtcstats_unittest.cc | 16 ++--- webrtc/stats/rtcstatscollector.cc | 32 ++++++---- webrtc/stats/rtcstatscollector.h | 19 +++--- webrtc/stats/rtcstatscollector_unittest.cc | 19 ++++-- webrtc/stats/rtcstatsreport_unittest.cc | 72 +++++++++++----------- 9 files changed, 102 insertions(+), 88 deletions(-) diff --git a/webrtc/api/rtcstats.h b/webrtc/api/rtcstats.h index 4b2d70ff97..1196e97373 100644 --- a/webrtc/api/rtcstats.h +++ b/webrtc/api/rtcstats.h @@ -49,17 +49,17 @@ class RTCStatsMemberInterface; // } class RTCStats { public: - RTCStats(const std::string& id, double timestamp) - : id_(id), timestamp_(timestamp) {} - RTCStats(std::string&& id, double timestamp) - : id_(std::move(id)), timestamp_(timestamp) {} + RTCStats(const std::string& id, int64_t timestamp_us) + : id_(id), timestamp_us_(timestamp_us) {} + RTCStats(std::string&& id, int64_t timestamp_us) + : id_(std::move(id)), timestamp_us_(timestamp_us) {} virtual ~RTCStats() {} virtual std::unique_ptr copy() const = 0; const std::string& id() const { return id_; } - // Time relative to the UNIX epoch (Jan 1, 1970, UTC), in seconds. - double timestamp() const { return timestamp_; } + // Time relative to the UNIX epoch (Jan 1, 1970, UTC), in microseconds. + int64_t timestamp_us() const { return timestamp_us_; } // Returns the static member variable |kType| of the implementing class. virtual const char* type() const = 0; // Returns a vector of pointers to all the RTCStatsMemberInterface members of @@ -88,7 +88,7 @@ class RTCStats { size_t additional_capacity) const; std::string const id_; - double timestamp_; + int64_t timestamp_us_; }; // All |RTCStats| classes should use this macro in a public section of the class @@ -112,8 +112,8 @@ class RTCStats { // rtcfoostats.h: // class RTCFooStats : public RTCStats { // public: -// RTCFooStats(const std::string& id, double timestamp) -// : RTCStats(id, timestamp), +// RTCFooStats(const std::string& id, int64_t timestamp_us) +// : RTCStats(id, timestamp_us), // foo("foo"), // bar("bar") { // } diff --git a/webrtc/api/rtcstats_objects.h b/webrtc/api/rtcstats_objects.h index 8c03edfb14..4bc889d5d0 100644 --- a/webrtc/api/rtcstats_objects.h +++ b/webrtc/api/rtcstats_objects.h @@ -19,8 +19,8 @@ namespace webrtc { class RTCPeerConnectionStats : public RTCStats { public: - RTCPeerConnectionStats(const std::string& id, double timestamp); - RTCPeerConnectionStats(std::string&& id, double timestamp); + RTCPeerConnectionStats(const std::string& id, int64_t timestamp_us); + RTCPeerConnectionStats(std::string&& id, int64_t timestamp_us); WEBRTC_RTCSTATS_IMPL(RTCStats, RTCPeerConnectionStats, &data_channels_opened, diff --git a/webrtc/stats/rtcstats.cc b/webrtc/stats/rtcstats.cc index 1a35739c61..14ee7825d5 100644 --- a/webrtc/stats/rtcstats.cc +++ b/webrtc/stats/rtcstats.cc @@ -51,7 +51,7 @@ std::string VectorOfStringsToString(const std::vector& strings) { std::string RTCStats::ToString() const { std::ostringstream oss; oss << type() << " {\n id: \"" << id_ << "\"\n timestamp: " - << timestamp_ << '\n'; + << timestamp_us_ << '\n'; for (const RTCStatsMemberInterface* member : Members()) { oss << " " << member->name() << ": "; if (member->is_defined()) { diff --git a/webrtc/stats/rtcstats_objects.cc b/webrtc/stats/rtcstats_objects.cc index 1cfe85f036..46962f20b6 100644 --- a/webrtc/stats/rtcstats_objects.cc +++ b/webrtc/stats/rtcstats_objects.cc @@ -15,13 +15,13 @@ namespace webrtc { const char RTCPeerConnectionStats::kType[] = "peer-connection"; RTCPeerConnectionStats::RTCPeerConnectionStats( - const std::string& id, double timestamp) - : RTCPeerConnectionStats(std::string(id), timestamp) { + const std::string& id, int64_t timestamp_us) + : RTCPeerConnectionStats(std::string(id), timestamp_us) { } RTCPeerConnectionStats::RTCPeerConnectionStats( - std::string&& id, double timestamp) - : RTCStats(std::move(id), timestamp), + std::string&& id, int64_t timestamp_us) + : RTCStats(std::move(id), timestamp_us), data_channels_opened("dataChannelsOpened"), data_channels_closed("dataChannelsClosed") { } diff --git a/webrtc/stats/rtcstats_unittest.cc b/webrtc/stats/rtcstats_unittest.cc index 44e75c41be..1d4e016044 100644 --- a/webrtc/stats/rtcstats_unittest.cc +++ b/webrtc/stats/rtcstats_unittest.cc @@ -17,8 +17,8 @@ namespace webrtc { class RTCTestStats : public RTCStats { public: - RTCTestStats(const std::string& id, double timestamp) - : RTCStats(id, timestamp), + RTCTestStats(const std::string& id, int64_t timestamp_us) + : RTCStats(id, timestamp_us), m_int32("mInt32"), m_uint32("mUint32"), m_int64("mInt64"), @@ -72,8 +72,8 @@ const char RTCTestStats::kType[] = "test-stats"; class RTCChildStats : public RTCStats { public: - RTCChildStats(const std::string& id, double timestamp) - : RTCStats(id, timestamp), + RTCChildStats(const std::string& id, int64_t timestamp_us) + : RTCStats(id, timestamp_us), child_int("childInt") {} WEBRTC_RTCSTATS_IMPL(RTCStats, RTCChildStats, @@ -86,8 +86,8 @@ const char RTCChildStats::kType[] = "child-stats"; class RTCGrandChildStats : public RTCChildStats { public: - RTCGrandChildStats(const std::string& id, double timestamp) - : RTCChildStats(id, timestamp), + RTCGrandChildStats(const std::string& id, int64_t timestamp_us) + : RTCChildStats(id, timestamp_us), grandchild_int("grandchildInt") {} WEBRTC_RTCSTATS_IMPL(RTCChildStats, RTCGrandChildStats, @@ -99,9 +99,9 @@ class RTCGrandChildStats : public RTCChildStats { const char RTCGrandChildStats::kType[] = "grandchild-stats"; TEST(RTCStatsTest, RTCStatsAndMembers) { - RTCTestStats stats("testId", 42.0); + RTCTestStats stats("testId", 42); EXPECT_EQ(stats.id(), "testId"); - EXPECT_EQ(stats.timestamp(), 42.0); + EXPECT_EQ(stats.timestamp_us(), static_cast(42)); std::vector members = stats.Members(); EXPECT_EQ(members.size(), static_cast(14)); for (const RTCStatsMemberInterface* member : members) { diff --git a/webrtc/stats/rtcstatscollector.cc b/webrtc/stats/rtcstatscollector.cc index 6cb2a31b44..4bedad62ee 100644 --- a/webrtc/stats/rtcstatscollector.cc +++ b/webrtc/stats/rtcstatscollector.cc @@ -16,32 +16,38 @@ #include "webrtc/api/peerconnection.h" #include "webrtc/base/checks.h" +#include "webrtc/base/timing.h" namespace webrtc { RTCStatsCollector::RTCStatsCollector( PeerConnection* pc, - double cache_lifetime, - std::unique_ptr timing) + int64_t cache_lifetime_us) : pc_(pc), - timing_(std::move(timing)), - cache_timestamp_(0.0), - cache_lifetime_(cache_lifetime) { + cache_timestamp_us_(0), + cache_lifetime_us_(cache_lifetime_us) { RTC_DCHECK(pc_); - RTC_DCHECK(timing_); RTC_DCHECK(IsOnSignalingThread()); - RTC_DCHECK_GE(cache_lifetime_, 0.0); + RTC_DCHECK_GE(cache_lifetime_us_, 0); } rtc::scoped_refptr RTCStatsCollector::GetStatsReport() { RTC_DCHECK(IsOnSignalingThread()); - double now = timing_->TimerNow(); - if (cached_report_ && now - cache_timestamp_ <= cache_lifetime_) + // "Now" using a monotonically increasing timer. + int64_t cache_now_us = rtc::TimeMicros(); + if (cached_report_ && + cache_now_us - cache_timestamp_us_ <= cache_lifetime_us_) { return cached_report_; - cache_timestamp_ = now; + } + cache_timestamp_us_ = cache_now_us; + // "Now" using a system clock, relative to the UNIX epoch (Jan 1, 1970, UTC), + // in microseconds. The system clock could be modified and is not necessarily + // monotonically increasing. + int64_t timestamp_us = static_cast( + rtc::Timing::WallTimeNow() * rtc::kNumMicrosecsPerSec); rtc::scoped_refptr report = RTCStatsReport::Create(); - report->AddStats(ProducePeerConnectionStats()); + report->AddStats(ProducePeerConnectionStats(timestamp_us)); cached_report_ = report; return cached_report_; @@ -57,7 +63,7 @@ bool RTCStatsCollector::IsOnSignalingThread() const { } std::unique_ptr -RTCStatsCollector::ProducePeerConnectionStats() const { +RTCStatsCollector::ProducePeerConnectionStats(int64_t timestamp_us) const { // TODO(hbos): If data channels are removed from the peer connection this will // yield incorrect counts. Address before closing crbug.com/636818. See // https://w3c.github.io/webrtc-stats/webrtc-stats.html#pcstats-dict*. @@ -71,7 +77,7 @@ RTCStatsCollector::ProducePeerConnectionStats() const { // There is always just one |RTCPeerConnectionStats| so its |id| can be a // constant. std::unique_ptr stats( - new RTCPeerConnectionStats("RTCPeerConnection", cache_timestamp_)); + new RTCPeerConnectionStats("RTCPeerConnection", timestamp_us)); stats->data_channels_opened = data_channels_opened; stats->data_channels_closed = static_cast(data_channels.size()) - data_channels_opened; diff --git a/webrtc/stats/rtcstatscollector.h b/webrtc/stats/rtcstatscollector.h index 1e4d6c3c26..735421d541 100644 --- a/webrtc/stats/rtcstatscollector.h +++ b/webrtc/stats/rtcstatscollector.h @@ -16,7 +16,7 @@ #include "webrtc/api/rtcstats_objects.h" #include "webrtc/api/rtcstatsreport.h" #include "webrtc/base/scoped_ref_ptr.h" -#include "webrtc/base/timing.h" +#include "webrtc/base/timeutils.h" namespace webrtc { @@ -28,9 +28,7 @@ class RTCStatsCollector { public: explicit RTCStatsCollector( PeerConnection* pc, - double cache_lifetime = 0.05, - std::unique_ptr timing = std::unique_ptr( - new rtc::Timing())); + int64_t cache_lifetime_us = 50 * rtc::kNumMicrosecsPerMillisec); // Gets a recent stats report. If there is a report cached that is still fresh // it is returned, otherwise new stats are gathered and returned. A report is @@ -44,13 +42,16 @@ class RTCStatsCollector { private: bool IsOnSignalingThread() const; - std::unique_ptr ProducePeerConnectionStats() const; + std::unique_ptr ProducePeerConnectionStats( + int64_t timestamp_us) const; PeerConnection* const pc_; - mutable std::unique_ptr timing_; - // Time relative to the UNIX epoch (Jan 1, 1970, UTC), in seconds. - double cache_timestamp_; - double cache_lifetime_; // In seconds. + // A timestamp, in microseconds, that is based on a timer that is + // monotonically increasing. That is, even if the system clock is modified the + // difference between the timer and this timestamp is how fresh the cached + // report is. + int64_t cache_timestamp_us_; + int64_t cache_lifetime_us_; rtc::scoped_refptr cached_report_; }; diff --git a/webrtc/stats/rtcstatscollector_unittest.cc b/webrtc/stats/rtcstatscollector_unittest.cc index a3b0572c07..f917a75809 100644 --- a/webrtc/stats/rtcstatscollector_unittest.cc +++ b/webrtc/stats/rtcstatscollector_unittest.cc @@ -21,9 +21,12 @@ #include "webrtc/api/test/mock_peerconnection.h" #include "webrtc/api/test/mock_webrtcsession.h" #include "webrtc/base/checks.h" +#include "webrtc/base/fakeclock.h" #include "webrtc/base/gunit.h" #include "webrtc/base/logging.h" -#include "webrtc/base/test/faketiming.h" +#include "webrtc/base/timedelta.h" +#include "webrtc/base/timeutils.h" +#include "webrtc/base/timing.h" #include "webrtc/media/base/fakemediaengine.h" using testing::Return; @@ -78,17 +81,16 @@ class RTCStatsCollectorTest : public testing::Test { public: RTCStatsCollectorTest() : test_(new rtc::RefCountedObject()), - timing_(new rtc::FakeTiming()), - collector_(&test_->pc(), 0.05, std::unique_ptr(timing_)) { + collector_(&test_->pc(), 50 * rtc::kNumMicrosecsPerMillisec) { } protected: rtc::scoped_refptr test_; - rtc::FakeTiming* timing_; // Owned by |collector_|. RTCStatsCollector collector_; }; TEST_F(RTCStatsCollectorTest, CachedStatsReport) { + rtc::ScopedFakeClock fake_clock; // Caching should ensure |a| and |b| are the same report. rtc::scoped_refptr a = collector_.GetStatsReport(); rtc::scoped_refptr b = collector_.GetStatsReport(); @@ -100,19 +102,24 @@ TEST_F(RTCStatsCollectorTest, CachedStatsReport) { EXPECT_TRUE(c); EXPECT_NE(b.get(), c.get()); // Invalidate cache by advancing time. - timing_->AdvanceTimeMillisecs(51.0); + fake_clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(51)); rtc::scoped_refptr d = collector_.GetStatsReport(); EXPECT_TRUE(d); EXPECT_NE(c.get(), d.get()); } TEST_F(RTCStatsCollectorTest, CollectRTCPeerConnectionStats) { + int64_t before = static_cast( + rtc::Timing::WallTimeNow() * rtc::kNumMicrosecsPerSec); rtc::scoped_refptr report = collector_.GetStatsReport(); + int64_t after = static_cast( + rtc::Timing::WallTimeNow() * rtc::kNumMicrosecsPerSec); EXPECT_EQ(report->GetStatsOfType().size(), static_cast(1)) << "Expecting 1 RTCPeerConnectionStats."; const RTCStats* stats = report->Get("RTCPeerConnection"); EXPECT_TRUE(stats); - EXPECT_EQ(stats->timestamp(), timing_->TimerNow()); + EXPECT_LE(before, stats->timestamp_us()); + EXPECT_LE(stats->timestamp_us(), after); { // Expected stats with no data channels const RTCPeerConnectionStats& pcstats = diff --git a/webrtc/stats/rtcstatsreport_unittest.cc b/webrtc/stats/rtcstatsreport_unittest.cc index b4722ab156..2dcb58428a 100644 --- a/webrtc/stats/rtcstatsreport_unittest.cc +++ b/webrtc/stats/rtcstatsreport_unittest.cc @@ -18,8 +18,8 @@ namespace webrtc { class RTCTestStats1 : public RTCStats { public: - RTCTestStats1(const std::string& id, double timestamp) - : RTCStats(id, timestamp), + RTCTestStats1(const std::string& id, int64_t timestamp_us) + : RTCStats(id, timestamp_us), integer("integer") {} WEBRTC_RTCSTATS_IMPL(RTCStats, RTCTestStats1, @@ -32,8 +32,8 @@ const char RTCTestStats1::kType[] = "test-stats-1"; class RTCTestStats2 : public RTCStats { public: - RTCTestStats2(const std::string& id, double timestamp) - : RTCStats(id, timestamp), + RTCTestStats2(const std::string& id, int64_t timestamp_us) + : RTCStats(id, timestamp_us), number("number") {} WEBRTC_RTCSTATS_IMPL(RTCStats, RTCTestStats2, @@ -46,8 +46,8 @@ const char RTCTestStats2::kType[] = "test-stats-2"; class RTCTestStats3 : public RTCStats { public: - RTCTestStats3(const std::string& id, double timestamp) - : RTCStats(id, timestamp), + RTCTestStats3(const std::string& id, int64_t timestamp_us) + : RTCStats(id, timestamp_us), string("string") {} WEBRTC_RTCSTATS_IMPL(RTCStats, RTCTestStats3, @@ -61,12 +61,12 @@ const char RTCTestStats3::kType[] = "test-stats-3"; TEST(RTCStatsReport, AddAndGetStats) { rtc::scoped_refptr report = RTCStatsReport::Create(); EXPECT_EQ(report->size(), static_cast(0)); - report->AddStats(std::unique_ptr(new RTCTestStats1("a0", 1.0))); - report->AddStats(std::unique_ptr(new RTCTestStats1("a1", 2.0))); - report->AddStats(std::unique_ptr(new RTCTestStats2("b0", 4.0))); - report->AddStats(std::unique_ptr(new RTCTestStats2("b1", 8.0))); - report->AddStats(std::unique_ptr(new RTCTestStats1("a2", 16.0))); - report->AddStats(std::unique_ptr(new RTCTestStats2("b2", 32.0))); + report->AddStats(std::unique_ptr(new RTCTestStats1("a0", 1))); + report->AddStats(std::unique_ptr(new RTCTestStats1("a1", 2))); + report->AddStats(std::unique_ptr(new RTCTestStats2("b0", 4))); + report->AddStats(std::unique_ptr(new RTCTestStats2("b1", 8))); + report->AddStats(std::unique_ptr(new RTCTestStats1("a2", 16))); + report->AddStats(std::unique_ptr(new RTCTestStats2("b2", 32))); EXPECT_EQ(report->size(), static_cast(6)); EXPECT_EQ(report->Get("missing"), nullptr); @@ -75,17 +75,17 @@ TEST(RTCStatsReport, AddAndGetStats) { std::vector a = report->GetStatsOfType(); EXPECT_EQ(a.size(), static_cast(3)); - uint32_t mask = 0; + int64_t mask = 0; for (const RTCTestStats1* stats : a) - mask |= static_cast(stats->timestamp()); - EXPECT_EQ(mask, static_cast(1 | 2 | 16)); + mask |= stats->timestamp_us(); + EXPECT_EQ(mask, static_cast(1 | 2 | 16)); std::vector b = report->GetStatsOfType(); EXPECT_EQ(b.size(), static_cast(3)); mask = 0; for (const RTCTestStats2* stats : b) - mask |= static_cast(stats->timestamp()); - EXPECT_EQ(mask, static_cast(4 | 8 | 32)); + mask |= stats->timestamp_us(); + EXPECT_EQ(mask, static_cast(4 | 8 | 32)); EXPECT_EQ(report->GetStatsOfType().size(), static_cast(0)); @@ -93,39 +93,39 @@ TEST(RTCStatsReport, AddAndGetStats) { TEST(RTCStatsReport, StatsOrder) { rtc::scoped_refptr report = RTCStatsReport::Create(); - report->AddStats(std::unique_ptr(new RTCTestStats1("C", 2.0))); - report->AddStats(std::unique_ptr(new RTCTestStats1("D", 3.0))); - report->AddStats(std::unique_ptr(new RTCTestStats2("B", 1.0))); - report->AddStats(std::unique_ptr(new RTCTestStats2("A", 0.0))); - report->AddStats(std::unique_ptr(new RTCTestStats2("E", 4.0))); - report->AddStats(std::unique_ptr(new RTCTestStats2("F", 5.0))); - report->AddStats(std::unique_ptr(new RTCTestStats2("G", 6.0))); - size_t i = 0; + report->AddStats(std::unique_ptr(new RTCTestStats1("C", 2))); + report->AddStats(std::unique_ptr(new RTCTestStats1("D", 3))); + report->AddStats(std::unique_ptr(new RTCTestStats2("B", 1))); + report->AddStats(std::unique_ptr(new RTCTestStats2("A", 0))); + report->AddStats(std::unique_ptr(new RTCTestStats2("E", 4))); + report->AddStats(std::unique_ptr(new RTCTestStats2("F", 5))); + report->AddStats(std::unique_ptr(new RTCTestStats2("G", 6))); + int64_t i = 0; for (const RTCStats& stats : *report) { - EXPECT_EQ(static_cast(stats.timestamp()), i); + EXPECT_EQ(stats.timestamp_us(), i); ++i; } - EXPECT_EQ(i, static_cast(7)); + EXPECT_EQ(i, static_cast(7)); } TEST(RTCStatsReport, TakeMembersFrom) { rtc::scoped_refptr a = RTCStatsReport::Create(); - a->AddStats(std::unique_ptr(new RTCTestStats1("B", 1.0))); - a->AddStats(std::unique_ptr(new RTCTestStats1("C", 2.0))); - a->AddStats(std::unique_ptr(new RTCTestStats1("E", 4.0))); + a->AddStats(std::unique_ptr(new RTCTestStats1("B", 1))); + a->AddStats(std::unique_ptr(new RTCTestStats1("C", 2))); + a->AddStats(std::unique_ptr(new RTCTestStats1("E", 4))); rtc::scoped_refptr b = RTCStatsReport::Create(); - b->AddStats(std::unique_ptr(new RTCTestStats1("A", 0.0))); - b->AddStats(std::unique_ptr(new RTCTestStats1("D", 3.0))); - b->AddStats(std::unique_ptr(new RTCTestStats1("F", 5.0))); + b->AddStats(std::unique_ptr(new RTCTestStats1("A", 0))); + b->AddStats(std::unique_ptr(new RTCTestStats1("D", 3))); + b->AddStats(std::unique_ptr(new RTCTestStats1("F", 5))); a->TakeMembersFrom(b); EXPECT_EQ(b->size(), static_cast(0)); - size_t i = 0; + int64_t i = 0; for (const RTCStats& stats : *a) { - EXPECT_EQ(static_cast(stats.timestamp()), i); + EXPECT_EQ(stats.timestamp_us(), i); ++i; } - EXPECT_EQ(i, static_cast(6)); + EXPECT_EQ(i, static_cast(6)); } } // namespace webrtc