Skip to content

Commit

Permalink
[cc/metrics] Some more debug checks.
Browse files Browse the repository at this point in the history
Add DCHECK() to verify that every impl-frame started is also terminated
as expected: either a frame is submitted, or no-damage is explicitly
reported.

Also, add 'R' to the debug-log for a reset, and always report 'e' for
end, even for an invalid sequence-number.

BUG=1035382

Change-Id: I1aeab1453fa0c1b80a02b2f1795ab898be71a060
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1976912
Reviewed-by: Xida Chen <xidachen@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Auto-Submit: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726688}
  • Loading branch information
sadrulhc authored and Commit Bot committed Dec 20, 2019
1 parent 2924a01 commit 1c1af9f
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 3 deletions.
14 changes: 11 additions & 3 deletions cc/metrics/frame_sequence_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ void FrameSequenceTracker::ReportBeginImplFrame(
#if DCHECK_IS_ON()
DCHECK(!is_inside_frame_) << TRACKER_DCHECK_MSG;
is_inside_frame_ = true;
last_started_impl_sequence_ = args.frame_id.sequence_number;

if (args.type == viz::BeginFrameArgs::NORMAL)
impl_frames_.insert(args.frame_id);
Expand Down Expand Up @@ -489,6 +490,7 @@ void FrameSequenceTracker::ReportSubmitFrame(

#if DCHECK_IS_ON()
DCHECK(is_inside_frame_) << TRACKER_DCHECK_MSG;
last_processed_impl_sequence_ = ack.frame_id.sequence_number;
#endif
if (first_submitted_frame_ == 0)
first_submitted_frame_ = frame_token;
Expand All @@ -508,7 +510,7 @@ void FrameSequenceTracker::ReportSubmitFrame(
if (!ShouldIgnoreBeginFrameSource(origin_args.frame_id.source_id) &&
main_changes_after_sequence_started && main_changes_include_new_changes &&
!main_change_had_no_damage) {
TRACKER_TRACE_STREAM << 'S';
TRACKER_TRACE_STREAM << "S(" << origin_args.frame_id.sequence_number << ")";

last_submitted_main_sequence_ = origin_args.frame_id.sequence_number;
main_frames_.push_back(frame_token);
Expand All @@ -529,13 +531,15 @@ void FrameSequenceTracker::ReportFrameEnd(const viz::BeginFrameArgs& args) {
if (ShouldIgnoreBeginFrameSource(args.frame_id.source_id))
return;

TRACKER_TRACE_STREAM << "e(" << args.frame_id.sequence_number << ")";
if (ShouldIgnoreSequence(args.frame_id.sequence_number)) {
is_inside_frame_ = false;
return;
}

TRACKER_TRACE_STREAM << "e(" << args.frame_id.sequence_number << ")";
DCHECK(is_inside_frame_) << TRACKER_DCHECK_MSG;
DCHECK_EQ(last_started_impl_sequence_, last_processed_impl_sequence_)
<< TRACKER_DCHECK_MSG;
is_inside_frame_ = false;
#endif
}
Expand Down Expand Up @@ -639,6 +643,9 @@ void FrameSequenceTracker::ReportImplFrameCausedNoDamage(
// dispatched for this frame-sequence. In such cases, ignore this call.
if (ShouldIgnoreSequence(ack.frame_id.sequence_number))
return;
#if DCHECK_IS_ON()
last_processed_impl_sequence_ = ack.frame_id.sequence_number;
#endif

TRACKER_TRACE_STREAM << 'n';
DCHECK_GT(impl_throughput().frames_expected, 0u) << TRACKER_DCHECK_MSG;
Expand Down Expand Up @@ -683,6 +690,7 @@ void FrameSequenceTracker::PauseFrameProduction() {
// received begin-frame.
begin_impl_frame_data_ = {0, 0, 0};
begin_main_frame_data_ = {0, 0, 0};
TRACKER_TRACE_STREAM << 'R';
}

void FrameSequenceTracker::UpdateTrackedFrameData(TrackedFrameData* frame_data,
Expand All @@ -703,7 +711,7 @@ void FrameSequenceTracker::UpdateTrackedFrameData(TrackedFrameData* frame_data,
bool FrameSequenceTracker::ShouldIgnoreBeginFrameSource(
uint64_t source_id) const {
if (begin_impl_frame_data_.previous_source == 0)
return false;
return source_id == viz::BeginFrameArgs::kManualSourceId;
return source_id != begin_impl_frame_data_.previous_source;
}

Expand Down
3 changes: 3 additions & 0 deletions cc/metrics/frame_sequence_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,9 @@ class CC_EXPORT FrameSequenceTracker {
// when DCHECK is on.
std::stringstream frame_sequence_trace_;

uint64_t last_started_impl_sequence_ = 0;
uint64_t last_processed_impl_sequence_ = 0;

// If ReportBeginImplFrame is never called on a arg, then ReportBeginMainFrame
// should ignore that arg.
base::flat_set<viz::BeginFrameId> impl_frames_;
Expand Down

0 comments on commit 1c1af9f

Please sign in to comment.