Skip to content

Commit

Permalink
Revert "Reland "Add prediction and attribution finding logic for viz …
Browse files Browse the repository at this point in the history 
…and blink""

This reverts commit 65599fb.

Reason for revert: Still DCHECKing. See query posted in bug.

Bug: 1351361

Original change's description:
> Reland "Add prediction and attribution finding logic for viz and blink"
>
> This is a reland of commit 31a41e2
>
> Original change's description:
> > Add prediction and attribution finding logic for viz and blink
> >
> > Implement logics to calculate the prediction and find the high latency
> > attribution for breakdown stages of kSendBeginMainFrameToCommit and
> > kSubmitCompositorFrameToPresentationCompositorFrame, which are the
> > viz and blink breakdown stages.
> >
> > Bug: 1334823
> > Change-Id: Iba4a399c030c81dba618dd956dbb171d46f3a7be
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3788225
> > Reviewed-by: Mingjing Zhang <mjzhang@chromium.org>
> > Commit-Queue: April Li <apli@google.com>
> > Reviewed-by: Jonathan Ross <jonross@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1032778}
>
> Bug: 1334823
> Change-Id: If38526ae284262301c50a9aa518649fb22d99598
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3824477
> Reviewed-by: Jonathan Ross <jonross@chromium.org>
> Reviewed-by: Mingjing Zhang <mjzhang@chromium.org>
> Commit-Queue: April Li <apli@google.com>
> Cr-Commit-Position: refs/heads/main@{#1033694}

Bug: 1334823
Change-Id: I822a9cc5ba5145a3ead3ca25d89ac8c1714924a9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3831625
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Auto-Submit: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1035108}
  • Loading branch information
@pbos
pbos authored and Chromium LUCI CQ committed Aug 15, 2022
1 parent 4aaca2a commit dac853f
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 497 deletions.
196 changes: 24 additions & 172 deletions cc/metrics/compositor_frame_reporter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,8 @@ constexpr int kEventLatencyHistogramBucketCount = 100;
constexpr base::TimeDelta kHighLatencyMin = base::Milliseconds(75);

// Number of breakdown stages of the current PipelineReporter
constexpr int kNumOfCompositorStages =
static_cast<int>(StageType::kStageTypeCount) - 1;
// Number of breakdown stages of the blink
constexpr int kNumOfBlinkStages =
static_cast<int>(BlinkBreakdown::kBreakdownCount);
// Number of breakdown stages of the viz
constexpr int kNumOfVizStages = static_cast<int>(VizBreakdown::kBreakdownCount);
constexpr int kNumOfStages = static_cast<int>(StageType::kStageTypeCount) - 1;

// Number of dispatch stages of the current EventLatency
constexpr int kNumDispatchStages =
static_cast<int>(EventMetrics::DispatchStage::kMaxValue);
Expand Down Expand Up @@ -377,12 +372,7 @@ CompositorFrameReporter::CompositorLatencyInfo::CompositorLatencyInfo() =
default;
CompositorFrameReporter::CompositorLatencyInfo::CompositorLatencyInfo(
base::TimeDelta init_value)
: top_level_stages(kNumOfCompositorStages, init_value),
blink_breakdown_stages(kNumOfBlinkStages, init_value),
viz_breakdown_stages(kNumOfVizStages, init_value),
total_latency(init_value),
total_blink_latency(init_value),
total_viz_latency(init_value) {}
: top_level_stages(kNumOfStages, init_value), total_latency(init_value) {}
CompositorFrameReporter::CompositorLatencyInfo::~CompositorLatencyInfo() =
default;

Expand Down Expand Up @@ -720,12 +710,10 @@ void CompositorFrameReporter::TerminateReporter() {
if (frame_termination_status_ == FrameTerminationStatus::kUnknown)
TerminateFrame(FrameTerminationStatus::kUnknown, Now());

if (!processed_blink_breakdown_)
processed_blink_breakdown_ = std::make_unique<ProcessedBlinkBreakdown>(
blink_start_time_, begin_main_frame_start_, blink_breakdown_);
if (!processed_viz_breakdown_)
processed_viz_breakdown_ = std::make_unique<ProcessedVizBreakdown>(
viz_start_time_, viz_breakdown_);
processed_blink_breakdown_ = std::make_unique<ProcessedBlinkBreakdown>(
blink_start_time_, begin_main_frame_start_, blink_breakdown_);
processed_viz_breakdown_ =
std::make_unique<ProcessedVizBreakdown>(viz_start_time_, viz_breakdown_);

DCHECK_EQ(current_stage_.start_time, base::TimeTicks());
const FrameInfo frame_info = GenerateFrameInfo();
Expand Down Expand Up @@ -1310,7 +1298,7 @@ void CompositorFrameReporter::CalculateCompositorLatencyPrediction(

// If the bad case of having `total_latency` of `previous_predictions` happens
// then it would mess up the prediction calculation, therefore, we want to
// reset the prediction by setting everything back to -1.
// reset the prediction by setting everything back to -1
if (previous_predictions.total_latency.is_zero())
previous_predictions = CompositorLatencyInfo(base::Microseconds(-1));

Expand All @@ -1323,11 +1311,6 @@ void CompositorFrameReporter::CalculateCompositorLatencyPrediction(
if (total_pipeline_latency.is_zero())
return;

processed_blink_breakdown_ = std::make_unique<ProcessedBlinkBreakdown>(
blink_start_time_, begin_main_frame_start_, blink_breakdown_);
processed_viz_breakdown_ =
std::make_unique<ProcessedVizBreakdown>(viz_start_time_, viz_breakdown_);

// Note that `current_stage_durations` would always have the same length as
// `previous_predictions`, since each index represent the breakdown stages of
// the PipelineReporter listed at enum class, StageType.
Expand All @@ -1342,22 +1325,6 @@ void CompositorFrameReporter::CalculateCompositorLatencyPrediction(
.top_level_stages[static_cast<int>(stage.stage_type)] = substageLatency;
}

for (auto it = processed_blink_breakdown_->CreateIterator(); it.IsValid();
it.Advance()) {
current_stage_durations
.blink_breakdown_stages[static_cast<int>(it.GetBreakdown())] =
it.GetLatency();
current_stage_durations.total_blink_latency += it.GetLatency();
}

for (auto it = processed_viz_breakdown_->CreateIterator(true); it.IsValid();
it.Advance()) {
current_stage_durations
.viz_breakdown_stages[static_cast<int>(it.GetBreakdown())] =
it.GetDuration();
current_stage_durations.total_viz_latency += it.GetDuration();
}

// Do not record current pipeline details or update predictions if no frame
// is submitted.
if (current_stage_durations
Expand All @@ -1375,52 +1342,14 @@ void CompositorFrameReporter::CalculateCompositorLatencyPrediction(
previous_predictions.total_latency) >= prediction_deviation_threshold)
FindHighLatencyAttribution(previous_predictions, current_stage_durations);

for (int i = 0; i < kNumOfCompositorStages; i++) {
for (int i = 0; i < kNumOfStages; i++) {
previous_predictions.top_level_stages[i] =
PredictLatency(previous_predictions.top_level_stages[i],
current_stage_durations.top_level_stages[i]);
}
previous_predictions.total_latency =
PredictLatency(previous_predictions.total_latency,
current_stage_durations.total_latency);

if (!current_stage_durations.total_blink_latency.is_zero()) {
for (int i = 0; i < kNumOfBlinkStages; i++) {
previous_predictions.blink_breakdown_stages[i] =
previous_predictions.total_blink_latency.is_zero()
? current_stage_durations.blink_breakdown_stages[i]
: PredictLatency(
previous_predictions.blink_breakdown_stages[i],
current_stage_durations.blink_breakdown_stages[i]);
}
previous_predictions.total_blink_latency =
previous_predictions.total_blink_latency.is_zero()
? current_stage_durations.total_blink_latency
: PredictLatency(previous_predictions.total_blink_latency,
current_stage_durations.total_blink_latency);
}

// TODO(crbug.com/1349930): implement check that ensure the prediction is
// correct by checking if platform supports breakdown of the stage
// SubmitCompositorFrameToPresentationCompositorFrame.SwapStartToSwapEnd,
// then SwapStartToSwapEnd should always be 0s and data for breakdown of it
// should always be available. (See enum class `VizBreakdown` for stage
// details.)
if (!current_stage_durations.total_viz_latency.is_zero()) {
for (int i = 0; i < kNumOfVizStages; i++) {
previous_predictions.viz_breakdown_stages[i] =
previous_predictions.total_viz_latency.is_zero()
? current_stage_durations.viz_breakdown_stages[i]
: PredictLatency(
previous_predictions.viz_breakdown_stages[i],
current_stage_durations.viz_breakdown_stages[i]);
}
previous_predictions.total_viz_latency =
previous_predictions.total_viz_latency.is_zero()
? current_stage_durations.total_viz_latency
: PredictLatency(previous_predictions.total_viz_latency,
current_stage_durations.total_viz_latency);
}
}
}

Expand Down Expand Up @@ -1470,7 +1399,7 @@ void CompositorFrameReporter::CalculateEventLatencyPrediction(
return;

CompositorFrameReporter::EventLatencyInfo actual_event_latency(
kNumDispatchStages, kNumOfCompositorStages);
kNumDispatchStages, kNumOfStages);
actual_event_latency.total_duration = base::Microseconds(0);

// Determine dispatch stage durations.
Expand Down Expand Up @@ -1557,7 +1486,7 @@ void CompositorFrameReporter::CalculateEventLatencyPrediction(
// Calculate new compositor stage predictions.
// TODO(crbug.com/1334827): Explore using existing PipelineReporter
// predictions for the compositor stage.
for (int i = 0; i < kNumOfCompositorStages; i++) {
for (int i = 0; i < kNumOfStages; i++) {
if (actual_event_latency.compositor_durations[i].is_positive()) {
predicted_event_latency.compositor_durations[i] =
CalculateWeightedAverage(
Expand Down Expand Up @@ -1719,86 +1648,19 @@ void CompositorFrameReporter::FindHighLatencyAttribution(
double contribution_change = -1;
double highest_contribution_change = -1;
std::vector<int> highest_contribution_change_index;
std::vector<int> highest_blink_contribution_change_index;
std::vector<int> highest_viz_contribution_change_index;

for (int i = 0; i < kNumOfCompositorStages; i++) {
switch (i) {
case static_cast<int>(StageType::kSendBeginMainFrameToCommit):
if (current_stage_durations.top_level_stages[i].is_zero() ||
previous_predictions.total_blink_latency.is_zero())
continue;

DCHECK(!current_stage_durations.total_blink_latency.is_zero())
<< "There should never be the case where in `stage_history`, data "
"of `kSendBeginMainFrameToCommit` exists, however, the breakdown"
" of blink does not exist";

for (int j = 0; j < kNumOfBlinkStages; j++) {
contribution_change =
(current_stage_durations.blink_breakdown_stages[j] /
current_stage_durations.total_latency) -
(previous_predictions.blink_breakdown_stages[j] /
previous_predictions.total_latency);

if (contribution_change > highest_contribution_change) {
highest_contribution_change = contribution_change;
highest_contribution_change_index.clear();
highest_viz_contribution_change_index.clear();
highest_blink_contribution_change_index = {j};
} else if (std::abs(contribution_change -
highest_contribution_change) < kEpsilon) {
highest_blink_contribution_change_index.push_back(j);
}
}
break;

case static_cast<int>(
StageType::kSubmitCompositorFrameToPresentationCompositorFrame):
if (current_stage_durations.top_level_stages[i].is_zero() ||
previous_predictions.total_viz_latency.is_zero())
continue;

DCHECK(!current_stage_durations.total_viz_latency.is_zero())
<< "There should never be the case where in `stage_history`, data "
"of `kSubmitCompositorFrameToPresentationCompositorFrame` "
"exists, however, the breakdown of viz does not exist";

for (int j = 0; j < kNumOfVizStages; j++) {
contribution_change =
(current_stage_durations.viz_breakdown_stages[j] /
current_stage_durations.total_latency) -
(previous_predictions.viz_breakdown_stages[j] /
previous_predictions.total_latency);

if (contribution_change > highest_contribution_change) {
highest_contribution_change = contribution_change;
highest_contribution_change_index.clear();
highest_blink_contribution_change_index.clear();
highest_viz_contribution_change_index = {j};
} else if (std::abs(contribution_change -
highest_contribution_change) < kEpsilon) {
highest_viz_contribution_change_index.push_back(j);
}
}
break;

default:
contribution_change = (current_stage_durations.top_level_stages[i] /
current_stage_durations.total_latency) -
(previous_predictions.top_level_stages[i] /
previous_predictions.total_latency);

if (contribution_change > highest_contribution_change) {
highest_contribution_change = contribution_change;
highest_blink_contribution_change_index.clear();
highest_viz_contribution_change_index.clear();
highest_contribution_change_index = {i};
} else if (std::abs(contribution_change - highest_contribution_change) <
kEpsilon) {
highest_contribution_change_index.push_back(i);
}
break;
for (int i = 0; i < kNumOfStages; i++) {
contribution_change = (current_stage_durations.top_level_stages[i] /
current_stage_durations.total_latency) -
(previous_predictions.top_level_stages[i] /
previous_predictions.total_latency);

if (contribution_change > highest_contribution_change) {
highest_contribution_change = contribution_change;
highest_contribution_change_index = {i};
} else if (std::abs(contribution_change - highest_contribution_change) <
kEpsilon) {
highest_contribution_change_index.push_back(i);
}
}

Expand All @@ -1809,16 +1671,6 @@ void CompositorFrameReporter::FindHighLatencyAttribution(
high_latency_substages_.push_back(
GetStageName(static_cast<StageType>(index)));
}
for (auto index : highest_blink_contribution_change_index) {
high_latency_substages_.push_back(
GetStageName(StageType::kSendBeginMainFrameToCommit, absl::nullopt,
static_cast<BlinkBreakdown>(index)));
}
for (auto index : highest_viz_contribution_change_index) {
high_latency_substages_.push_back(GetStageName(
StageType::kSubmitCompositorFrameToPresentationCompositorFrame,
static_cast<VizBreakdown>(index)));
}
}

void CompositorFrameReporter::FindEventLatencyAttribution(
Expand Down Expand Up @@ -1880,7 +1732,7 @@ void CompositorFrameReporter::FindEventLatencyAttribution(
actual_event_latency.transition_name, high_latency_stages);

// Check compositor stage change
for (int i = 0; i < kNumOfCompositorStages; i++) {
for (int i = 0; i < kNumOfStages; i++) {
contribution_change = (actual_event_latency.compositor_durations[i] /
actual_event_latency.total_duration) -
(predicted_event_latency.compositor_durations[i] /
Expand Down
6 changes: 1 addition & 5 deletions cc/metrics/compositor_frame_reporter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,12 +251,8 @@ class CC_EXPORT CompositorFrameReporter {
~CompositorLatencyInfo();

std::vector<base::TimeDelta> top_level_stages;
std::vector<base::TimeDelta> blink_breakdown_stages;
std::vector<base::TimeDelta> viz_breakdown_stages;

// TODO(crbug.com/1334823): add viz and blink breakdown
base::TimeDelta total_latency;
base::TimeDelta total_blink_latency;
base::TimeDelta total_viz_latency;
};

CompositorFrameReporter(const ActiveTrackers& active_trackers,
Expand Down

0 comments on commit dac853f

Please sign in to comment.