Use a condvar instead of a channel to implement finish_pending_tasks

Nathan Sobo and Max Brunsfeld created 4 years ago

Co-Authored-By: Max Brunsfeld <maxbrunsfeld@gmail.com>

Change summary

Cargo.lock            | 202 ++++++++++++++++++++++++++++++++++++++++++++
gpui/Cargo.toml       |   1 
gpui/src/app.rs       |  65 ++++++--------
gpui/src/presenter.rs |   2 
4 files changed, 231 insertions(+), 39 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -78,6 +78,22 @@ dependencies = [
  "futures-lite",
 ]
 
+[[package]]
+name = "async-global-executor"
+version = "2.0.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9586ec52317f36de58453159d48351bc244bc24ced3effc1fce22f3d48664af6"
+dependencies = [
+ "async-channel",
+ "async-executor",
+ "async-io",
+ "async-mutex",
+ "blocking",
+ "futures-lite",
+ "num_cpus",
+ "once_cell",
+]
+
 [[package]]
 name = "async-io"
 version = "1.3.1"
@@ -107,6 +123,15 @@ dependencies = [
  "event-listener",
 ]
 
+[[package]]
+name = "async-mutex"
+version = "1.4.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "479db852db25d9dbf6204e6cb6253698f175c15726470f78af0d918e99d6156e"
+dependencies = [
+ "event-listener",
+]
+
 [[package]]
 name = "async-net"
 version = "1.5.0"
@@ -135,6 +160,34 @@ dependencies = [
  "winapi",
 ]
 
+[[package]]
+name = "async-std"
+version = "1.9.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d9f06685bad74e0570f5213741bea82158279a4103d988e57bfada11ad230341"
+dependencies = [
+ "async-channel",
+ "async-global-executor",
+ "async-io",
+ "async-lock",
+ "async-process",
+ "crossbeam-utils 0.8.2",
+ "futures-channel",
+ "futures-core",
+ "futures-io",
+ "futures-lite",
+ "gloo-timers",
+ "kv-log-macro",
+ "log",
+ "memchr",
+ "num_cpus",
+ "once_cell",
+ "pin-project-lite",
+ "pin-utils",
+ "slab",
+ "wasm-bindgen-futures",
+]
+
 [[package]]
 name = "async-task"
 version = "4.0.3"
@@ -238,6 +291,12 @@ dependencies = [
  "memchr",
 ]
 
+[[package]]
+name = "bumpalo"
+version = "3.6.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "63396b8a4b9de3f4fdfb320ab6080762242f66a8ef174c49d8e19b674db4cdbe"
+
 [[package]]
 name = "byteorder"
 version = "1.4.2"
@@ -690,6 +749,15 @@ version = "0.1.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "a06f77d526c1a601b7c4cdd98f54b5eaabffc14d5f2f0296febdc7f357c6d3ba"
 
+[[package]]
+name = "futures-channel"
+version = "0.3.12"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f2d31b7ec7efab6eefc7c57233bb10b847986139d88cc2f5a02a1ae6871a1846"
+dependencies = [
+ "futures-core",
+]
+
 [[package]]
 name = "futures-core"
 version = "0.3.12"
@@ -770,11 +838,25 @@ dependencies = [
  "regex",
 ]
 
+[[package]]
+name = "gloo-timers"
+version = "0.2.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "47204a46aaff920a1ea58b11d03dec6f704287d27561724a4631e450654a891f"
+dependencies = [
+ "futures-channel",
+ "futures-core",
+ "js-sys",
+ "wasm-bindgen",
+ "web-sys",
+]
+
 [[package]]
 name = "gpui"
 version = "0.1.0"
 dependencies = [
  "anyhow",
+ "async-std",
  "async-task",
  "bindgen",
  "cc",
@@ -851,6 +933,24 @@ version = "0.4.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "dd25036021b0de88a0aff6b850051563c6516d0bf53f8638938edbb9de732736"
 
+[[package]]
+name = "js-sys"
+version = "0.3.50"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "2d99f9e3e84b8f67f846ef5b4cbbc3b1c29f6c759fcbce6f01aa0e73d932a24c"
+dependencies = [
+ "wasm-bindgen",
+]
+
+[[package]]
+name = "kv-log-macro"
+version = "1.0.7"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "0de8b303297635ad57c9f5059fd9cee7a47f8e8daa09df0fcd07dd39fb22977f"
+dependencies = [
+ "log",
+]
+
 [[package]]
 name = "lazy_static"
 version = "1.4.0"
@@ -895,6 +995,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "51b9bbe6c47d51fc3e1a9b945965946b4c44142ab8792c50835a980d362c2710"
 dependencies = [
  "cfg-if 1.0.0",
+ "value-bag",
 ]
 
 [[package]]
@@ -1106,6 +1207,12 @@ version = "0.2.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "439697af366c49a6d0a010c56a0d97685bc140ce0d377b13a2ea2aa42d64a827"
 
+[[package]]
+name = "pin-utils"
+version = "0.1.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184"
+
 [[package]]
 name = "pkg-config"
 version = "0.3.19"
@@ -1483,6 +1590,12 @@ dependencies = [
  "termcolor",
 ]
 
+[[package]]
+name = "slab"
+version = "0.4.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c111b5bd5695e56cffe5129854aa230b39c93a305372fdbb2668ca2394eea9f8"
+
 [[package]]
 name = "smallvec"
 version = "1.6.1"
@@ -1532,9 +1645,9 @@ checksum = "8fb1df15f412ee2e9dfc1c504260fa695c1c3f10fe9f4a6ee2d2184d7d6450e2"
 
 [[package]]
 name = "syn"
-version = "1.0.60"
+version = "1.0.67"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c700597eca8a5a762beb35753ef6b94df201c81cca676604f547495a0d7f0081"
+checksum = "6498a9efc342871f91cc2d0d694c674368b4ceb40f62b65a7a08c3792935e702"
 dependencies = [
  "proc-macro2",
  "quote",
@@ -1617,6 +1730,15 @@ version = "0.1.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "f14ee04d9415b52b3aeab06258a3f07093182b88ba0f9b8d203f211a7a7d41c7"
 
+[[package]]
+name = "value-bag"
+version = "1.0.0-alpha.6"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "6b676010e055c99033117c2343b33a40a30b91fecd6c49055ac9cd2d6c305ab1"
+dependencies = [
+ "ctor",
+]
+
 [[package]]
 name = "vec-arena"
 version = "1.0.0"
@@ -1664,6 +1786,82 @@ version = "0.10.0+wasi-snapshot-preview1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "1a143597ca7c7793eff794def352d41792a93c481eb1042423ff7ff72ba2c31f"
 
+[[package]]
+name = "wasm-bindgen"
+version = "0.2.73"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "83240549659d187488f91f33c0f8547cbfef0b2088bc470c116d1d260ef623d9"
+dependencies = [
+ "cfg-if 1.0.0",
+ "wasm-bindgen-macro",
+]
+
+[[package]]
+name = "wasm-bindgen-backend"
+version = "0.2.73"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ae70622411ca953215ca6d06d3ebeb1e915f0f6613e3b495122878d7ebec7dae"
+dependencies = [
+ "bumpalo",
+ "lazy_static",
+ "log",
+ "proc-macro2",
+ "quote",
+ "syn",
+ "wasm-bindgen-shared",
+]
+
+[[package]]
+name = "wasm-bindgen-futures"
+version = "0.4.23"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "81b8b767af23de6ac18bf2168b690bed2902743ddf0fb39252e36f9e2bfc63ea"
+dependencies = [
+ "cfg-if 1.0.0",
+ "js-sys",
+ "wasm-bindgen",
+ "web-sys",
+]
+
+[[package]]
+name = "wasm-bindgen-macro"
+version = "0.2.73"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3e734d91443f177bfdb41969de821e15c516931c3c3db3d318fa1b68975d0f6f"
+dependencies = [
+ "quote",
+ "wasm-bindgen-macro-support",
+]
+
+[[package]]
+name = "wasm-bindgen-macro-support"
+version = "0.2.73"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d53739ff08c8a68b0fdbcd54c372b8ab800b1449ab3c9d706503bc7dd1621b2c"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+ "wasm-bindgen-backend",
+ "wasm-bindgen-shared",
+]
+
+[[package]]
+name = "wasm-bindgen-shared"
+version = "0.2.73"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d9a543ae66aa233d14bb765ed9af4a33e81b8b58d1584cf1b47ff8cd0b9e4489"
+
+[[package]]
+name = "web-sys"
+version = "0.3.50"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a905d57e488fec8861446d3393670fb50d27a262344013181c2cdf9fff5481be"
+dependencies = [
+ "js-sys",
+ "wasm-bindgen",
+]
+
 [[package]]
 name = "wepoll-sys"
 version = "3.0.1"

gpui/Cargo.toml 🔗

@@ -5,6 +5,7 @@ name = "gpui"
 version = "0.1.0"
 
 [dependencies]
+async-std = {version = "1.9.0", features = ["unstable"]}
 async-task = "4.0.3"
 ctor = "0.1"
 etagere = "0.2"

gpui/src/app.rs 🔗

@@ -8,11 +8,12 @@ use crate::{
     AssetCache, AssetSource, FontCache, TextLayoutCache,
 };
 use anyhow::{anyhow, Result};
+use async_std::sync::Condvar;
 use keymap::MatchResult;
 use parking_lot::Mutex;
 use pathfinder_geometry::{rect::RectF, vector::vec2f};
 use platform::Event;
-use smol::{channel, prelude::*};
+use smol::prelude::*;
 use std::{
     any::{type_name, Any, TypeId},
     cell::RefCell,
@@ -313,7 +314,7 @@ pub struct MutableAppContext {
     background: Arc<executor::Background>,
     future_handlers: Rc<RefCell<HashMap<usize, FutureHandler>>>,
     stream_handlers: Rc<RefCell<HashMap<usize, StreamHandler>>>,
-    task_done: (channel::Sender<usize>, channel::Receiver<usize>),
+    task_done: Arc<Condvar>,
     pending_effects: VecDeque<Effect>,
     pending_flushes: usize,
     flushing_effects: bool,
@@ -350,7 +351,7 @@ impl MutableAppContext {
             background: Arc::new(executor::Background::new()),
             future_handlers: Default::default(),
             stream_handlers: Default::default(),
-            task_done: channel::unbounded(),
+            task_done: Default::default(),
             pending_effects: VecDeque::new(),
             pending_flushes: 0,
             flushing_effects: false,
@@ -983,8 +984,7 @@ impl MutableAppContext {
             task_id,
             task,
             TaskHandlerMap::Future(self.future_handlers.clone()),
-            self.task_done.0.clone(),
-            self.background.clone(),
+            self.task_done.clone(),
         )
     }
 
@@ -1022,8 +1022,7 @@ impl MutableAppContext {
             task_id,
             task,
             TaskHandlerMap::Stream(self.stream_handlers.clone()),
-            self.task_done.0.clone(),
-            self.background.clone(),
+            self.task_done.clone(),
         )
     }
 
@@ -1067,7 +1066,7 @@ impl MutableAppContext {
         };
 
         self.flush_effects();
-        self.task_done(task_id);
+        self.task_done.notify_all();
         result
     }
 
@@ -1166,19 +1165,10 @@ impl MutableAppContext {
         };
 
         self.flush_effects();
-        self.task_done(task_id);
+        self.task_done.notify_all();
         result
     }
 
-    fn task_done(&self, task_id: usize) {
-        let task_done = self.task_done.0.clone();
-        self.foreground
-            .spawn(async move {
-                let _ = task_done.send(task_id).await;
-            })
-            .detach()
-    }
-
     pub fn finish_pending_tasks(&self) -> impl Future<Output = ()> {
         let mut pending_tasks = self
             .future_handlers
@@ -1188,15 +1178,27 @@ impl MutableAppContext {
             .collect::<HashSet<_>>();
         pending_tasks.extend(self.stream_handlers.borrow().keys());
 
-        let task_done = self.task_done.1.clone();
+        let task_done = self.task_done.clone();
+        let future_handlers = self.future_handlers.clone();
+        let stream_handlers = self.stream_handlers.clone();
 
         async move {
-            while !pending_tasks.is_empty() {
-                if let Ok(task_id) = task_done.recv().await {
-                    pending_tasks.remove(&task_id);
-                } else {
-                    break;
+            // A Condvar expects the condition to be protected by a Mutex, but in this case we know
+            // that this logic will always run on the main thread.
+            let mutex = async_std::sync::Mutex::new(());
+            loop {
+                {
+                    let future_handlers = future_handlers.borrow();
+                    let stream_handlers = stream_handlers.borrow();
+                    pending_tasks.retain(|task_id| {
+                        future_handlers.contains_key(task_id)
+                            || stream_handlers.contains_key(task_id)
+                    });
+                    if pending_tasks.is_empty() {
+                        break;
+                    }
                 }
+                task_done.wait(mutex.lock().await).await;
             }
         }
     }
@@ -2357,8 +2359,7 @@ pub struct Task<T> {
     id: usize,
     task: Option<executor::Task<T>>,
     handler_map: TaskHandlerMap,
-    background: Arc<executor::Background>,
-    task_done: channel::Sender<usize>,
+    task_done: Arc<Condvar>,
 }
 
 enum TaskHandlerMap {
@@ -2372,15 +2373,13 @@ impl<T> Task<T> {
         id: usize,
         task: executor::Task<T>,
         handler_map: TaskHandlerMap,
-        task_done: channel::Sender<usize>,
-        background: Arc<executor::Background>,
+        task_done: Arc<Condvar>,
     ) -> Self {
         Self {
             id,
             task: Some(task),
             handler_map,
             task_done,
-            background,
         }
     }
 
@@ -2420,13 +2419,7 @@ impl<T> Drop for Task<T> {
                 map.borrow_mut().remove(&self.id);
             }
         }
-        let task_done = self.task_done.clone();
-        let task_id = self.id;
-        self.background
-            .spawn(async move {
-                let _ = task_done.send(task_id).await;
-            })
-            .detach()
+        self.task_done.notify_all();
     }
 }

gpui/src/presenter.rs 🔗

@@ -2,7 +2,7 @@ use crate::{
     app::{AppContext, MutableAppContext, WindowInvalidation},
     elements::Element,
     font_cache::FontCache,
-    platform::{self, Event},
+    platform::Event,
     text_layout::TextLayoutCache,
     AssetCache, ElementBox, Scene,
 };