Closed Bug 1073545 Opened 10 years ago Closed 10 years ago

Add Vsync Markers to SPS Profiler

Categories

(Core :: Gecko Profiler, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: mchang, Assigned: mchang)

References

Details

Attachments

(4 files, 3 obsolete files)

Instrument the SPS profiler to use vsync marker data. This will enable us to see when we miss a frame.
Version: 26 Branch → Trunk
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)
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!
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)
Attachment #8497589 - Flags: review?(bgirard) → review+
I also reviewed the github changes. Looks really nice. Great work!
Depends on: 1075096
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+
Attachment #8497589 - Attachment is obsolete: true
Attachment #8497192 - Flags: review?(bgirard)
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)
Comment on attachment 8497192 [details] [review]
Pull Request - Enable Cleopatra to display vsync markers

Landed on Cleopatra Front End
Attachment #8497192 - Flags: review?(bgirard)
Try issues on OS x 10.8. Trying again - https://tbpl.mozilla.org/?tree=Try&rev=36795dc453ba
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+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/094128a6d5a6
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: