1194112 - Enable Move when possible in exclusive mode for MediaEventSource to save copy and allow move-only types

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

[JW Wang [:jwwang] [:jw_wang]]

Spin off of bug 1193117.

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

See bug 1193117 comment 2.

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ce77a4423e8

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

Attachment: MozReview Request: Bug 1194112. Part 1 - extract event dispatch code from ListenerImpl to its own class.

Bug 1194112. Part 1 - extract event dispatch code from ListenerImpl to its own class.

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

Attachment: MozReview Request: Bug 1194112. Part 2 - small code refactoring to reduce typing.

Bug 1194112. Part 2 - small code refactoring to reduce typing.

Comment 5

[JW Wang [:jwwang] [:jw_wang]]

Attachment: MozReview Request: Bug 1194112. Part 3 - use perfect forwarding in MediaEventProducer::Notify() so MediaEventSource can decide whether to copy or move according to its ListenerMode.

Bug 1194112. Part 3 - use perfect forwarding in MediaEventProducer::Notify() so MediaEventSource can decide whether to copy or move according to its ListenerMode.

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

Attachment: MozReview Request: Bug 1194112. Part 4 - reimplement Listener/ListenerImpl to support Move.

Bug 1194112. Part 4 - reimplement Listener/ListenerImpl to support Move.

Comment 7

[JW Wang [:jwwang] [:jw_wang]]

Attachment: MozReview Request: Bug 1194112. Part 5 - remove dead code.

Bug 1194112. Part 5 - remove dead code.

Comment 8

[JW Wang [:jwwang] [:jw_wang]]

Attachment: MozReview Request: Bug 1193117. Part 6 - add a test case to test the exclusive mode.

Bug 1193117. Part 6 - add a test case to test the exclusive mode.

Comment 9

[Matthew Gregan [:kinetik]]

Comment on attachment 8648538 [details]
MozReview Request: Bug 1194112. Part 1 - extract event dispatch code from ListenerImpl to its own class.

https://reviewboard.mozilla.org/r/16225/#review14593

Ship It!

Comment 10

[Matthew Gregan [:kinetik]]

Comment on attachment 8648539 [details]
MozReview Request: Bug 1194112. Part 2 - small code refactoring to reduce typing.

https://reviewboard.mozilla.org/r/16227/#review14595

Ship It!

Comment 11

[Matthew Gregan [:kinetik]]

Comment on attachment 8648540 [details]
MozReview Request: Bug 1194112. Part 3 - use perfect forwarding in MediaEventProducer::Notify() so MediaEventSource can decide whether to copy or move according to its ListenerMode.

https://reviewboard.mozilla.org/r/16229/#review14597

Ship It!

Comment 12

[Matthew Gregan [:kinetik]]

Comment on attachment 8648541 [details]
MozReview Request: Bug 1194112. Part 4 - reimplement Listener/ListenerImpl to support Move.

https://reviewboard.mozilla.org/r/16231/#review14601

Ship It!

Comment 13

[Matthew Gregan [:kinetik]]

Comment on attachment 8648542 [details]
MozReview Request: Bug 1194112. Part 5 - remove dead code.

https://reviewboard.mozilla.org/r/16233/#review14603

Ship It!

Comment 14

[Matthew Gregan [:kinetik]]

Comment on attachment 8648543 [details]
MozReview Request: Bug 1193117. Part 6 - add a test case to test the exclusive mode.

https://reviewboard.mozilla.org/r/16235/#review14605

Ship It!

Comment 15

[JW Wang [:jwwang] [:jw_wang]]

Thanks for the review!

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a7ceb6162a63
https://hg.mozilla.org/integration/mozilla-inbound/rev/acbc0d7e21cd
https://hg.mozilla.org/integration/mozilla-inbound/rev/81291b4e6dc3
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b202008a431
https://hg.mozilla.org/integration/mozilla-inbound/rev/0c54ee53678f
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0eee19c83cd

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/2b568650c03a for Linux64 Hazard failure:
subprocess.CalledProcessError: Command '['/builds/slave/l64-br-haz_m-in_dep-0000000000/build/obj-opt-js/dist/bin/js', '/builds/slave/l64-br-haz_m-in_dep-0000000000/build/source/js/src/devtools/rootAnalysis/computeGCFunctions.js', 'callgraph.txt', 'gcFunctions.tmp', 'gcFunctions_list.tmp', 'gcEdges.tmp', 'suppressedFunctions_list.tmp']' returned non-zero exit status 3

Comment 18

[JW Wang [:jwwang] [:jw_wang]]

00:13:23 INFO - CalledProcessError: Command '['hg', 'log', '-r', 'd0eee19c83cd9b2130c11eeea832dbceeafa37a9', '--template', '{node}']' returned non-zero exit status 255
00:13:23 INFO - d0eee19c83cd9b2130c11eeea832dbceeafa37a9 does not exist in /builds/hg-shared/integration/mozilla-inbound

I can't see how this error is related to my changes... It looks like an error of hg repo.

Comment 19

[JW Wang [:jwwang] [:jw_wang]]

https://treeherder.mozilla.org/logviewer.html#?job_id=10500793&repo=try
Got an error in the hazard build even with part 1 pushed only. I have no idea how my code changes could cause such errors.


Hi Ehsan,
Do you have any idea about these hazard build error messages? 

06:52:09 INFO - CalledProcessError: Command '['hg', 'log', '-r', '31707542ff6f064bfc6879e4e39d83dac81ea506', '--template', '{node}']' returned non-zero exit status 255 

 08:26:09 INFO - /builds/slave/l64-br-haz_try_dep-00000000000/build/source/js/src/devtools/rootAnalysis/loadCallgraph.js:77:21 InternalError: allocation size overflow
08:26:09 ERROR - Traceback (most recent call last):

Comment 20

[JW Wang [:jwwang] [:jw_wang]]

https://wiki.mozilla.org/Javascript:Hazard_Builds
My changes are nothing about js. I wonder how it would be...

Comment 21

[(no longer active)]

(In reply to JW Wang [:jwwang] from comment #19)
> Hi Ehsan,
> Do you have any idea about these hazard build error messages? 
> 
> 06:52:09 INFO - CalledProcessError: Command '['hg', 'log', '-r',
> '31707542ff6f064bfc6879e4e39d83dac81ea506', '--template', '{node}']'
> returned non-zero exit status 255 
> 
>  08:26:09 INFO -
> /builds/slave/l64-br-haz_try_dep-00000000000/build/source/js/src/devtools/
> rootAnalysis/loadCallgraph.js:77:21 InternalError: allocation size overflow
> 08:26:09 ERROR - Traceback (most recent call last):

No, and I'm not even sure why you're asking me.  I think sfink owns that analysis.

Comment 22

[Steve Fink [:sfink] [:s:]]

(In reply to Ehsan Akhgari (don't ask for review please) from comment #21)
> (In reply to JW Wang [:jwwang] from comment #19)
> > Hi Ehsan,
> > Do you have any idea about these hazard build error messages? 
> > 
> No, and I'm not even sure why you're asking me.  I think sfink owns that
> analysis.

Yes, this is me.

(In reply to JW Wang [:jwwang] from comment #20)
> https://wiki.mozilla.org/Javascript:Hazard_Builds
> My changes are nothing about js. I wonder how it would be...

The hazard analysis is checking the full tree's source code. But it's true that you should only be able to trigger problems by touching code that uses the JSAPI, or code the calls code that uses the JSAPI, etc.

That said, this particular problem is new to me and seems like there's something going wrong with the analysis. "InternalError: allocation size overflow". Wtf? I will look into this immediately.

Comment 23

[Steve Fink [:sfink] [:s:]]

Oh, and by the way:

(In reply to JW Wang [:jwwang] from comment #19)
> 06:52:09 INFO - CalledProcessError: Command '['hg', 'log', '-r',
> '31707542ff6f064bfc6879e4e39d83dac81ea506', '--template', '{node}']'
> returned non-zero exit status 255 

this seems to be a nuisance error message that occurs even in successful runs. I really thought there was a bug filed for this, but I'm not finding it now.

Comment 24

[JW Wang [:jwwang] [:jw_wang]]

(In reply to Ehsan Akhgari (don't ask for review please) from comment #21)
> No, and I'm not even sure why you're asking me.  I think sfink owns that
> analysis.

Sorry. I though you know about hazard builds as well static analysis builds. Thanks for forwarding the ni to the right person. :)

Comment 25

[JW Wang [:jwwang] [:jw_wang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b78e5f59f769
Looks like the problem is fixed. Thanks! Steve.

Comment 26

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2afb1fcf608b
https://hg.mozilla.org/integration/mozilla-inbound/rev/e55b03747ebd
https://hg.mozilla.org/integration/mozilla-inbound/rev/37fb89797a7f
https://hg.mozilla.org/integration/mozilla-inbound/rev/1254f92e2493
https://hg.mozilla.org/integration/mozilla-inbound/rev/d9d782e4fbc4
https://hg.mozilla.org/integration/mozilla-inbound/rev/2875976a9a6f

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2afb1fcf608b
https://hg.mozilla.org/mozilla-central/rev/e55b03747ebd
https://hg.mozilla.org/mozilla-central/rev/37fb89797a7f
https://hg.mozilla.org/mozilla-central/rev/1254f92e2493
https://hg.mozilla.org/mozilla-central/rev/d9d782e4fbc4
https://hg.mozilla.org/mozilla-central/rev/2875976a9a6f