Add Vsync Markers to SPS Profiler

RESOLVED FIXED in mozilla35

Status

()

Core
Gecko Profiler
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mchang, Assigned: mchang)

Tracking

Trunk
mozilla35
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 3 obsolete attachments)

(Assignee)

Description

4 years ago
Instrument the SPS profiler to use vsync marker data. This will enable us to see when we miss a frame.
(Assignee)

Updated

4 years ago
Version: 26 Branch → Trunk
(Assignee)

Comment 1

4 years ago
Created attachment 8497189 [details] [diff] [review]
Output Vsync Timestamps in Profiles

The overall goal of this bug is to instrument the profiler to output HardwareComposer vsync times and display those in cleopatra. This portion enables the gecko profiler to create ProfilerMarkerPayloads at vsync intervals on the Compositor thread so we only have to profiler the compositor thread to get vsync markers.
Attachment #8497189 - Flags: review?(bgirard)
(Assignee)

Comment 2

4 years ago
Created attachment 8497190 [details]
Sample Profile w/ Vsync Timestamps
(Assignee)

Comment 3

4 years ago
Created attachment 8497191 [details]
Screenshot of Vsync Markers in Cleopatra
(Assignee)

Comment 4

4 years ago
Created attachment 8497192 [details] [review]
Pull Request - Enable Cleopatra to display vsync markers
Attachment #8497192 - Flags: review?(bgirard)
(Assignee)

Comment 5

4 years ago
From comment 1, the part I really dislike is how to synchronize between the android timestamp and mozilla timestamp. If a vsync interval is 16.6ms, and the HwcComposer::Vsync executes within roughly the same interval, we create a marker as the current timestamp. Then we dispatch vsyncs in TimeDuration::FromMicroseconds(androidTimestamp) distances from the start point. Otherwise, if we take TimeStamp::Now() in HwcComposer::Vsync, we will get timestamps of when the function executes, not when the vsync actually occurred. On a flame, using mozilla::TimeStamp now drifts between 14ms - 18ms which is not very good. If you have a better idea of how to sync mozilla::TimeStamps and Android TimeStamps, that'd be great!
(Assignee)

Comment 6

4 years ago
Created attachment 8497589 [details] [diff] [review]
Output Vsync Timestamps in Profiles

Found a better way. During HwcComposer::Init, we take timestamps in both android and mozilla timestamp objects. We use this as a reference point to create a Mozilla::TimeStamp object that is aligned with android timestamps.
Attachment #8497189 - Attachment is obsolete: true
Attachment #8497189 - Flags: review?(bgirard)
Attachment #8497589 - Flags: review?(bgirard)

Updated

4 years ago
Attachment #8497589 - Flags: review?(bgirard) → review+
I also reviewed the github changes. Looks really nice. Great work!
(Assignee)

Updated

4 years ago
Depends on: 1075096
(Assignee)

Comment 8

4 years ago
Created attachment 8498251 [details] [diff] [review]
Output Vsync Timestamps in Profiles. r=benwa

Carrying r+. Fixed a whitespace and updated patch to proper title. Try: https://tbpl.mozilla.org/?tree=Try&rev=caddcd330ce8.

Also updated MDN to document how to read the profiler frames data.
Attachment #8498251 - Flags: review+
(Assignee)

Updated

4 years ago
Attachment #8497589 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Attachment #8497192 - Flags: review?(bgirard)
(Assignee)

Comment 9

4 years ago
Comment on attachment 8497192 [details] [review]
Pull Request - Enable Cleopatra to display vsync markers

Updated github pull request with comments from the pull request. I also made the vsync marker 0.5 so it makes it harder to tell it is a bar. Also have a wiki from comment 8.
Attachment #8497192 - Flags: review?(bgirard)
(Assignee)

Comment 10

4 years ago
Comment on attachment 8497192 [details] [review]
Pull Request - Enable Cleopatra to display vsync markers

Landed on Cleopatra Front End
Attachment #8497192 - Flags: review?(bgirard)
(Assignee)

Comment 11

4 years ago
Try issues on OS x 10.8. Trying again - https://tbpl.mozilla.org/?tree=Try&rev=36795dc453ba
(Assignee)

Comment 12

4 years ago
Created attachment 8499807 [details] [diff] [review]
Bug 1073545 - Add Vsync Timestamps to Profiler. r=benwa

Carrying r+. Modified to fix a build issue on b2g emulators. Successful try - https://tbpl.mozilla.org/?tree=Try&rev=7e403d341172
Attachment #8498251 - Attachment is obsolete: true
Attachment #8499807 - Flags: review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/094128a6d5a6
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.