Bug 1667696 - Use standard rust channels on Windows. r=jrmuizel
authorNicolas Silva <nsilva@mozilla.com>
Mon, 05 Oct 2020 14:01:08 +0000
changeset 551665 e6d7b596358d33fb54a73f330e89543ddf99da39
parent 551664 c5d47bafb1c39e8798a9f10d81e92e887455e7b5
child 551666 95db4cd7c8b1e36cf09064a6d7193f0433d44645
push id37839
push userbtara@mozilla.com
push dateTue, 06 Oct 2020 16:17:06 +0000
treeherdermozilla-central@19bc84a9ed83 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs1667696
milestone83.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1667696 - Use standard rust channels on Windows. r=jrmuizel While we measuered somewhat surprisingly high performance improvements on linux when replacing standard channels with crossbeam ones on Linux, there has been a CONTENT_FRAME_TIME regression on Windows around the same time. In doubt, this patch makes us use standard channels on Windows to see if it fixes the regression. This patch will be reverted if it doesn't turn out restore the CONTENT_FRAME_TIME numbers. Swgl needs to continue using crossbeam because it depends on select which doesn't exist in standard channels. Differential Revision: https://phabricator.services.mozilla.com/D92383
gfx/webrender_bindings/src/swgl_bindings.rs
gfx/wr/webrender/src/debug_server.rs
gfx/wr/webrender_api/src/channel.rs
--- a/gfx/webrender_bindings/src/swgl_bindings.rs
+++ b/gfx/webrender_bindings/src/swgl_bindings.rs
@@ -568,17 +568,17 @@ impl SwCompositeGraphNode {
                 let band_index = self.band_index.fetch_add(1, Ordering::SeqCst);
                 job.process(band_index);
             }
         }
     }
 
     /// After processing a band, check all child dependencies and remove this parent from
     /// their dependency counts. If applicable, queue the new child bands for composition.
-    fn unblock_children(&self, sender: &channel::Sender<Arc<SwCompositeGraphNode>>) {
+    fn unblock_children(&self, sender: &channel::crossbeam::Sender<Arc<SwCompositeGraphNode>>) {
         if self.num_bands.fetch_sub(1, Ordering::SeqCst) > 1 {
             return;
         }
         // Clear the job to release any locked resources.
         self.job.replace(None);
         for child in self.children.borrow().iter() {
             // Remove the child's parent dependency on this node. If there are no more
             // parent dependencies left, send all the bands for composition.
@@ -592,34 +592,34 @@ impl SwCompositeGraphNode {
     }
 }
 
 /// The SwComposite thread processes a queue of composite jobs, also signaling
 /// via a condition when all available jobs have been processed, as tracked by
 /// the job count.
 struct SwCompositeThread {
     /// Queue of available composite jobs
-    job_sender: channel::Sender<Arc<SwCompositeGraphNode>>,
-    job_receiver: channel::Receiver<Arc<SwCompositeGraphNode>>,
+    job_sender: channel::crossbeam::Sender<Arc<SwCompositeGraphNode>>,
+    job_receiver: channel::crossbeam::Receiver<Arc<SwCompositeGraphNode>>,
     /// Count of unprocessed jobs still in the queue
     job_count: AtomicUsize,
     /// Condition signaled when there are no more jobs left to process.
-    jobs_completed: channel::Receiver<()>,
+    jobs_completed: channel::crossbeam::Receiver<()>,
 }
 
 /// The SwCompositeThread struct is shared between the SwComposite thread
 /// and the rendering thread so that both ends can access the job queue.
 unsafe impl Sync for SwCompositeThread {}
 
 impl SwCompositeThread {
     /// Create the SwComposite thread. Requires a SWGL context in which
     /// to do the composition.
     fn new() -> Arc<SwCompositeThread> {
-        let (job_sender, job_receiver) = channel::unbounded_channel();
-        let (notify_completed, jobs_completed) = channel::fast_channel(1);
+        let (job_sender, job_receiver) = channel::crossbeam::unbounded();
+        let (notify_completed, jobs_completed) = channel::crossbeam::bounded(1);
         let info = Arc::new(SwCompositeThread {
             job_sender,
             job_receiver,
             job_count: AtomicUsize::new(0),
             jobs_completed,
         });
         let result = info.clone();
         let thread_name = "SwComposite";
@@ -712,17 +712,17 @@ impl SwCompositeThread {
         // need to know if jobs are completed. If the job count hits zero here,
         // then we know the SwComposite thread is already done since all queued
         // jobs have been processed.
         if self.job_count.fetch_sub(1, Ordering::SeqCst) <= 1 {
             return;
         }
         // Otherwise, there are remaining jobs that we need to wait for.
         loop {
-            channel::select! {
+            channel::crossbeam::select! {
                 // Steal jobs from the SwComposite thread if it is busy.
                 recv(self.job_receiver) -> graph_node => if let Ok(graph_node) = graph_node {
                     if self.process_job(graph_node) {
                         // If this was the final job, then just exit.
                         break;
                     }
                 },
                 // If all jobs have been completed, it is safe to exit.
--- a/gfx/wr/webrender/src/debug_server.rs
+++ b/gfx/wr/webrender/src/debug_server.rs
@@ -1,14 +1,14 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use api::units::DeviceIntSize;
-use api::crossbeam_channel::{bounded, Sender, Receiver};
+use api::channel::{fast_channel, Sender, Receiver};
 use api::DebugFlags;
 use crate::render_api::{ApiMsg, DebugCommand};
 use crate::print_tree::PrintTreePrinter;
 use crate::renderer;
 use std::thread;
 use ws;
 use base64::encode;
 use image_loader;
@@ -106,17 +106,17 @@ pub struct DebugServerImpl {
     join_handle: Option<thread::JoinHandle<()>>,
     broadcaster: ws::Sender,
     debug_rx: Receiver<DebugMsg>,
     senders: Vec<ws::Sender>,
 }
 
 impl DebugServerImpl {
     pub fn new(api_tx: Sender<ApiMsg>) -> DebugServerImpl {
-        let (debug_tx, debug_rx) = bounded(64);
+        let (debug_tx, debug_rx) = fast_channel(64);
 
         let socket = ws::Builder::new()
             .build(move |out| {
                 Server {
                     ws: out,
                     debug_tx: debug_tx.clone(),
                     api_tx: api_tx.clone(),
                     debug_flags: DebugFlags::empty(),
--- a/gfx/wr/webrender_api/src/channel.rs
+++ b/gfx/wr/webrender_api/src/channel.rs
@@ -2,17 +2,24 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use crate::{Epoch, PipelineId};
 use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 use std::io::{self, Cursor, Error, ErrorKind, Read};
 use std::mem;
-pub use crossbeam_channel::{select, Sender, Receiver};
+
+pub use crossbeam_channel as crossbeam;
+
+#[cfg(not(target_os = "windows"))]
+pub use crossbeam_channel::{Sender, Receiver};
+
+#[cfg(target_os = "windows")]
+pub use std::sync::mpsc::{Sender, Receiver};
 
 #[derive(Clone)]
 pub struct Payload {
     /// An epoch used to get the proper payload for a pipeline id frame request.
     ///
     /// TODO(emilio): Is this still relevant? We send the messages for the same
     /// pipeline in order, so we shouldn't need it. Seems like this was only
     /// wallpapering (in most cases) the underlying problem in #991.
@@ -127,28 +134,47 @@ impl<'de, T> Deserialize<'de> for MsgSen
     fn deserialize<D>(_: D) -> Result<MsgSender<T>, D::Error>
                       where D: Deserializer<'de> {
         unreachable!();
     }
 }
 
 /// A create a channel intended for one-shot uses, for example the channels
 /// created to block on a synchronous query and then discarded,
+#[cfg(not(target_os = "windows"))]
 pub fn single_msg_channel<T>() -> (Sender<T>, Receiver<T>) {
     crossbeam_channel::bounded(1)
 }
 
 /// A fast MPMC message channel that can hold a fixed number of messages.
 ///
 /// If the channel is full, the sender will block upon sending extra messages
 /// until the receiver has consumed some messages.
 /// The capacity parameter should be chosen either:
 ///  - high enough to avoid blocking on the common cases,
 ///  - or, on the contrary, using the blocking behavior as a means to prevent
 ///    fast producers from building up work faster than it is consumed.
+#[cfg(not(target_os = "windows"))]
 pub fn fast_channel<T>(capacity: usize) -> (Sender<T>, Receiver<T>) {
     crossbeam_channel::bounded(capacity)
 }
 
 /// Creates an MPMC channel that is a bit slower than the fast_channel but doesn't
 /// have a limit on the number of messages held at a given time and therefore
 /// doesn't block when sending.
+#[cfg(not(target_os = "windows"))]
 pub use crossbeam_channel::unbounded as unbounded_channel;
+
+
+#[cfg(target_os = "windows")]
+pub fn fast_channel<T>(_cap: usize) -> (Sender<T>, Receiver<T>) {
+    std::sync::mpsc::channel()
+}
+
+#[cfg(target_os = "windows")]
+pub fn unbounded_channel<T>() -> (Sender<T>, Receiver<T>) {
+    std::sync::mpsc::channel()
+}
+
+#[cfg(target_os = "windows")]
+pub fn single_msg_channel<T>() -> (Sender<T>, Receiver<T>) {
+    std::sync::mpsc::channel()
+}