Closed Bug 1482032 Opened 3 years ago Closed 3 years ago

Only register a value for CONTENT_FRAME_TIME if we actually drew something

Categories

(Core :: Graphics: WebRender, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: sotaro, Assigned: sotaro)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files, 3 obsolete files)

Created by Bug 1481950 Comment 1.
Assignee: nobody → sotaro.ikeda.g
Blocks: 1481950
With attachment 8998733 [details] [diff] [review], WrBridge()->SendSetFocusTarget() is still called. It could be handled as a different problem.
By Bug 1481950 comment 3, attachment 8998734 [details] [diff] [review] might affect to MozAfterPaint.
(In reply to Sotaro Ikeda [:sotaro] from comment #5)
> By Bug 1481950 comment 3, attachment 8998734 [details] [diff] [review] might
> affect to MozAfterPaint.

Copy of Bug 1481950 comment 3.

---------------------------------------------

Thanks Sotaro!

It might also make sense to only register a value for CONTENT_FRAME_TIME if we actually drew something. Both backends seem to have fallbacks path where we call DidComposite/NotifyPipelineRendered even when no work was done.

I think we need some code to run to ensure MozAfterPaint is delivered (since JS can be waiting for it), but we should be able to skip recording the telemetry value.
Attachment #8998734 - Attachment is obsolete: true
Summary: Reduce forwarding EmptyTransaction if it is not necessary → Only register a value for CONTENT_FRAME_TIME if we actually drew something
Changed summary by comment 6.
Comment on attachment 8999506 [details] [diff] [review]
patch part 2 - Only register a value for CONTENT_FRAME_TIME if we actually drew something

:mattwoodrow, can you feedback to the patch?
Attachment #8999506 - Flags: review?(matt.woodrow)
Attachment #8999506 - Flags: review?(matt.woodrow) → feedback?(matt.woodrow)
Comment on attachment 8999506 [details] [diff] [review]
patch part 2 - Only register a value for CONTENT_FRAME_TIME if we actually drew something

Review of attachment 8999506 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!

Have you checked in about:telemetry to see if the low values go away?
Attachment #8999506 - Flags: feedback?(matt.woodrow) → feedback+
(In reply to Matt Woodrow (:mattwoodrow) from comment #12)
> 
> Have you checked in about:telemetry to see if the low values go away?

Yes, I checked the CONTENT_FRAME_TIME in about:telemetry. The very low values went away.


Before applying the patches, CONTENT_FRAME_TIME was like the following.

****************************************
CONTENT_FRAME_TIME
159 samples, average = 107.3, sum = 17068

  2 |  0  0%
  3 |#  3  2%
  4 |##  6  4%
  5 |#  4  3%
  9 |  1  1%
 11 |  1  1%
 13 |#  4  3%
 15 |#  3  2%
 21 |  1  1%
 40 |  1  1%
 55 |#  2  1%
 64 |#  3  2%
 75 |#  3  2%
 88 |#  3  2%
103 |#########################  73  46%
120 |##########  29  18%
140 |###  9  6%
164 |##  5  3%
192 |#  4  3%
224 |#  3  2%
357 |  1  1%
417 |  0  0%

****************************************


After applying the patches, CONTENT_FRAME_TIME was like the following. The very low values went away.

****************************************
CONTENT_FRAME_TIME
187 samples, average = 122.3, sum = 22868

  9 |  0  0%
 11 |  1  1%
 34 |  1  1%
 64 |#  5  3%
 75 |#  4  2%
 88 |###  10  5%
103 |#########################  100  53%
120 |###########  43  23%
140 |##  8  4%
164 |##  6  3%
192 |#  4  2%
224 |#  3  2%
262 |  1  1%
417 |  1  1%
487 |  0  0%


****************************************
Attachment #8999504 - Flags: review?(matt.woodrow)
Attachment #8999506 - Flags: review?(matt.woodrow)
Attachment #8999504 - Flags: review?(matt.woodrow) → review+
Attachment #8999506 - Flags: review?(matt.woodrow) → review+
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1f4543fd3fb
part 1 - A bit clean up of FlushTransactionIdsForEpoch() r=mattwoodrow
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc38e7f7ca73
part 2 - Only register a value for CONTENT_FRAME_TIME if we actually drew something r=mattwoodrow
https://hg.mozilla.org/mozilla-central/rev/c1f4543fd3fb
https://hg.mozilla.org/mozilla-central/rev/bc38e7f7ca73
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.