basic_compositor_video test generating infinite values

RESOLVED FIXED in Firefox 50

Status

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: wlach, Assigned: cpearce)

Tracking

unspecified
mozilla51
Points:
---

Firefox Tracking Flags

(firefox50 fixed, firefox51 fixed)

Details

Attachments

(2 attachments)

In bug 1295536, we found an instance of PERFHERDER_DATA with infinite values in it:

https://archive.mozilla.org/pub/firefox/tinderbox-builds/autoland-win64/1471329804/autoland_win8_64_test-g4-e10s-bm112-tests1-windows-build126.txt.gz
00:53:02     INFO -  PERFHERDER_DATA: {"framework": {"name": "talos"}, "suites": [{"extraOptions": ["e10s"], "name": "basic_compositor_video", "lowerIsBetter": true, "alertThreshold": 2.0, "value": Infinity, "subtests": [{"name": "240p.120fps.mp4_scale_fullscreen_startup", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [5.139743150684931, 5.154939862542956, 5.201490467937608, 5.235785340314136, 5.130923076923077, 5.202954939341422, 5.173327586206896, 5.1196160409556315, 5.130846153846154, 5.043218487394958, 5.042336134453782, 5.119957337883959], "value": 5.130923076923077, "unit": "ms/frame"}, {"name": "240p.120fps.mp4_scale_fullscreen_inclip", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [4.879073170731706, 4.929532019704433, 4.8906356968215166, 4.963498759305211, 4.846053268765132, 4.914398034398034, 4.939086419753087, 4.926674876847289, 4.845508474576273, 4.9026225490196085, 4.917518427518428, 4.914557739557741], "value": 4.914557739557741, "unit": "ms/frame"}, {"name": "240p.120fps.mp4_scale_1_startup", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [1.6703786191536742, 1.670604120267261, 1.670036171396772, 1.6704145798553147, 1.6705206013363025, 1.6699387868670001, 1.6705985523385298, 1.66971897607123, 1.6704148106904235, 1.6706069042316263, 1.6706820712694883, 1.6704704899777283], "value": 1.6704704899777283, "unit": "ms/frame"}, {"name": "240p.120fps.mp4_scale_1_inclip", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [1.6663280599500419, 1.6669041666666666, 1.6677541666666669, 1.6635951787198673, 1.666361365528727, 1.6663155703580357, 1.6663405495420496, 1.6668833333333335, 1.6663572023313906, 1.666899999999999, 1.6669083333333325, 1.6669500000000002], "value": 1.6668833333333335, "unit": "ms/frame"}, {"name": "240p.120fps.mp4_scale_1.1_startup", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [1.6695965498052305, 1.669913745130773, 1.6698859209794101, 1.6697690595436843, 1.6695353366722308, 1.6695965498052305, 1.6693020022246945, 1.6697050639955486, 1.6696076794657762, 1.6700139120756814, 1.6694771968854278, 1.6697996661101842], "value": 1.6697050639955486, "unit": "ms/frame"}, {"name": "240p.120fps.mp4_scale_1.1_inclip", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [1.666908333333334, 1.6663447127393844, 1.6663322231473767, 1.665499583680266, 1.6677291666666663, 1.6663363863447131, 1.6669208333333336, 1.666895833333333, 1.6663447127393844, 1.6669000000000005, 1.6668791666666676, 1.666895833333333], "value": 1.6668791666666676, "unit": "ms/frame"}, {"name": "240p.120fps.mp4_scale_2_startup", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [1.6702003338898155, 1.6695436839176405, 1.669752365052865, 1.6698692264885917, 1.6703700612131345, 1.6693770856507224, 1.6704510022271721, 1.6696466332776858, 1.6692880978865412, 1.6695242070116874, 1.6695300333704108, 1.670359131403118], "value": 1.6696466332776858, "unit": "ms/frame"}, {"name": "240p.120fps.mp4_scale_2_inclip", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [1.6668291666666664, 1.6668458333333334, 1.666908333333334, 1.6663696919233983, 1.666344712739383, 1.667558333333333, 1.665499583680266, 1.6677666666666664, 1.6676999999999984, 1.6678166666666645, 1.6669166666666662, 1.6668291666666664], "value": 1.666908333333334, "unit": "ms/frame"}, {"name": "480p.60fps.webm_scale_fullscreen_startup", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [5.860253906250001, 5.8857352941176515, 5.748180076628358, 5.750067049808429, 5.80465183752418, 5.736653919694073, 5.748888888888892, 5.80287234042553, 5.871281800391387, 5.861728515624996, 5.862685546874999, 5.861757812500002], "value": 5.80465183752418, "unit": "ms/frame"}, {"name": "480p.60fps.webm_scale_fullscreen_inclip", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [5.55633333333333, 5.559430555555557, 5.540637119113568, 5.556361111111114, 5.558833333333334, 5.55633333333333, 5.5750417827298016, 5.587136871508379, 5.59015363128492, 5.57428969359332, 5.590432960893855, 5.572186629526465], "value": 5.572186629526465, "unit": "ms/frame"}, {"name": "480p.60fps.webm_scale_1_startup", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [3.34526198439242, 3.3457580824972135, 3.349123883928574, 3.349157366071426, 3.3487834821428595, 3.3484877232142884, 3.349866071428571, 3.348370535714285, 3.3488337053571433, 3.3490457589285705, 3.349369419642857, 3.3495535714285722], "value": 3.3490457589285705, "unit": "ms/frame"}, {"name": "480p.60fps.webm_scale_1_inclip", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [3.333774999999999, 3.333758333333329, 3.3338583333333314, 3.333816666666668, 3.3354416666666657, 3.333699999999996, 3.3298752079866905, 3.333241666666672, 3.329933444259563, 3.3282279534109867, 3.3299500831946744, 3.3338916666666654], "value": 3.333699999999996, "unit": "ms/frame"}, {"name": "480p.60fps.webm_scale_1.1_startup", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [3.348805803571427, 3.3452898550724623, 3.345574136008918, 3.3457915273132617, 3.3450390189520625, 3.3460702341137116, 3.3456243032330044, 3.3454124860646632, 3.3454626532887413, 3.3464771460423686, 3.349207589285714, 3.348454241071429], "value": 3.3456243032330044, "unit": "ms/frame"}, {"name": "480p.60fps.webm_scale_1.1_inclip", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [3.32975873544094, 3.33373333333333, 3.333533333333338, 3.333883333333336, 3.333799999999998, 3.333833333333338, 3.333816666666656, 3.33373333333333, 3.333691666666673, 3.335141666666665, 3.32975873544094, 3.3281697171381084], "value": 3.33373333333333, "unit": "ms/frame"}, {"name": "480p.60fps.webm_scale_2_startup", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [3.349296875000001, 3.349564732142856, 3.34907366071429, 3.34862723214286, 3.348945312499999, 3.348867187499999, 3.34961495535714, 3.34862723214286, 3.349832589285711, 3.3491406250000018, 3.348677455357144, 3.3492912946428532], "value": 3.34907366071429, "unit": "ms/frame"}, {"name": "480p.60fps.webm_scale_2_inclip", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [3.329991680532441, 3.333858333333337, 3.3296921797004937, 3.3336583333333327, 3.335166666666676, 3.3336583333333327, 3.3299833610648912, 3.33375, 3.3303161397670618, 3.3301497504159703, 3.3354249999999954, 3.3353583333333394], "value": 3.3336583333333327, "unit": "ms/frame"}, {"name": "1080p.60fps.mp4_scale_fullscreen_startup", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [130.50347826086963, 136.37249999999997, 136.36840909090924, 136.38022727272752, 136.3918181818183, 142.93619047619063, 130.46869565217386, 142.94761904761913, 130.48413043478268, 142.85571428571433, 136.42136363636382, 157.934736842105], "value": 136.3918181818183, "unit": "ms/frame"}, {"name": "1080p.60fps.mp4_scale_fullscreen_inclip", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity], "value": Infinity, "unit": "ms/frame"}, {"name": "1080p.60fps.mp4_scale_1_startup", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [56.6231132075472, 60.000400000000084, 57.71086538461546, 61.226938775510256, 58.853431372549046, 56.60943396226407, 55.56648148148155, 60.02470000000001, 58.83490196078435, 60.03080000000002, 60.02959999999992, 61.24122448979606], "value": 60.000400000000084, "unit": "ms/frame"}, {"name": "1080p.60fps.mp4_scale_1_inclip", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity], "value": Infinity, "unit": "ms/frame"}, {"name": "1080p.60fps.mp4_scale_1.1_startup", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [62.52031249999997, 61.23551020408165, 61.238571428571476, 61.248775510204126, 58.8288235294117, 61.23387755102039, 63.8360638297872, 62.52937499999992, 62.51624999999998, 65.22684782608691, 61.24316326530614, 66.68188888888884], "value": 61.248775510204126, "unit": "ms/frame"}, {"name": "1080p.60fps.mp4_scale_1.1_inclip", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity], "value": Infinity, "unit": "ms/frame"}, {"name": "1080p.60fps.mp4_scale_2_startup", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [56.617169811320736, 56.613396226415134, 55.571851851851804, 54.55436363636373, 56.60386792452825, 55.56898148148145, 60.0175, 58.83313725490192, 56.616886792452824, 56.611037735849095, 57.709903846153864, 56.64066037735839], "value": 56.613396226415134, "unit": "ms/frame"}, {"name": "1080p.60fps.mp4_scale_2_inclip", "lowerIsBetter": true, "alertThreshold": 2.0, "replicates": [Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity, Infinity], "value": Infinity, "unit": "ms/frame"}]}]}

Avi, IIRC you're the author of this test. Could you investigate? I've made perfherder reject these kinds of values in bug 1295536.
Flags: needinfo?(avihpit)
odd, I just saw an alert for win8 basic_compositor as a 40+% regression, was trying to fill in some holes to get the root cause- I suspect it is related:
https://treeherder.mozilla.org/perf.html#/alerts?id=2493
Ethan Lin wrote it, and hopefully he'll be able to asses what's up with it.

@Matt IIRC there was some recent change to how dropped frames work (though I don't recall if it landed or not), and since this test stresses the composition, it's possible that different dropped frames behavior could affect the test in more than one way. Thoughts?
Flags: needinfo?(matt.woodrow)
Flags: needinfo?(ethlin)
Flags: needinfo?(avihpit)
I don't think we've changed the frame drop logic in the last few months, so this seems unlikely to be related.
Flags: needinfo?(matt.woodrow)
Seems likely that it's bug 1258870, that landed less than 12 hours before this was filed.
Yeah. That's the dropped frames logic change I recalled. Thanks.
It looks like the test will divide by 0 (giving Infinity) if no frames get painted. That may actually be the desired behavior here, or we could report something more explicit in that case.

Basically the video decoder can't keep up and we drop all frames since they come out of the decoder too late.

Chris, should we consider *not* doing this for normal MP4 videos and only do it for mp4? Normal videos can't easily drop the quality in response to dropped frames, so maybe presenting late frames is better than none?

Ultimately it means that this test is gated on decoder performance.
Flags: needinfo?(cpearce)
> Chris, should we consider *not* doing this for normal MP4 videos and only do it for mp4? 

Not sure I understand what this sentence means, but regardless, if we consider the new behavior good (can't judge it myself, but I trust you guys on it) in case we can't keep up, then we should probably keep it.

However, this makes it really harder to measure the compositor throughput, and I think it's something we do want to keep measuring.

Maybe this new drop behavior could be put behind a pref, which we'll set accordingly when running this test?
(In reply to Avi Halachmi (:avih) from comment #7)
> > Chris, should we consider *not* doing this for normal MP4 videos and only do it for mp4? 
> 
> Not sure I understand what this sentence means, but regardless, if we
> consider the new behavior good (can't judge it myself, but I trust you guys
> on it) in case we can't keep up, then we should probably keep it.
> 
> However, this makes it really harder to measure the compositor throughput,
> and I think it's something we do want to keep measuring.
> 
> Maybe this new drop behavior could be put behind a pref, which we'll set
> accordingly when running this test?

The second 'mp4' was meant to say 'MSE'.
I've attached a patch which adds a pref that the talos tests can set to test with behaviour that should be equivalent to the old.
Flags: needinfo?(cpearce)
Comment on attachment 8782839 [details]
Bug 1295630 - Provide a way for A/V sync to be ruined, so talos can test compositor throughput by painting expired frames.

https://reviewboard.mozilla.org/r/72868/#review70634
Attachment #8782839 - Flags: review?(matt.woodrow) → review+
Flags: needinfo?(ethlin)
Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4bfd89efb0c1
Provide a way for A/V sync to be ruined, so talos can test compositor throughput by painting expired frames. r=mattwoodrow
https://hg.mozilla.org/mozilla-central/rev/4bfd89efb0c1
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
This is still happening, revealed when I tried to enforce a "no infinite values" on perfherder data: https://bugzilla.mozilla.org/show_bug.cgi?id=1295536#c21 https://treeherder.mozilla.org/logviewer.html#?job_id=2348544&repo=autoland

I guess we need to actually enable the preference in Talos.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(I can take care of this tomorrow, but fwiw it would have been nice if the original patch did this)
Comment on attachment 8784509 [details]
Bug 1295630 - Allow a/v syncing to be ruined when running talos

https://reviewboard.mozilla.org/r/73946/#review71792

oh!
Attachment #8784509 - Flags: review?(jmaher) → review+
don't we want this only for the video composition test?
i.e. around here? http://hg.mozilla.org/mozilla-central/file/tip/testing/talos/talos/test.py#l555
(In reply to Avi Halachmi (:avih) from comment #19)
> don't we want this only for the video composition test?
> i.e. around here?
> http://hg.mozilla.org/mozilla-central/file/tip/testing/talos/talos/test.
> py#l555

Good point, I'll update the patch.
I have requested Aurora uplift on the A/V sync fix in Bug 1258870. If that is approved, you'll need to uplift the patches here too, otherwise you'll have failures on Aurora as well.
Pushed by wlachance@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d931c9b5ab72
Allow a/v syncing to be ruined when running talos r=jmaher
Bug 1258870 has landed on Aurora. You'll want to uplift the patches here to Aurora too no doubt.
Comment on attachment 8782839 [details]
Bug 1295630 - Provide a way for A/V sync to be ruined, so talos can test compositor throughput by painting expired frames.

Approval Request Comment
[Feature/regressing bug #]: Ability to test compositing performance with Talos - bug 1295630

[User impact if declined]: None, but we will lose Talos test coverage for video compositing on win32, which could let some other bug slip through.

[Describe test coverage new/current, TreeHerder]: This patch is about keeping an existing test running 

[Risks and why]: Very low. This patch should only change behaviour when explicitly enabled via a preference (and is disabled by default).
 
[String/UUID change made/needed]: None.
Attachment #8782839 - Flags: approval-mozilla-aurora?
Comment on attachment 8784509 [details]
Bug 1295630 - Allow a/v syncing to be ruined when running talos

Approval Request Comment
[Feature/regressing bug #]: Ability to test compositing performance with Talos - bug 1295630

[User impact if declined]: None, but we will lose Talos test coverage for video compositing on win32, which could let some other bug slip through.

[Describe test coverage new/current, TreeHerder]: This patch is about keeping an existing test running 

[Risks and why]: None. This patch only affects how Talos is run, and doesn't change what we actually ship to users. It will need the other patch in this bug applied to work though.
 
[String/UUID change made/needed]: None.
Attachment #8784509 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/d931c9b5ab72
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Comment on attachment 8784509 [details]
Bug 1295630 - Allow a/v syncing to be ruined when running talos

Test-only patches do not need relman review, they are auto approved.
Attachment #8784509 - Flags: approval-mozilla-aurora?
Whiteboard: [checkin-needed-aurora]
Comment on attachment 8782839 [details]
Bug 1295630 - Provide a way for A/V sync to be ruined, so talos can test compositor throughput by painting expired frames.

Aurora50+
Attachment #8782839 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [checkin-needed-aurora]
Can we get the 2nd patch uplifted too ? it causes bug 1299230.

thank you
Flags: needinfo?(ryanvm)
Whoops, sorry about that one!

https://hg.mozilla.org/releases/mozilla-aurora/rev/7852ac7e74fa
Assignee: nobody → cpearce
Flags: needinfo?(ryanvm)
You need to log in before you can comment on or make changes to this bug.