Skip to content

Commit

Permalink
Change frame tracker's trace object type to stringstream
Browse files Browse the repository at this point in the history
This CL changes the tracker's trace object to a stringstream, allowing
the logging of more types of data. This CL also takes advantage of said
feature to log |begin_main_frame_data_.previous_sequence| and
|args.sequence_number| in ReportBeginMainFrame() and
ReportMainFrameCausedNoDamage().

Bug: 1017291
Change-Id: I81330b10fa2347658587ee1f9e320fb185ae35cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1903609
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Mingjing Zhang <mjzhang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713621}
  • Loading branch information
Mingjing Zhang authored and Commit Bot committed Nov 7, 2019
1 parent 223bf9f commit c0ce541
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 19 deletions.
26 changes: 16 additions & 10 deletions cc/metrics/frame_sequence_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@

// This macro is used with DCHECK to provide addition debug info.
#if DCHECK_IS_ON()
#define TRACKER_DCHECK_MSG \
" in " << kFrameSequenceTrackerTypeNames[this->type_] \
<< " tracker: " << frame_sequence_trace_ << " (" \
<< frame_sequence_trace_.size() << ")";
#define TRACKER_TRACE_STREAM frame_sequence_trace_
#define TRACKER_DCHECK_MSG \
" in " << kFrameSequenceTrackerTypeNames[this->type_] \
<< " tracker: " << frame_sequence_trace_.str() << " (" \
<< frame_sequence_trace_.str().size() << ")";
#else
#define TRACKER_TRACE_STREAM EAT_STREAM_PARAMETERS
#define TRACKER_DCHECK_MSG ""
#endif

Expand Down Expand Up @@ -293,7 +295,7 @@ void FrameSequenceTracker::ReportBeginImplFrame(
if (ShouldIgnoreBeginFrameSource(args.source_id))
return;

LogFrameSequenceTrace('b');
TRACKER_TRACE_STREAM << 'b';
UpdateTrackedFrameData(&begin_impl_frame_data_, args.source_id,
args.sequence_number);
impl_throughput_.frames_expected +=
Expand All @@ -311,7 +313,9 @@ void FrameSequenceTracker::ReportBeginMainFrame(
if (ShouldIgnoreBeginFrameSource(args.source_id))
return;

LogFrameSequenceTrace('B');
TRACKER_TRACE_STREAM << 'B';
TRACKER_TRACE_STREAM << "(" << begin_main_frame_data_.previous_sequence << ","
<< args.sequence_number << ")";
UpdateTrackedFrameData(&begin_main_frame_data_, args.source_id,
args.sequence_number);
if (first_received_main_sequence_ == 0)
Expand Down Expand Up @@ -345,7 +349,7 @@ void FrameSequenceTracker::ReportSubmitFrame(
origin_args.sequence_number >= first_received_main_sequence_) {
if (last_submitted_main_sequence_ == 0 ||
origin_args.sequence_number > last_submitted_main_sequence_) {
LogFrameSequenceTrace('S');
TRACKER_TRACE_STREAM << 'S';

last_submitted_main_sequence_ = origin_args.sequence_number;
main_frames_.push_back(frame_token);
Expand Down Expand Up @@ -381,7 +385,7 @@ void FrameSequenceTracker::ReportFramePresented(
return;
}

LogFrameSequenceTrace('P');
TRACKER_TRACE_STREAM << 'P';

TRACE_EVENT_ASYNC_STEP_INTO_WITH_TIMESTAMP0(
"cc,benchmark", "FrameSequenceTracker", this, "FramePresented",
Expand Down Expand Up @@ -457,7 +461,7 @@ void FrameSequenceTracker::ReportImplFrameCausedNoDamage(
ack.sequence_number < begin_impl_frame_data_.previous_sequence) {
return;
}
LogFrameSequenceTrace('n');
TRACKER_TRACE_STREAM << 'n';
DCHECK_GT(impl_throughput_.frames_expected, 0u) << TRACKER_DCHECK_MSG;
DCHECK_GT(impl_throughput_.frames_expected, impl_throughput_.frames_produced)
<< TRACKER_DCHECK_MSG;
Expand All @@ -482,7 +486,9 @@ void FrameSequenceTracker::ReportMainFrameCausedNoDamage(
return;
}

LogFrameSequenceTrace('N');
TRACKER_TRACE_STREAM << 'N';
TRACKER_TRACE_STREAM << "(" << begin_main_frame_data_.previous_sequence << ","
<< args.sequence_number << ")";
DCHECK_GT(main_throughput_.frames_expected, 0u) << TRACKER_DCHECK_MSG;
DCHECK_GT(main_throughput_.frames_expected, main_throughput_.frames_produced)
<< TRACKER_DCHECK_MSG;
Expand Down
12 changes: 3 additions & 9 deletions cc/metrics/frame_sequence_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,12 +182,6 @@ class CC_EXPORT FrameSequenceTracker {
termination_status_ = TerminationStatus::kScheduledForTermination;
}

inline void LogFrameSequenceTrace(unsigned char letter) {
#if DCHECK_IS_ON()
frame_sequence_trace_.push_back(letter);
#endif
}

struct TrackedFrameData {
// Represents the |BeginFrameArgs::source_id| and
// |BeginFrameArgs::sequence_number| fields of the last processed
Expand Down Expand Up @@ -292,15 +286,15 @@ class CC_EXPORT FrameSequenceTracker {
const base::TimeDelta time_delta_to_report_ = base::TimeDelta::FromSeconds(5);

#if DCHECK_IS_ON()
// This string represents a sequence of frame reporting activities on the
// current tracker. Each letter can be one of the following:
// This stringstream represents a sequence of frame reporting activities on
// the current tracker. Each letter can be one of the following:
// {'B', 'N', 'b', 'n', 'S', 'P'}, where
// 'B' = ReportBeginMainFrame(), 'N' = ReportMainFrameCausedNoDamage(),
// 'b' = ReportBeginImplFrame(), 'n' = ReportMainFrameCausedNoDamage(),
// 'S' = ReportSubmitFrame() and 'P' = ReportFramePresented().
// Note that |frame_sequence_trace_| is only defined and populated
// when DCHECK is on.
std::string frame_sequence_trace_;
std::stringstream frame_sequence_trace_;
#endif
};

Expand Down

0 comments on commit c0ce541

Please sign in to comment.