Update WebRTC's TaskQueue to share chromium's SequencedTaskRunner |
||
Issue descriptionWebRTC has support for TaskQueues, a comparable concept to the now available SequencedTaskRunner implementation. The WebRTC TaskQueue implementation in Chromium is currently implemented on top of base::Thread and base::MessageLoop in a 1:1 fashion. We can (and should) now take advantage of Chromium's SequencedTaskRunner implementation, task traits etc and significantly reduce the number of threads used by WebRTC based web applications. As can be seen in issue 542522, creating threads is currently one of the top crashers for WebRTC, which shows out that in addition to this being a performance issue (and arguably not a critical one at that), it's more important to do this from a stability perspective. There's furthermore a bug wrt garbage collection of PeerConnection instances, which causes lingering instances to keep 2-3 unused threads alive per instance which get allocated at PeerConnection ctor time. For this reason, the number of PeerConnection instances has been limited to 500-750 depending on platform. In preliminary testing, allocating the maximum number of PeerConnection instances on Windows, a renderer process will consume ~1530 threads. When relying on SequencedTaskRunner, this number goes down to ~25. Depending on how threads in the thread pool can be shared (with proper avoidance of MayBlock()), that number could possibly be even lower. The important thing is that the number remains constant and PC instances no longer trigger the construction of threads unless the thread pool needs it.
,
Feb 5 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b247fdd9263f6bf166b330ef875845d65f696992 commit b247fdd9263f6bf166b330ef875845d65f696992 Author: Tommi <tommi@chromium.org> Date: Mon Feb 05 09:51:46 2018 Update MediaStreamAudioProcessorTest to use ScopedTaskEnvironment instead of MessageLoop. This in anticipation of WebRTC using SequencedTaskRunner via TaskQueue. Change-Id: Ibe863aab34424c2d3c4dba3d885dadfc5c6f7f79 Bug: 808801 Reviewed-on: https://chromium-review.googlesource.com/900642 Reviewed-by: Olga Sharonova <olka@chromium.org> Commit-Queue: Olga Sharonova <olka@chromium.org> Cr-Commit-Position: refs/heads/master@{#534356} [modify] https://crrev.com/b247fdd9263f6bf166b330ef875845d65f696992/content/renderer/media/media_stream_audio_processor_unittest.cc
,
Feb 5 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0acde692b19c1945ba137230af7b386acd39fe9f commit 0acde692b19c1945ba137230af7b386acd39fe9f Author: Tommi <tommi@chromium.org> Date: Mon Feb 05 16:26:59 2018 Update WebrtcTransportTest to use ScopedTaskEnvironment instead of MessageLoopForIO directly. This in anticipation of WebRTC using SequencedTaskRunner via TaskQueue. Change-Id: I844aa6fd7e74b1a52b0e4a4f9cb033ec3e0c2c10 Bug: 808801 Reviewed-on: https://chromium-review.googlesource.com/899283 Reviewed-by: Joe Downing <joedow@chromium.org> Commit-Queue: Tommi <tommi@chromium.org> Cr-Commit-Position: refs/heads/master@{#534398} [modify] https://crrev.com/0acde692b19c1945ba137230af7b386acd39fe9f/remoting/protocol/webrtc_transport_unittest.cc
,
Mar 9 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5773863a93f2b00e2a40fa134ad32f1da48ae588 commit 5773863a93f2b00e2a40fa134ad32f1da48ae588 Author: Tommi <tommi@chromium.org> Date: Fri Mar 09 22:12:45 2018 Make WebRTC task queues run on chromium's SequencedTaskRunner. This changes the previous 1:1 TQ:thread relationship and associates WebRTC task queues with Chromium's thread pool. The effect of this is that PeerConnection objects, will start to share threads for "rtc_event_log", "EncoderQueue" and "call_worker_queue". Constructing an instance of PeerConnection will be much less costly and likely not cost any extra thread creation at all, which is why we currently have a limit on the number of instances that we allow creating. Change-Id: Ifd218c5e14e2ce634fd428606e9713a815bd8205 Bug: 808801 Reviewed-on: https://chromium-review.googlesource.com/890738 Commit-Queue: Tommi <tommi@chromium.org> Reviewed-by: Guido Urdaneta <guidou@chromium.org> Reviewed-by: Hans Wennborg <hans@chromium.org> Cr-Commit-Position: refs/heads/master@{#542246} [modify] https://crrev.com/5773863a93f2b00e2a40fa134ad32f1da48ae588/third_party/webrtc_overrides/DEPS [modify] https://crrev.com/5773863a93f2b00e2a40fa134ad32f1da48ae588/third_party/webrtc_overrides/rtc_base/task_queue.cc
,
Mar 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/be53eac959ea4146e4fa7d9db046f5ec4c861307 commit be53eac959ea4146e4fa7d9db046f5ec4c861307 Author: Tommi <tommi@chromium.org> Date: Mon Mar 12 15:30:03 2018 Add call to TaskQueue::Impl::Stop() in TQ's dtor. This call got removed in my last CL by mistake: https://chromium-review.googlesource.com/c/chromium/src/+/890738 Bug: 808801 , 820936, 820769 , 820830 , 820827 Change-Id: I6c6efb3d40d43bc564e18b013ddf5d5d3b09e1eb Reviewed-on: https://chromium-review.googlesource.com/958218 Commit-Queue: Tommi <tommi@chromium.org> Reviewed-by: Guido Urdaneta <guidou@chromium.org> Cr-Commit-Position: refs/heads/master@{#542486} [modify] https://crrev.com/be53eac959ea4146e4fa7d9db046f5ec4c861307/third_party/webrtc_overrides/rtc_base/task_queue.cc
,
Mar 12 2018
|
||
►
Sign in to add a comment |
||
Comment 1 by tommi@chromium.org
, Feb 3 2018