1030435 - Re-enable peerIdentity test

Status: RESOLVED WONTFIX

(Core :: WebRTC, defect, P5)

Description

[Martin Thomson [:mt:]]

Bug 1021776 noted an intermittent failure caused by poor emulator performance.  This was initially at low rates, but it increased over time to fail on about 30-40% of runs.

Aggressively backing off on the timers seems to alleviate the problems: https://tbpl.mozilla.org/?tree=Try&rev=7a4d7c1f6b47

Comment 1

[Martin Thomson [:mt:]]

Attachment: Attempting to ease load on B2G emulator for tests

Comment 2

[Martin Thomson [:mt:]]

Attachment: Re-enabling gUM peerIdentity test

Comment 3

[Martin Thomson [:mt:]]

Comment on attachment 8446214 [details] [diff] [review]
Attempting to ease load on B2G emulator for tests

This one exponentially backs off the timers so that the emulator has a chance.  It also suppresses rendering and disconnects nodes that aren't in use.  This seems to do the trick.

Maybe when we get principal-based events on tracks this can be removed.  Until then, I think that this is safe to proceed with.

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8446214 [details] [diff] [review]
Attempting to ease load on B2G emulator for tests

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

lgtm.

::: dom/media/tests/mochitest/blacksilence.js
@@ +35,5 @@
>        }
> +    }
> +    function setupNext() {
> +      timeout = setTimeout(periodic, 200 << num);
> +      num++;

Maybe add a comment explaining why this approach is here and seems to work.

@@ +81,5 @@
>      var e = document.createElement(type);
>      e.width = 32;
>      e.height = 24;
> +    // var display = document.getElementById('display');
> +    // display.appendChild(e);

I think commented-out code frowned on. Maybe add a comment explaining what needs to happen to re-enable i? It's not clear.

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

> >      var e = document.createElement(type);
> >      e.width = 32;
> >      e.height = 24;
> > +    // var display = document.getElementById('display');
> > +    // display.appendChild(e);

Also, is it a problem that mkElement() now returns something not attached to anything?

Comment 6

[Martin Thomson [:mt:]]

(In reply to Jan-Ivar Bruaroey [:jib] from comment #4)
> I think commented-out code frowned on. Maybe add a comment explaining what
> needs to happen to re-enable i? It's not clear.

That's just an accident.  I thought that I'd cut that out.

(In reply to Jan-Ivar Bruaroey [:jib] from comment #5)
> Also, is it a problem that mkElement() now returns something not attached to
> anything?

The tests pass :)  The basic idea here is that the element is still attached to the document, but since it's not in the tree it doesn't incur any rendering costs.  This is as aggressive a reduction in cycles as I could manage without being too disruptive.

Comment 7

[Martin Thomson [:mt:]]

Attachment: Attempting to ease load on B2G emulator for tests r=jib

Comment 8

[Martin Thomson [:mt:]]

Comment on attachment 8446647 [details] [diff] [review]
Attempting to ease load on B2G emulator for tests r=jib

Carrying r=jib

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/247b2cbfadf6
https://hg.mozilla.org/integration/mozilla-inbound/rev/a4d9432cdd70

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Still hitting intermittent failures:
https://tbpl.mozilla.org/php/getParsedLog.php?id=42643186&tree=Mozilla-Inbound

I backed out the re-enabling part (but left in the first patch).
https://hg.mozilla.org/integration/mozilla-inbound/rev/4179297b82a3

Comment 11

[Martin Thomson [:mt:]]

Thanks, that was the right call.  It looks like my try runs didn't hit the slowest machines.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Windows failure too:
https://tbpl.mozilla.org/php/getParsedLog.php?id=42648705&tree=Mozilla-Inbound

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Whoops, that's not the gUM test, so maybe unrelated.

Comment 14

[Martin Thomson [:mt:]]

Yeah, that looks pretty serious.  But it isn't really related to this bug.  Definitely a bug, but it's beyond my modest debugging abilities to diagnose faults that occur in the kernel.

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/247b2cbfadf6

Comment 16

[Maire Reavy [:mreavy]]

Still relevant?  Overtaken by work on other identity bugs?

Comment 17

[Martin Thomson [:mt:]]

This still needs to be done, sadly.  But performance on the emulator tests is still too poor.  Byron has suggested that we might reduce frame rate (and maybe also resolution) on the fake source to alleviate some of the burden.

Comment 18

[Bulk Bug Changes for mreavy's org]

Mass change P4->P5 to align with new Mozilla triage process.

Comment 19

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:mt, maybe it's time to close this bug?

Comment 20

[Martin Thomson [:mt:]]

Long OBE.  Thanks for the reminder.