Commit 0bd76148 authored by Xida Chen's avatar Xida Chen Committed by Commit Bot

Revert "Change to report the throughput metrics every 1 seconds"

This reverts commit d6905eb4.

Reason for revert:
Per offline discussion with sadrul@ and flackr@, will limit this change to universal tracker.

Original change's description:
> Change to report the throughput metrics every 1 seconds
> 
> After compared with interval of reporting the throughput metrics every
> 1 second and 5 seconds, we found out that 1 second is closer to the real
> data, so we decide to change to the interval to 1 second.
> 
> Bug: 1040634
> Change-Id: I94527a45ffe390fc6d44a2430f52dcb2346d6cbe
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2332978
> Reviewed-by: Xida Chen <xidachen@chromium.org>
> Reviewed-by: Robert Flack <flackr@chromium.org>
> Commit-Queue: Lan Wei <lanwei@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#795132}

TBR=flackr@chromium.org,lanwei@chromium.org,xidachen@chromium.org

Change-Id: Ic6ed30890891a59b02fff03cf9d80189a8b14a59
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1040634
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2340983Reviewed-by: default avatarXida Chen <xidachen@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795411}
parent d5278fe6
......@@ -22,7 +22,7 @@ namespace {
// Avoid reporting any throughput metric for sequences that do not have a
// sufficient number of frames.
constexpr int kMinFramesForThroughputMetric = 20;
constexpr int kMinFramesForThroughputMetric = 100;
constexpr int kBuiltinSequenceNum =
static_cast<int>(FrameSequenceTrackerType::kMaxType) + 1;
......
......@@ -53,15 +53,15 @@ TEST(FrameSequenceMetricsTest, MergeMetrics) {
// Create a metric with only a small number of frames. It shouldn't report any
// metrics.
FrameSequenceMetrics first(FrameSequenceTrackerType::kTouchScroll, nullptr);
first.impl_throughput().frames_expected = 15;
first.impl_throughput().frames_produced = 5;
first.impl_throughput().frames_expected = 20;
first.impl_throughput().frames_produced = 10;
EXPECT_FALSE(first.HasEnoughDataForReporting());
// Create a second metric with too few frames to report any metrics.
auto second = std::make_unique<FrameSequenceMetrics>(
FrameSequenceTrackerType::kTouchScroll, nullptr);
second->impl_throughput().frames_expected = 10;
second->impl_throughput().frames_produced = 5;
second->impl_throughput().frames_expected = 90;
second->impl_throughput().frames_produced = 60;
EXPECT_FALSE(second->HasEnoughDataForReporting());
// Merge the two metrics. The result should have enough frames to report
......@@ -74,13 +74,13 @@ TEST(FrameSequenceMetricsTest, MergeMetrics) {
TEST(FrameSequenceMetricsTest, ScrollingThreadMergeMetrics) {
FrameSequenceMetrics first(FrameSequenceTrackerType::kTouchScroll, nullptr);
first.SetScrollingThread(FrameSequenceMetrics::ThreadType::kCompositor);
first.impl_throughput().frames_expected = 15;
first.impl_throughput().frames_produced = 5;
first.impl_throughput().frames_expected = 20;
first.impl_throughput().frames_produced = 10;
auto second = std::make_unique<FrameSequenceMetrics>(
FrameSequenceTrackerType::kTouchScroll, nullptr);
second->SetScrollingThread(FrameSequenceMetrics::ThreadType::kMain);
second->main_throughput().frames_expected = 15;
second->main_throughput().frames_expected = 50;
second->main_throughput().frames_produced = 10;
ASSERT_DEATH(first.Merge(std::move(second)), "");
......@@ -93,10 +93,10 @@ TEST(FrameSequenceMetricsTest, AllMetricsReported) {
// Create a metric with enough frames on impl to be reported, but not enough
// on main.
FrameSequenceMetrics first(FrameSequenceTrackerType::kTouchScroll, nullptr);
first.impl_throughput().frames_expected = 30;
first.impl_throughput().frames_produced = 10;
first.main_throughput().frames_expected = 10;
first.main_throughput().frames_produced = 5;
first.impl_throughput().frames_expected = 120;
first.impl_throughput().frames_produced = 80;
first.main_throughput().frames_expected = 20;
first.main_throughput().frames_produced = 10;
EXPECT_TRUE(first.HasEnoughDataForReporting());
first.ReportMetrics();
......@@ -113,9 +113,9 @@ TEST(FrameSequenceMetricsTest, AllMetricsReported) {
auto second = std::make_unique<FrameSequenceMetrics>(
FrameSequenceTrackerType::kTouchScroll, nullptr);
second->impl_throughput().frames_expected = 30;
second->impl_throughput().frames_produced = 20;
second->main_throughput().frames_expected = 10;
second->impl_throughput().frames_expected = 110;
second->impl_throughput().frames_produced = 100;
second->main_throughput().frames_expected = 90;
first.Merge(std::move(second));
EXPECT_TRUE(first.HasEnoughDataForReporting());
first.ReportMetrics();
......@@ -128,10 +128,10 @@ TEST(FrameSequenceMetricsTest, AllMetricsReported) {
EXPECT_FALSE(first.HasDataLeftForReporting());
FrameSequenceMetrics third(FrameSequenceTrackerType::kUniversal, nullptr);
third.impl_throughput().frames_expected = 30;
third.impl_throughput().frames_produced = 10;
third.main_throughput().frames_expected = 30;
third.main_throughput().frames_produced = 10;
third.impl_throughput().frames_expected = 120;
third.impl_throughput().frames_produced = 80;
third.main_throughput().frames_expected = 120;
third.main_throughput().frames_produced = 80;
EXPECT_TRUE(third.HasEnoughDataForReporting());
third.ReportMetrics();
......
......@@ -230,8 +230,8 @@ class CC_EXPORT FrameSequenceTracker {
// present a frame even if it is ignored by ReportSubmitFrame.
base::flat_set<uint32_t> ignored_frame_tokens_;
// Report the throughput metrics every 1 seconds.
const base::TimeDelta time_delta_to_report_ = base::TimeDelta::FromSeconds(1);
// Report the throughput metrics every 5 seconds.
const base::TimeDelta time_delta_to_report_ = base::TimeDelta::FromSeconds(5);
uint64_t last_started_impl_sequence_ = 0;
uint64_t last_processed_impl_sequence_ = 0;
......
......@@ -2044,12 +2044,12 @@ TEST_F(FrameSequenceTrackerTest, MergeTrackersScrollOnSameThread) {
// Do a short scroll on the compositor thread, then do another short scroll on
// the compositor thread. Make sure these are merged.
base::HistogramTester histogram_tester;
const char first_sequence[] = "b(1)s(1)e(1,0)P(1)b(10)s(2)e(10,0)P(2)";
const char first_sequence[] = "b(1)s(1)e(1,0)P(1)b(80)s(2)e(80,0)P(2)";
GenerateSequence(first_sequence);
collection_.StopSequence(FrameSequenceTrackerType::kTouchScroll);
CreateNewTracker(FrameSequenceMetrics::ThreadType::kCompositor);
const char second_sequence[] = "b(11)s(3)e(11,0)P(3)b(21)s(4)e(21,0)P(4)";
const char second_sequence[] = "b(81)s(3)e(81,0)P(3)b(101)s(4)e(101,0)P(4)";
GenerateSequence(second_sequence);
collection_.StopSequence(FrameSequenceTrackerType::kTouchScroll);
......@@ -2065,12 +2065,13 @@ TEST_F(FrameSequenceTrackerTest, MergeTrackersScrollOnDifferentThreads) {
// Do a short scroll on the compositor thread, then do another short scroll on
// the main-thread. Make sure these are not merged.
base::HistogramTester histogram_tester;
const char compscroll_sequence[] = "b(1)s(1)e(1,0)P(1)b(10)s(2)e(10,0)P(2)";
const char compscroll_sequence[] = "b(1)s(1)e(1,0)P(1)b(80)s(2)e(80,0)P(2)";
GenerateSequence(compscroll_sequence);
collection_.StopSequence(FrameSequenceTrackerType::kTouchScroll);
CreateNewTracker(FrameSequenceMetrics::ThreadType::kMain);
const char mainscroll_sequence[] = "b(11)s(3)e(11,0)P(3)b(21)s(4)e(21,0)P(4)";
const char mainscroll_sequence[] =
"b(81)s(3)e(81,0)P(3)b(101)s(4)e(101,0)P(4)";
GenerateSequence(mainscroll_sequence);
collection_.StopSequence(FrameSequenceTrackerType::kTouchScroll);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment