Open Bug 782657 Opened 7 years ago Updated 5 years ago

Convert use of sigslot to multithreaded

Categories

(Core :: WebRTC, defect)

defect
Not set

Tracking

()

mozilla20
Blocking Flags:
backlog parking-lot

People

(Reporter: ekr, Unassigned)

References

Details

Attachments

(1 file)

We use sigslot to make mtransport and mediapipeline work. There is a multithreaded mode which is activated by a define. We need it. This is a build issue.
Perhaps more to the point (though a different issue), we need to either subset libjingle for m-c (so we get sigslot without everything), or add sigslot to chromium/ipc, or import it separately, or reimplement it.
Whiteboard: [WebRTC], [blocking-webrtc+]
Sigslot has been moved over, but we still need to decide to turn on the multi-threaded mode.  ekr, do we need multithreaded mode, or is it just a good idea, or does it not matter in our use?
At minimum it's a good idea.
Blocks: 811764
Comment on attachment 690201 [details] [diff] [review]
switch all uses of sigslot to default to multithreaded (win32 already defaults that way)

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

This looks good for Mac and Linux. Does it force POSIX threads on Windows tho?
Comment on attachment 690201 [details] [diff] [review]
switch all uses of sigslot to default to multithreaded (win32 already defaults that way)

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

This looks good for Mac and Linux. Does it force POSIX threads on Windows tho?
No, it uses windows threading, but specifying this makes sure it selects non-single-threaded.
Attachment #690201 - Flags: review?(ekr) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ea2ccfd704e
Assignee: nobody → rjesup
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → mozilla20
Backed out due to deadlocks (reported by ekr, I saw one on shutdown but didn't diagnose the cause):

https://hg.mozilla.org/integration/mozilla-inbound/rev/3bb5a5a91af6

Also, the patch had the wrong bug number (752657) so it's just as well ;-)

Marking leave-open so if first patch gets uplifted this won't get closed.
Whiteboard: [WebRTC], [blocking-webrtc+] → [WebRTC], [blocking-webrtc+][leave-open]
Not sure we even want this anymore...  blocking-
Whiteboard: [WebRTC], [blocking-webrtc+][leave-open] → [WebRTC], [blocking-webrtc-][leave-open]
Byron: Do we still want this?  Or is it no longer relevant?  Or would it let us simplify other code eventually?
Assignee: rjesup → nobody
backlog: --- → parking-lot
Flags: needinfo?(docfaraday)
QA Contact: jsmith
Whiteboard: [WebRTC], [blocking-webrtc-][leave-open]
It might be a necessary step toward simplifying some code, although it won't be sufficient.
Flags: needinfo?(docfaraday)
Ok, so we should keep it (as it has a patch and may be a dependency of other simplification work).  Perhaps we should open a bug on reducing the number of cross-thread transfers, and have this a blocker or see-also for that.
You need to log in before you can comment on or make changes to this bug.