Skip to content

Commit

Permalink
[cc/metrics] Some more debug checks.
Browse files Browse the repository at this point in the history
. Validate that main frame starts for the same compositor/impl frame.
. A 'no damage' response from the main-thread can only happen from the
  most recently started main-frame.
. Move the stream-logging before some early exits.

BUG=none

Change-Id: I4037070d9b87b3aa424ad14ad5ceb57969113d1a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1980311
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Auto-Submit: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#727168}
  • Loading branch information
sadrulhc authored and Commit Bot committed Dec 23, 2019
1 parent 2388bf4 commit 2f848d2
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 5 deletions.
25 changes: 20 additions & 5 deletions cc/metrics/frame_sequence_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -429,9 +429,14 @@ void FrameSequenceTracker::ReportBeginImplFrame(
if (ShouldIgnoreBeginFrameSource(args.frame_id.source_id))
return;

TRACKER_TRACE_STREAM << "b(" << args.frame_id.sequence_number << ")";

#if DCHECK_IS_ON()
DCHECK(!is_inside_frame_) << TRACKER_DCHECK_MSG;
is_inside_frame_ = true;

DCHECK_EQ(last_started_impl_sequence_, 0u) << TRACKER_DCHECK_MSG;
DCHECK_EQ(last_processed_impl_sequence_, 0u) << TRACKER_DCHECK_MSG;
last_started_impl_sequence_ = args.frame_id.sequence_number;

if (args.type == viz::BeginFrameArgs::NORMAL)
Expand All @@ -444,7 +449,6 @@ void FrameSequenceTracker::ReportBeginImplFrame(
reset_all_state_ = false;
}

TRACKER_TRACE_STREAM << "b(" << args.frame_id.sequence_number << ")";
DCHECK(!frame_had_no_compositor_damage_) << TRACKER_DCHECK_MSG;
DCHECK(!compositor_frame_submitted_) << TRACKER_DCHECK_MSG;

Expand Down Expand Up @@ -475,6 +479,9 @@ void FrameSequenceTracker::ReportBeginMainFrame(
if (args.type == viz::BeginFrameArgs::NORMAL) {
DCHECK(impl_frames_.contains(args.frame_id));
}
last_started_main_sequence_ = args.frame_id.sequence_number;
DCHECK_EQ(last_started_impl_sequence_, last_started_main_sequence_)
<< TRACKER_DCHECK_MSG;
#endif

UpdateTrackedFrameData(&begin_main_frame_data_, args.frame_id.source_id,
Expand Down Expand Up @@ -580,6 +587,7 @@ void FrameSequenceTracker::ReportFrameEnd(const viz::BeginFrameArgs& args) {
DCHECK_EQ(last_started_impl_sequence_, last_processed_impl_sequence_)
<< TRACKER_DCHECK_MSG;
is_inside_frame_ = false;
last_started_impl_sequence_ = last_processed_impl_sequence_ = 0;
#endif
}

Expand All @@ -605,11 +613,11 @@ void FrameSequenceTracker::ReportFramePresented(
return;
}

TRACKER_TRACE_STREAM << "P(" << frame_token << ")";

if (ignored_frame_tokens_.contains(frame_token))
return;

TRACKER_TRACE_STREAM << "P(" << frame_token << ")";

TRACE_EVENT_ASYNC_STEP_INTO_WITH_TIMESTAMP0(
"cc,benchmark", "FrameSequenceTracker", metrics_.get(), "FramePresented",
feedback.timestamp);
Expand Down Expand Up @@ -706,11 +714,12 @@ void FrameSequenceTracker::ReportMainFrameCausedNoDamage(
if (ShouldIgnoreBeginFrameSource(args.frame_id.source_id))
return;

TRACKER_TRACE_STREAM << "N(" << begin_main_frame_data_.previous_sequence
<< "," << args.frame_id.sequence_number << ")";

if (ShouldIgnoreSequence(args.frame_id.sequence_number))
return;

TRACKER_TRACE_STREAM << "N(" << begin_main_frame_data_.previous_sequence
<< "," << args.frame_id.sequence_number << ")";
if (last_no_main_damage_sequence_ == args.frame_id.sequence_number)
return;

Expand All @@ -725,6 +734,12 @@ void FrameSequenceTracker::ReportMainFrameCausedNoDamage(

if (begin_main_frame_data_.previous_sequence == args.frame_id.sequence_number)
begin_main_frame_data_.previous_sequence = 0;

#if DCHECK_IS_ON()
DCHECK_EQ(last_started_main_sequence_, args.frame_id.sequence_number)
<< TRACKER_DCHECK_MSG;
last_started_main_sequence_ = 0;
#endif
}

void FrameSequenceTracker::PauseFrameProduction() {
Expand Down
2 changes: 2 additions & 0 deletions cc/metrics/frame_sequence_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,8 @@ class CC_EXPORT FrameSequenceTracker {
uint64_t last_started_impl_sequence_ = 0;
uint64_t last_processed_impl_sequence_ = 0;

uint64_t last_started_main_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 2f848d2

Please sign in to comment.