Closed Bug 1242268 Opened 4 years ago Closed 3 years ago

Intermittent test_WebAudioMemoryReporting.html | Non-zero usage for explicit/webaudio/audio-node/ScriptProcessorNode/dom-nodes

Categories

(Core :: Web Audio, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
e10s + ---
firefox50 --- fixed

People

(Reporter: philor, Assigned: karlt)

References

Details

(Keywords: intermittent-failure)

Attachments

(6 files)

Version: 45 Branch → unspecified
Blocks: e10s-tests
tracking-e10s: --- → +
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1242268&entireHistory=true&tree=trunk
says this was a short term problem, occurring on revisions from bbfbd1e73bf1
to 31373f8aaa98, mochitest-3 and mochitest-e10s-3, Win 7, opt and pgo.

However, this has shown up again on Ubuntu 16.04 in
https://bugzilla.mozilla.org/show_bug.cgi?id=1281179#c8
Assignee: nobody → karlt
Blocks: 1281179
No longer blocks: e10s-tests
I suspect the Linux failures at least are either due to timeout during wait or the use of wait without checking the condition.

http://searchfox.org/mozilla-central/rev/ef24c234ed53b3ba50a1734f6b946942e4434b5b/dom/media/MediaStreamGraph.cpp#3474

man 3p pthread_cond_timedwait:

"When using condition variables there is always a Boolean predicate involv‐
ing shared variables associated with each condition wait that is  true  if
the  thread  should proceed. Spurious wakeups from the pthread_cond_timed‐
wait() or pthread_cond_wait() functions may occur. Since the  return  from
pthread_cond_timedwait()  or  pthread_cond_wait()  does not imply anything
about the value of this predicate, the predicate  should  be  re-evaluated
upon such return."

https://hg.mozilla.org/try/rev/00cb64ed7e3a82c449ce049cfe65fbfa25702239
may be a good enough bandaid if this is holding back bug 1281179, but I'll look at using RegisterWeakAsyncMemoryReporter() to do this properly.
Review commit: https://reviewboard.mozilla.org/r/62076/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/62076/
Attachment #8767610 - Flags: review?(padenot)
Attachment #8767611 - Flags: review?(padenot)
Attachment #8767612 - Flags: review?(padenot)
Attachment #8767613 - Flags: review?(padenot)
Attachment #8767614 - Flags: review?(padenot)
Attachment #8767615 - Flags: review?(padenot)
This will permit allowing the main thread to run while collecting
reports from graph thread objects.

Review commit: https://reviewboard.mozilla.org/r/62084/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/62084/
This removes the one second timeout for MSG collection, extending the timeout
period to the 50 second timeout of nsMemoryReporterManager.

Also removed:

* The condition variable logic that can stop waiting without checking the
  condition set when memory reports are complete.

* Races with mAudioStreamSizes modification on two threads after wait timeout.

Memory from streams in offline graphs that are not yet running is now also
included.

Review commit: https://reviewboard.mozilla.org/r/62086/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/62086/
(In reply to Karl Tomlinson (ni?:karlt) from comment #1)
> However, this has shown up again on Ubuntu 16.04 in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1281179#c8

These patches address this:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b6afe6747bb1

Another try run here.  The many linux failures are due to using a different vm.
Other platform results are good.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0c63c4b16c8c
Summary: Intermittent e10s test_WebAudioMemoryReporting.html | Non-zero usage for explicit/webaudio/audio-node/ScriptProcessorNode/dom-nodes → Intermittent test_WebAudioMemoryReporting.html | Non-zero usage for explicit/webaudio/audio-node/ScriptProcessorNode/dom-nodes
Comment on attachment 8767610 [details]
bug 1242268 document mLifecycleState multithread access management

https://reviewboard.mozilla.org/r/62076/#review58800
Attachment #8767610 - Flags: review?(padenot) → review+
Comment on attachment 8767611 [details]
bug 1242268 don't try to wake up an empty MSG that is shutting down to report memory

https://reviewboard.mozilla.org/r/62078/#review58802
Attachment #8767611 - Flags: review?(padenot) → review+
Comment on attachment 8767612 [details]
bug 1242268 use pointers instead of copies of string literals in AudioNode memory reporting

https://reviewboard.mozilla.org/r/62080/#review58804
Attachment #8767612 - Flags: review?(padenot) → review+
Comment on attachment 8767613 [details]
bug 1242268 store a node type string pointer on engines and use that for memory reporting

https://reviewboard.mozilla.org/r/62082/#review58806
Attachment #8767613 - Flags: review?(padenot) → review+
Attachment #8767614 - Flags: review?(padenot) → review+
Comment on attachment 8767614 [details]
bug 1242268 collect memory reports of AudioNode dom objects on the main thread

https://reviewboard.mozilla.org/r/62084/#review58808
Comment on attachment 8767615 [details]
bug 1242268 record MSG memory sizes asynchronously

https://reviewboard.mozilla.org/r/62086/#review58810
Attachment #8767615 - Flags: review?(padenot) → review+
is there anything blocking this from landing?
Karl is on PTO, and I don't know why he didn't land that earlier, but he must have had a good reason. I don't know much, sorry.
Flags: needinfo?(padenot)
another week has passed, possibly :karlt is back and caught up- would like to figure out what remains here and get this landed :)
Flags: needinfo?(karlt)
Pushed by ktomlinson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0c21f0c5a296
document mLifecycleState multithread access management r=padenot
https://hg.mozilla.org/integration/autoland/rev/20c9343e899c
don't try to wake up an empty MSG that is shutting down to report memory r=padenot
https://hg.mozilla.org/integration/autoland/rev/3640f261f9e0
use pointers instead of copies of string literals in AudioNode memory reporting r=padenot
https://hg.mozilla.org/integration/autoland/rev/86bf660ce796
store a node type string pointer on engines and use that for memory reporting r=padenot
https://hg.mozilla.org/integration/autoland/rev/f66344471c52
collect memory reports of AudioNode dom objects on the main thread r=padenot
https://hg.mozilla.org/integration/autoland/rev/a85f3a50fd5f
record MSG memory sizes asynchronously r=padenot
Flags: needinfo?(karlt)
Duplicate of this bug: 1225282
You need to log in before you can comment on or make changes to this bug.