From 170d27b04cff70c34d8ff4876a9cdd618ca452bd Mon Sep 17 00:00:00 2001 From: Isaac Clayton Date: Tue, 12 Jul 2022 16:32:41 +0200 Subject: [PATCH 01/10] Start working on plugin epoch async yield --- crates/plugin_runtime/build.rs | 2 +- crates/plugin_runtime/src/plugin.rs | 21 ++++++++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/crates/plugin_runtime/build.rs b/crates/plugin_runtime/build.rs index 542be4fb5f41be91ec045a598095eb78d7a65271..3371b0ab391b22f6c03a62546e8eed78d7e8cc8b 100644 --- a/crates/plugin_runtime/build.rs +++ b/crates/plugin_runtime/build.rs @@ -60,7 +60,7 @@ fn main() { fn create_default_engine() -> Engine { let mut config = Config::default(); config.async_support(true); - // config.epoch_interruption(true); + config.epoch_interruption(true); Engine::new(&config).expect("Could not create engine") } diff --git a/crates/plugin_runtime/src/plugin.rs b/crates/plugin_runtime/src/plugin.rs index d2570410f9437015037fe50d82f36b4198aec39d..6e0764d1a73a548ff7bec99b2c1a02efa677d3a2 100644 --- a/crates/plugin_runtime/src/plugin.rs +++ b/crates/plugin_runtime/src/plugin.rs @@ -71,7 +71,7 @@ pub struct PluginBuilder { pub fn create_default_engine() -> Result { let mut config = Config::default(); config.async_support(true); - // config.epoch_interruption(true); + config.epoch_interruption(true); Engine::new(&config) } @@ -303,11 +303,12 @@ impl Plugin { println!(); } - async fn init( + async fn init + Send + 'static>( precompiled: bool, module: Vec, plugin: PluginBuilder, - ) -> Result { + spawn_incrementer: impl Fn(F) -> T, + ) -> Result<(Self, T), Error> { // initialize the WebAssembly System Interface context let engine = plugin.engine; let mut linker = plugin.linker; @@ -322,7 +323,8 @@ impl Plugin { alloc: None, }, ); - // store.epoch_deadline_async_yield_and_update(todo!()); + store.epoch_deadline_async_yield_and_update(1); + let module = if precompiled { unsafe { Module::deserialize(&engine, module)? } } else { @@ -342,7 +344,16 @@ impl Plugin { free_buffer, }); - Ok(Plugin { store, instance }) + let plugin = Plugin { store, instance }; + let incrementer = spawn_incrementer(async move { + loop { + smol::Timer::after(std::time::Duration::from_millis(100)).await; + + engine.increment_epoch(); + } + }); + + Ok((plugin, incrementer)) } /// Attaches a file or directory the the given system path to the runtime. From 7f11a3236412e9e44cfa5c34df6ffeda0d29ba69 Mon Sep 17 00:00:00 2001 From: Isaac Clayton Date: Tue, 12 Jul 2022 17:07:33 +0200 Subject: [PATCH 02/10] Implement periodic yielding using epoch_deadline_async_yield_and_update --- crates/plugin_runtime/src/lib.rs | 4 +- crates/plugin_runtime/src/plugin.rs | 46 +++++++++++++++------ crates/zed/src/languages/language_plugin.rs | 4 +- 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/crates/plugin_runtime/src/lib.rs b/crates/plugin_runtime/src/lib.rs index 555d92587459e7ad76fedbaa76c09e59c8d8c612..c4c145d801c9519dcf6773d7d8f3fd695b08f3b7 100644 --- a/crates/plugin_runtime/src/lib.rs +++ b/crates/plugin_runtime/src/lib.rs @@ -23,7 +23,7 @@ mod tests { } async { - let mut runtime = PluginBuilder::new_with_default_ctx() + let (mut runtime, incrementer) = PluginBuilder::new_with_default_ctx() .unwrap() .host_function("mystery_number", |input: u32| input + 7) .unwrap() @@ -53,6 +53,8 @@ mod tests { .await .unwrap(); + std::thread::spawn(move || incrementer.block_on()); + let plugin = TestPlugin { noop: runtime.function("noop").unwrap(), constant: runtime.function("constant").unwrap(), diff --git a/crates/plugin_runtime/src/plugin.rs b/crates/plugin_runtime/src/plugin.rs index 6e0764d1a73a548ff7bec99b2c1a02efa677d3a2..dbc6a539df41292072b1bb4d5e6f779cd9ae5107 100644 --- a/crates/plugin_runtime/src/plugin.rs +++ b/crates/plugin_runtime/src/plugin.rs @@ -62,6 +62,8 @@ pub struct PluginBuilder { wasi_ctx: WasiCtx, engine: Engine, linker: Linker, + delta: u64, + epoch: std::time::Duration, } /// Creates a default engine for compiling Wasm. @@ -83,10 +85,11 @@ impl PluginBuilder { let linker = Linker::new(&engine); Ok(PluginBuilder { - // host_functions: HashMap::new(), wasi_ctx, engine, linker, + delta: 1, + epoch: std::time::Duration::from_millis(100), }) } @@ -242,7 +245,17 @@ impl PluginBuilder { /// Initializes a [`Plugin`] from a given compiled Wasm module. /// Both binary (`.wasm`) and text (`.wat`) module formats are supported. - pub async fn init>(self, precompiled: bool, module: T) -> Result { + pub async fn init>( + self, + precompiled: bool, + module: T, + ) -> Result< + ( + Plugin, + std::pin::Pin + Send + 'static>>, + ), + Error, + > { Plugin::init(precompiled, module.as_ref().to_vec(), self).await } } @@ -303,12 +316,17 @@ impl Plugin { println!(); } - async fn init + Send + 'static>( + async fn init( precompiled: bool, module: Vec, plugin: PluginBuilder, - spawn_incrementer: impl Fn(F) -> T, - ) -> Result<(Self, T), Error> { + ) -> Result< + ( + Self, + std::pin::Pin + Send + 'static>>, + ), + Error, + > { // initialize the WebAssembly System Interface context let engine = plugin.engine; let mut linker = plugin.linker; @@ -323,7 +341,6 @@ impl Plugin { alloc: None, }, ); - store.epoch_deadline_async_yield_and_update(1); let module = if precompiled { unsafe { Module::deserialize(&engine, module)? } @@ -331,6 +348,16 @@ impl Plugin { Module::new(&engine, module)? }; + // set up automatic yielding after given duration + store.epoch_deadline_async_yield_and_update(plugin.delta); + let epoch = plugin.epoch; + let incrementer = Box::pin(async move { + loop { + smol::Timer::after(epoch).await; + engine.increment_epoch(); + } + }); + // load the provided module into the asynchronous runtime linker.module_async(&mut store, "", &module).await?; let instance = linker.instantiate_async(&mut store, &module).await?; @@ -345,13 +372,6 @@ impl Plugin { }); let plugin = Plugin { store, instance }; - let incrementer = spawn_incrementer(async move { - loop { - smol::Timer::after(std::time::Duration::from_millis(100)).await; - - engine.increment_epoch(); - } - }); Ok((plugin, incrementer)) } diff --git a/crates/zed/src/languages/language_plugin.rs b/crates/zed/src/languages/language_plugin.rs index ac649c425d6687b023a1a4bfdac12617a3ef930f..cf812fd2ad0dc164b15062f5d61e0479485e0f22 100644 --- a/crates/zed/src/languages/language_plugin.rs +++ b/crates/zed/src/languages/language_plugin.rs @@ -9,7 +9,7 @@ use std::{any::Any, path::PathBuf, sync::Arc}; use util::ResultExt; pub async fn new_json(executor: Arc) -> Result { - let plugin = PluginBuilder::new_with_default_ctx()? + let (plugin, incrementer) = PluginBuilder::new_with_default_ctx()? .host_function_async("command", |command: String| async move { let mut args = command.split(' '); let command = args.next().unwrap(); @@ -25,6 +25,8 @@ pub async fn new_json(executor: Arc) -> Result { include_bytes!("../../../../plugins/bin/json_language.wasm.pre"), ) .await?; + + executor.spawn(incrementer).detach(); PluginLspAdapter::new(plugin, executor).await } From d04c3388b4d70367f0dfe9b63edb74a651d6accf Mon Sep 17 00:00:00 2001 From: Isaac Clayton Date: Wed, 13 Jul 2022 11:09:06 +0200 Subject: [PATCH 03/10] Switch from epoch to fuel --- crates/plugin_runtime/build.rs | 2 +- crates/plugin_runtime/src/lib.rs | 4 +- crates/plugin_runtime/src/plugin.rs | 44 +++++---------------- crates/zed/src/languages/language_plugin.rs | 3 +- 4 files changed, 12 insertions(+), 41 deletions(-) diff --git a/crates/plugin_runtime/build.rs b/crates/plugin_runtime/build.rs index 3371b0ab391b22f6c03a62546e8eed78d7e8cc8b..0bae17d7cda4c37b1ca7dd952d920399af408dbf 100644 --- a/crates/plugin_runtime/build.rs +++ b/crates/plugin_runtime/build.rs @@ -60,7 +60,7 @@ fn main() { fn create_default_engine() -> Engine { let mut config = Config::default(); config.async_support(true); - config.epoch_interruption(true); + config.consume_fuel(true); Engine::new(&config).expect("Could not create engine") } diff --git a/crates/plugin_runtime/src/lib.rs b/crates/plugin_runtime/src/lib.rs index c4c145d801c9519dcf6773d7d8f3fd695b08f3b7..555d92587459e7ad76fedbaa76c09e59c8d8c612 100644 --- a/crates/plugin_runtime/src/lib.rs +++ b/crates/plugin_runtime/src/lib.rs @@ -23,7 +23,7 @@ mod tests { } async { - let (mut runtime, incrementer) = PluginBuilder::new_with_default_ctx() + let mut runtime = PluginBuilder::new_with_default_ctx() .unwrap() .host_function("mystery_number", |input: u32| input + 7) .unwrap() @@ -53,8 +53,6 @@ mod tests { .await .unwrap(); - std::thread::spawn(move || incrementer.block_on()); - let plugin = TestPlugin { noop: runtime.function("noop").unwrap(), constant: runtime.function("constant").unwrap(), diff --git a/crates/plugin_runtime/src/plugin.rs b/crates/plugin_runtime/src/plugin.rs index dbc6a539df41292072b1bb4d5e6f779cd9ae5107..1db81375bf401e0e30addfa304d202aa878a0a37 100644 --- a/crates/plugin_runtime/src/plugin.rs +++ b/crates/plugin_runtime/src/plugin.rs @@ -62,8 +62,7 @@ pub struct PluginBuilder { wasi_ctx: WasiCtx, engine: Engine, linker: Linker, - delta: u64, - epoch: std::time::Duration, + fuel_refill: u64, } /// Creates a default engine for compiling Wasm. @@ -73,7 +72,7 @@ pub struct PluginBuilder { pub fn create_default_engine() -> Result { let mut config = Config::default(); config.async_support(true); - config.epoch_interruption(true); + config.consume_fuel(true); Engine::new(&config) } @@ -88,8 +87,7 @@ impl PluginBuilder { wasi_ctx, engine, linker, - delta: 1, - epoch: std::time::Duration::from_millis(100), + fuel_refill: 1000, }) } @@ -245,17 +243,7 @@ impl PluginBuilder { /// Initializes a [`Plugin`] from a given compiled Wasm module. /// Both binary (`.wasm`) and text (`.wat`) module formats are supported. - pub async fn init>( - self, - precompiled: bool, - module: T, - ) -> Result< - ( - Plugin, - std::pin::Pin + Send + 'static>>, - ), - Error, - > { + pub async fn init>(self, precompiled: bool, module: T) -> Result { Plugin::init(precompiled, module.as_ref().to_vec(), self).await } } @@ -320,13 +308,7 @@ impl Plugin { precompiled: bool, module: Vec, plugin: PluginBuilder, - ) -> Result< - ( - Self, - std::pin::Pin + Send + 'static>>, - ), - Error, - > { + ) -> Result { // initialize the WebAssembly System Interface context let engine = plugin.engine; let mut linker = plugin.linker; @@ -348,15 +330,9 @@ impl Plugin { Module::new(&engine, module)? }; - // set up automatic yielding after given duration - store.epoch_deadline_async_yield_and_update(plugin.delta); - let epoch = plugin.epoch; - let incrementer = Box::pin(async move { - loop { - smol::Timer::after(epoch).await; - engine.increment_epoch(); - } - }); + // set up automatic yielding after fuel expires + store.out_of_fuel_async_yield(u64::MAX, plugin.fuel_refill); + store.add_fuel(plugin.fuel_refill).unwrap(); // load the provided module into the asynchronous runtime linker.module_async(&mut store, "", &module).await?; @@ -371,9 +347,7 @@ impl Plugin { free_buffer, }); - let plugin = Plugin { store, instance }; - - Ok((plugin, incrementer)) + Ok(Plugin { store, instance }) } /// Attaches a file or directory the the given system path to the runtime. diff --git a/crates/zed/src/languages/language_plugin.rs b/crates/zed/src/languages/language_plugin.rs index cf812fd2ad0dc164b15062f5d61e0479485e0f22..f5bfa661b3b9e3c12bc3d326d41bc3215cc6ec7b 100644 --- a/crates/zed/src/languages/language_plugin.rs +++ b/crates/zed/src/languages/language_plugin.rs @@ -9,7 +9,7 @@ use std::{any::Any, path::PathBuf, sync::Arc}; use util::ResultExt; pub async fn new_json(executor: Arc) -> Result { - let (plugin, incrementer) = PluginBuilder::new_with_default_ctx()? + let plugin = PluginBuilder::new_with_default_ctx()? .host_function_async("command", |command: String| async move { let mut args = command.split(' '); let command = args.next().unwrap(); @@ -26,7 +26,6 @@ pub async fn new_json(executor: Arc) -> Result { ) .await?; - executor.spawn(incrementer).detach(); PluginLspAdapter::new(plugin, executor).await } From 10670dba70638c5b347513aa73c5631f9ba0df65 Mon Sep 17 00:00:00 2001 From: Isaac Clayton Date: Wed, 13 Jul 2022 11:57:00 +0200 Subject: [PATCH 04/10] Add support for configuring plugin yield mechanism, stashing --- crates/plugin_runtime/build.rs | 4 - crates/plugin_runtime/src/lib.rs | 2 +- crates/plugin_runtime/src/plugin.rs | 105 ++++++++++++++++---- crates/zed/src/languages/language_plugin.rs | 4 +- 4 files changed, 88 insertions(+), 27 deletions(-) diff --git a/crates/plugin_runtime/build.rs b/crates/plugin_runtime/build.rs index 0bae17d7cda4c37b1ca7dd952d920399af408dbf..75a8d1d78f1b7db39199a17f0058059911b3950d 100644 --- a/crates/plugin_runtime/build.rs +++ b/crates/plugin_runtime/build.rs @@ -54,13 +54,9 @@ fn main() { } /// Creates a default engine for compiling Wasm. -/// N.B.: this must create the same `Engine` as -/// the `create_default_engine` function -/// in `plugin_runtime/src/plugin.rs`. fn create_default_engine() -> Engine { let mut config = Config::default(); config.async_support(true); - config.consume_fuel(true); Engine::new(&config).expect("Could not create engine") } diff --git a/crates/plugin_runtime/src/lib.rs b/crates/plugin_runtime/src/lib.rs index 555d92587459e7ad76fedbaa76c09e59c8d8c612..75a195011470f71b414a821b02be256a4f2bf5d7 100644 --- a/crates/plugin_runtime/src/lib.rs +++ b/crates/plugin_runtime/src/lib.rs @@ -23,7 +23,7 @@ mod tests { } async { - let mut runtime = PluginBuilder::new_with_default_ctx() + let mut runtime = PluginBuilder::new_with_default_ctx(PluginYield::default_fuel()) .unwrap() .host_function("mystery_number", |input: u32| input + 7) .unwrap() diff --git a/crates/plugin_runtime/src/plugin.rs b/crates/plugin_runtime/src/plugin.rs index 1db81375bf401e0e30addfa304d202aa878a0a37..9436a11bc63df07438774435e7371d8d6e477254 100644 --- a/crates/plugin_runtime/src/plugin.rs +++ b/crates/plugin_runtime/src/plugin.rs @@ -1,5 +1,6 @@ use std::future::Future; +use std::time::Duration; use std::{fs::File, marker::PhantomData, path::Path}; use anyhow::{anyhow, Error}; @@ -54,6 +55,33 @@ impl Clone for WasiFn { } } +pub enum PluginYield { + Epoch { + delta: u64, + epoch: std::time::Duration, + }, + Fuel { + initial: u64, + refill: u64, + }, +} + +impl PluginYield { + pub fn default_epoch() -> Self { + PluginYield::Epoch { + delta: 1, + epoch: Duration::from_millis(1), + } + } + + pub fn default_fuel() -> Self { + PluginYield::Fuel { + initial: 1000, + refill: 1000, + } + } +} + /// This struct is used to build a new [`Plugin`], using the builder pattern. /// Create a new default plugin with `PluginBuilder::new_with_default_ctx`, /// and add host-side exported functions using `host_function` and `host_function_async`. @@ -62,43 +90,66 @@ pub struct PluginBuilder { wasi_ctx: WasiCtx, engine: Engine, linker: Linker, - fuel_refill: u64, -} - -/// Creates a default engine for compiling Wasm. -/// N.B.: this must create the same `Engine` as -/// the `create_default_engine` function -/// in `plugin_runtime/build.rs`. -pub fn create_default_engine() -> Result { - let mut config = Config::default(); - config.async_support(true); - config.consume_fuel(true); - Engine::new(&config) + yield_when: PluginYield, + created_epoch_incrementer: bool, } impl PluginBuilder { /// Create a new [`PluginBuilder`] with the given WASI context. /// Using the default context is a safe bet, see [`new_with_default_context`]. - pub fn new(wasi_ctx: WasiCtx) -> Result { - let engine = create_default_engine()?; + pub fn new(wasi_ctx: WasiCtx, yield_when: PluginYield) -> Result { + let mut config = Config::default(); + config.async_support(true); + + match yield_when { + PluginYield::Epoch { .. } => { + config.epoch_interruption(true); + } + PluginYield::Fuel { .. } => { + config.consume_fuel(true); + } + } + + let engine = Engine::new(&config)?; let linker = Linker::new(&engine); Ok(PluginBuilder { wasi_ctx, engine, linker, - fuel_refill: 1000, + yield_when, + created_epoch_incrementer: false, }) } /// Create a new `PluginBuilder` that inherits the /// host processes' access to `stdout` and `stderr`. - pub fn new_with_default_ctx() -> Result { + pub fn new_with_default_ctx(yield_when: PluginYield) -> Result { let wasi_ctx = WasiCtxBuilder::new() .inherit_stdout() .inherit_stderr() .build(); - Self::new(wasi_ctx) + Self::new(wasi_ctx, yield_when) + } + + /// Creates a epoch incrementer if this plugin is configured to increment epochs. + /// Will panic if this plugin is not configured to increment epochs. + pub fn create_epoch_incrementer( + &mut self, + ) -> std::pin::Pin + Send + 'static>> { + if let PluginYield::Epoch { epoch, .. } = self.yield_when { + self.created_epoch_incrementer = true; + let epoch = epoch.clone(); + let engine = &self.engine; + Box::pin(async move { + loop { + smol::Timer::after(epoch); + engine.increment_epoch(); + } + }) + } else { + panic!("Tried creating an epoch incrementer, but one does not yet exist.") + } } /// Add an `async` host function. See [`host_function`] for details. @@ -243,7 +294,14 @@ impl PluginBuilder { /// Initializes a [`Plugin`] from a given compiled Wasm module. /// Both binary (`.wasm`) and text (`.wat`) module formats are supported. + /// Will panic if this is plugin uses `PluginYield::Epoch`, + /// but an epoch incrementer has not yet been created. pub async fn init>(self, precompiled: bool, module: T) -> Result { + if let PluginYield::Epoch { .. } = self.yield_when { + if !self.created_epoch_incrementer { + panic!("Must create epoch incrementer to run epoch-based plugin"); + } + } Plugin::init(precompiled, module.as_ref().to_vec(), self).await } } @@ -330,9 +388,16 @@ impl Plugin { Module::new(&engine, module)? }; - // set up automatic yielding after fuel expires - store.out_of_fuel_async_yield(u64::MAX, plugin.fuel_refill); - store.add_fuel(plugin.fuel_refill).unwrap(); + // set up automatic yielding based on configuration + match plugin.yield_when { + PluginYield::Epoch { delta, .. } => { + store.epoch_deadline_async_yield_and_update(delta); + } + PluginYield::Fuel { initial, refill } => { + store.add_fuel(initial).unwrap(); + store.out_of_fuel_async_yield(u64::MAX, refill); + } + } // load the provided module into the asynchronous runtime linker.module_async(&mut store, "", &module).await?; diff --git a/crates/zed/src/languages/language_plugin.rs b/crates/zed/src/languages/language_plugin.rs index f5bfa661b3b9e3c12bc3d326d41bc3215cc6ec7b..ab0e726b511de240fae9e962302c2a7f4ce71892 100644 --- a/crates/zed/src/languages/language_plugin.rs +++ b/crates/zed/src/languages/language_plugin.rs @@ -4,12 +4,12 @@ use client::http::HttpClient; use futures::lock::Mutex; use gpui::executor::Background; use language::{LanguageServerName, LspAdapter}; -use plugin_runtime::{Plugin, PluginBuilder, WasiFn}; +use plugin_runtime::{Plugin, PluginBuilder, PluginYield, WasiFn}; use std::{any::Any, path::PathBuf, sync::Arc}; use util::ResultExt; pub async fn new_json(executor: Arc) -> Result { - let plugin = PluginBuilder::new_with_default_ctx()? + let plugin = PluginBuilder::new_with_default_ctx(PluginYield::default_epoch())? .host_function_async("command", |command: String| async move { let mut args = command.split(' '); let command = args.next().unwrap(); From 8974b0c490b4c8c4c3f49efba1b2adc715a38dfd Mon Sep 17 00:00:00 2001 From: Isaac Clayton Date: Wed, 13 Jul 2022 12:41:47 +0200 Subject: [PATCH 05/10] Work on supporting both epoch and fuel --- crates/plugin_runtime/src/lib.rs | 2 +- crates/plugin_runtime/src/plugin.rs | 142 +++++++++++++------- crates/zed/src/languages/language_plugin.rs | 5 +- 3 files changed, 96 insertions(+), 53 deletions(-) diff --git a/crates/plugin_runtime/src/lib.rs b/crates/plugin_runtime/src/lib.rs index 75a195011470f71b414a821b02be256a4f2bf5d7..b59f03a55f9b5c35f3bdc78dcfe8d2ad1c6495d0 100644 --- a/crates/plugin_runtime/src/lib.rs +++ b/crates/plugin_runtime/src/lib.rs @@ -23,7 +23,7 @@ mod tests { } async { - let mut runtime = PluginBuilder::new_with_default_ctx(PluginYield::default_fuel()) + let mut runtime = PluginBuilder::new_fuel_with_default_ctx(PluginYield::default_fuel()) .unwrap() .host_function("mystery_number", |input: u32| input + 7) .unwrap() diff --git a/crates/plugin_runtime/src/plugin.rs b/crates/plugin_runtime/src/plugin.rs index 9436a11bc63df07438774435e7371d8d6e477254..777d40c26571b3eabb7582c9b61855f848b5aa4d 100644 --- a/crates/plugin_runtime/src/plugin.rs +++ b/crates/plugin_runtime/src/plugin.rs @@ -55,27 +55,43 @@ impl Clone for WasiFn { } } +pub struct PluginYieldEpoch { + delta: u64, + epoch: std::time::Duration, +} + +impl Into for PluginYieldEpoch { + fn into(self) -> PluginYield { + PluginYield::Epoch(self) + } +} + +pub struct PluginYieldFuel { + initial: u64, + refill: u64, +} + +impl Into for PluginYieldFuel { + fn into(self) -> PluginYield { + PluginYield::Fuel(self) + } +} + pub enum PluginYield { - Epoch { - delta: u64, - epoch: std::time::Duration, - }, - Fuel { - initial: u64, - refill: u64, - }, + Epoch(PluginYieldEpoch), + Fuel(PluginYieldFuel), } impl PluginYield { - pub fn default_epoch() -> Self { - PluginYield::Epoch { + pub fn default_epoch() -> PluginYieldEpoch { + PluginYieldEpoch { delta: 1, epoch: Duration::from_millis(1), } } - pub fn default_fuel() -> Self { - PluginYield::Fuel { + pub fn default_fuel() -> PluginYieldFuel { + PluginYieldFuel { initial: 1000, refill: 1000, } @@ -91,65 +107,94 @@ pub struct PluginBuilder { engine: Engine, linker: Linker, yield_when: PluginYield, - created_epoch_incrementer: bool, } impl PluginBuilder { - /// Create a new [`PluginBuilder`] with the given WASI context. - /// Using the default context is a safe bet, see [`new_with_default_context`]. - pub fn new(wasi_ctx: WasiCtx, yield_when: PluginYield) -> Result { + fn create_engine(yield_when: &PluginYield) -> Result<(Engine, Linker), Error> { let mut config = Config::default(); config.async_support(true); + let engine = Engine::new(&config)?; + let linker = Linker::new(&engine); match yield_when { - PluginYield::Epoch { .. } => { + PluginYield::Epoch(_) => { config.epoch_interruption(true); } - PluginYield::Fuel { .. } => { + PluginYield::Fuel(_) => { config.consume_fuel(true); } } - let engine = Engine::new(&config)?; - let linker = Linker::new(&engine); + Ok((engine, linker)) + } + + /// Create a new [`PluginBuilder`] with the given WASI context. + /// Using the default context is a safe bet, see [`new_with_default_context`]. + pub fn new_epoch( + wasi_ctx: WasiCtx, + yield_epoch: PluginYieldEpoch, + callback: C, + ) -> Result + where + C: FnOnce(std::pin::Pin + Send + 'static>>) -> (), + { + let epoch = yield_epoch.epoch; + let yield_when = PluginYield::Epoch(yield_epoch); + let (engine, linker) = Self::create_engine(&yield_when)?; + + let engine_ref = &engine; + + // we can't create the future until after initializing + // because we need the engine to load the plugin + // we could use an Arc, but that'd suck + callback(Box::pin(async move { + loop { + smol::Timer::after(epoch).await; + engine_ref.increment_epoch(); + } + })); + + Ok(PluginBuilder { + wasi_ctx, + engine, + linker, + yield_when, + }) + } + + pub fn new_fuel(wasi_ctx: WasiCtx, yield_fuel: PluginYieldFuel) -> Result { + let yield_when = PluginYield::Fuel(yield_fuel); + let (engine, linker) = Self::create_engine(&yield_when)?; Ok(PluginBuilder { wasi_ctx, engine, linker, yield_when, - created_epoch_incrementer: false, }) } - /// Create a new `PluginBuilder` that inherits the + /// Create a new `WasiCtx` that inherits the /// host processes' access to `stdout` and `stderr`. - pub fn new_with_default_ctx(yield_when: PluginYield) -> Result { - let wasi_ctx = WasiCtxBuilder::new() + fn default_ctx() -> WasiCtx { + WasiCtxBuilder::new() .inherit_stdout() .inherit_stderr() - .build(); - Self::new(wasi_ctx, yield_when) + .build() } - /// Creates a epoch incrementer if this plugin is configured to increment epochs. - /// Will panic if this plugin is not configured to increment epochs. - pub fn create_epoch_incrementer( - &mut self, - ) -> std::pin::Pin + Send + 'static>> { - if let PluginYield::Epoch { epoch, .. } = self.yield_when { - self.created_epoch_incrementer = true; - let epoch = epoch.clone(); - let engine = &self.engine; - Box::pin(async move { - loop { - smol::Timer::after(epoch); - engine.increment_epoch(); - } - }) - } else { - panic!("Tried creating an epoch incrementer, but one does not yet exist.") - } + pub fn new_epoch_with_default_ctx( + yield_epoch: PluginYieldEpoch, + callback: C, + ) -> Result + where + C: FnOnce(std::pin::Pin + Send + 'static>>) -> (), + { + Self::new_epoch(Self::default_ctx(), yield_epoch, callback) + } + + pub fn new_fuel_with_default_ctx(yield_fuel: PluginYieldFuel) -> Result { + Self::new_fuel(Self::default_ctx(), yield_fuel) } /// Add an `async` host function. See [`host_function`] for details. @@ -297,11 +342,6 @@ impl PluginBuilder { /// Will panic if this is plugin uses `PluginYield::Epoch`, /// but an epoch incrementer has not yet been created. pub async fn init>(self, precompiled: bool, module: T) -> Result { - if let PluginYield::Epoch { .. } = self.yield_when { - if !self.created_epoch_incrementer { - panic!("Must create epoch incrementer to run epoch-based plugin"); - } - } Plugin::init(precompiled, module.as_ref().to_vec(), self).await } } @@ -390,10 +430,10 @@ impl Plugin { // set up automatic yielding based on configuration match plugin.yield_when { - PluginYield::Epoch { delta, .. } => { + PluginYield::Epoch(PluginYieldEpoch { delta, .. }) => { store.epoch_deadline_async_yield_and_update(delta); } - PluginYield::Fuel { initial, refill } => { + PluginYield::Fuel(PluginYieldFuel { initial, refill }) => { store.add_fuel(initial).unwrap(); store.out_of_fuel_async_yield(u64::MAX, refill); } diff --git a/crates/zed/src/languages/language_plugin.rs b/crates/zed/src/languages/language_plugin.rs index ab0e726b511de240fae9e962302c2a7f4ce71892..78242865ab4b1d77725b10a41992e8b4b7e99d49 100644 --- a/crates/zed/src/languages/language_plugin.rs +++ b/crates/zed/src/languages/language_plugin.rs @@ -9,7 +9,10 @@ use std::{any::Any, path::PathBuf, sync::Arc}; use util::ResultExt; pub async fn new_json(executor: Arc) -> Result { - let plugin = PluginBuilder::new_with_default_ctx(PluginYield::default_epoch())? + let plugin = + PluginBuilder::new_epoch_with_default_ctx(PluginYield::default_epoch(), |future| { + executor.spawn(future).detach() + })? .host_function_async("command", |command: String| async move { let mut args = command.split(' '); let command = args.next().unwrap(); From 8b376dd6139f8183d325328d7f6d4d56a2a7535c Mon Sep 17 00:00:00 2001 From: Isaac Clayton Date: Wed, 13 Jul 2022 13:15:12 +0200 Subject: [PATCH 06/10] Fix resulting errors and introduce functional executor callback --- crates/plugin_runtime/src/plugin.rs | 88 ++++++++++++--------- crates/zed/src/languages/language_plugin.rs | 5 +- 2 files changed, 55 insertions(+), 38 deletions(-) diff --git a/crates/plugin_runtime/src/plugin.rs b/crates/plugin_runtime/src/plugin.rs index 777d40c26571b3eabb7582c9b61855f848b5aa4d..3515bfd8e70432e9274c907a2d6b9acbf52ed8f8 100644 --- a/crates/plugin_runtime/src/plugin.rs +++ b/crates/plugin_runtime/src/plugin.rs @@ -60,25 +60,16 @@ pub struct PluginYieldEpoch { epoch: std::time::Duration, } -impl Into for PluginYieldEpoch { - fn into(self) -> PluginYield { - PluginYield::Epoch(self) - } -} - pub struct PluginYieldFuel { initial: u64, refill: u64, } -impl Into for PluginYieldFuel { - fn into(self) -> PluginYield { - PluginYield::Fuel(self) - } -} - pub enum PluginYield { - Epoch(PluginYieldEpoch), + Epoch { + yield_epoch: PluginYieldEpoch, + initialize_incrementer: Box () + Send>, + }, Fuel(PluginYieldFuel), } @@ -117,7 +108,7 @@ impl PluginBuilder { let linker = Linker::new(&engine); match yield_when { - PluginYield::Epoch(_) => { + PluginYield::Epoch { .. } => { config.epoch_interruption(true); } PluginYield::Fuel(_) => { @@ -136,23 +127,27 @@ impl PluginBuilder { callback: C, ) -> Result where - C: FnOnce(std::pin::Pin + Send + 'static>>) -> (), + C: FnOnce(std::pin::Pin + Send + 'static>>) -> () + + Send + + 'static, { - let epoch = yield_epoch.epoch; - let yield_when = PluginYield::Epoch(yield_epoch); - let (engine, linker) = Self::create_engine(&yield_when)?; - - let engine_ref = &engine; - // we can't create the future until after initializing // because we need the engine to load the plugin - // we could use an Arc, but that'd suck - callback(Box::pin(async move { - loop { - smol::Timer::after(epoch).await; - engine_ref.increment_epoch(); - } - })); + let epoch = yield_epoch.epoch; + let initialize_incrementer = Box::new(move |engine: Engine| { + callback(Box::pin(async move { + loop { + smol::Timer::after(epoch).await; + engine.increment_epoch(); + } + })) + }); + + let yield_when = PluginYield::Epoch { + yield_epoch, + initialize_incrementer, + }; + let (engine, linker) = Self::create_engine(&yield_when)?; Ok(PluginBuilder { wasi_ctx, @@ -188,7 +183,9 @@ impl PluginBuilder { callback: C, ) -> Result where - C: FnOnce(std::pin::Pin + Send + 'static>>) -> (), + C: FnOnce(std::pin::Pin + Send + 'static>>) -> () + + Send + + 'static, { Self::new_epoch(Self::default_ctx(), yield_epoch, callback) } @@ -342,7 +339,7 @@ impl PluginBuilder { /// Will panic if this is plugin uses `PluginYield::Epoch`, /// but an epoch incrementer has not yet been created. pub async fn init>(self, precompiled: bool, module: T) -> Result { - Plugin::init(precompiled, module.as_ref().to_vec(), self).await + Plugin::init(precompiled, module.as_ref(), self).await } } @@ -402,11 +399,7 @@ impl Plugin { println!(); } - async fn init( - precompiled: bool, - module: Vec, - plugin: PluginBuilder, - ) -> Result { + async fn init(precompiled: bool, module: &[u8], plugin: PluginBuilder) -> Result { // initialize the WebAssembly System Interface context let engine = plugin.engine; let mut linker = plugin.linker; @@ -430,8 +423,12 @@ impl Plugin { // set up automatic yielding based on configuration match plugin.yield_when { - PluginYield::Epoch(PluginYieldEpoch { delta, .. }) => { + PluginYield::Epoch { + yield_epoch: PluginYieldEpoch { delta, .. }, + initialize_incrementer, + } => { store.epoch_deadline_async_yield_and_update(delta); + initialize_incrementer(engine); } PluginYield::Fuel(PluginYieldFuel { initial, refill }) => { store.add_fuel(initial).unwrap(); @@ -455,6 +452,25 @@ impl Plugin { Ok(Plugin { store, instance }) } + // async fn init_fuel( + // precompiled: bool, + // module: &[u8], + // plugin: PluginBuilder, + // ) -> Result { + // let (_, plugin) = Self::init_inner(precompiled, module, plugin).await?; + // Ok(plugin) + // } + + // async fn init_epoch( + // precompiled: bool, + // module: &[u8], + // plugin: PluginBuilder, + // callback: C, + // ) -> Result { + // let (_, plugin) = Self::init_inner(precompiled, module, plugin).await?; + // Ok(plugin) + // } + /// Attaches a file or directory the the given system path to the runtime. /// Note that the resource must be freed by calling `remove_resource` afterwards. pub fn attach_path>(&mut self, path: T) -> Result { diff --git a/crates/zed/src/languages/language_plugin.rs b/crates/zed/src/languages/language_plugin.rs index 78242865ab4b1d77725b10a41992e8b4b7e99d49..8417f0ff9a6f699878309f406a8ef875f3f76317 100644 --- a/crates/zed/src/languages/language_plugin.rs +++ b/crates/zed/src/languages/language_plugin.rs @@ -9,9 +9,10 @@ use std::{any::Any, path::PathBuf, sync::Arc}; use util::ResultExt; pub async fn new_json(executor: Arc) -> Result { + let executor_ref = executor.clone(); let plugin = - PluginBuilder::new_epoch_with_default_ctx(PluginYield::default_epoch(), |future| { - executor.spawn(future).detach() + PluginBuilder::new_epoch_with_default_ctx(PluginYield::default_epoch(), move |future| { + executor_ref.spawn(future).detach() })? .host_function_async("command", |command: String| async move { let mut args = command.split(' '); From b3e1fd0740dc54168d3ee80ad5cf3615450ac903 Mon Sep 17 00:00:00 2001 From: Isaac Clayton Date: Wed, 13 Jul 2022 13:19:51 +0200 Subject: [PATCH 07/10] Rename a few items and add documentation --- crates/plugin_runtime/src/plugin.rs | 36 +++++++++++------------------ 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/crates/plugin_runtime/src/plugin.rs b/crates/plugin_runtime/src/plugin.rs index 3515bfd8e70432e9274c907a2d6b9acbf52ed8f8..410185078f8de77fc961916fecdd36277dcf49e6 100644 --- a/crates/plugin_runtime/src/plugin.rs +++ b/crates/plugin_runtime/src/plugin.rs @@ -101,6 +101,7 @@ pub struct PluginBuilder { } impl PluginBuilder { + /// Creates an engine with the proper configuration given the yield mechanism in use fn create_engine(yield_when: &PluginYield) -> Result<(Engine, Linker), Error> { let mut config = Config::default(); config.async_support(true); @@ -121,10 +122,11 @@ impl PluginBuilder { /// Create a new [`PluginBuilder`] with the given WASI context. /// Using the default context is a safe bet, see [`new_with_default_context`]. + /// This plugin will yield after each fixed configurable epoch. pub fn new_epoch( wasi_ctx: WasiCtx, yield_epoch: PluginYieldEpoch, - callback: C, + spawn_detached_future: C, ) -> Result where C: FnOnce(std::pin::Pin + Send + 'static>>) -> () @@ -135,7 +137,7 @@ impl PluginBuilder { // because we need the engine to load the plugin let epoch = yield_epoch.epoch; let initialize_incrementer = Box::new(move |engine: Engine| { - callback(Box::pin(async move { + spawn_detached_future(Box::pin(async move { loop { smol::Timer::after(epoch).await; engine.increment_epoch(); @@ -157,6 +159,9 @@ impl PluginBuilder { }) } + /// Create a new [`PluginBuilder`] with the given WASI context. + /// Using the default context is a safe bet, see [`new_with_default_context`]. + /// This plugin will yield after a configurable amount of fuel is consumed. pub fn new_fuel(wasi_ctx: WasiCtx, yield_fuel: PluginYieldFuel) -> Result { let yield_when = PluginYield::Fuel(yield_fuel); let (engine, linker) = Self::create_engine(&yield_when)?; @@ -178,18 +183,22 @@ impl PluginBuilder { .build() } + /// Create a new `PluginBuilder` with the default `WasiCtx` (see [`default_ctx`]). + /// This plugin will yield after each fixed configurable epoch. pub fn new_epoch_with_default_ctx( yield_epoch: PluginYieldEpoch, - callback: C, + spawn_detached_future: C, ) -> Result where C: FnOnce(std::pin::Pin + Send + 'static>>) -> () + Send + 'static, { - Self::new_epoch(Self::default_ctx(), yield_epoch, callback) + Self::new_epoch(Self::default_ctx(), yield_epoch, spawn_detached_future) } + /// Create a new `PluginBuilder` with the default `WasiCtx` (see [`default_ctx`]). + /// This plugin will yield after a configurable amount of fuel is consumed. pub fn new_fuel_with_default_ctx(yield_fuel: PluginYieldFuel) -> Result { Self::new_fuel(Self::default_ctx(), yield_fuel) } @@ -452,25 +461,6 @@ impl Plugin { Ok(Plugin { store, instance }) } - // async fn init_fuel( - // precompiled: bool, - // module: &[u8], - // plugin: PluginBuilder, - // ) -> Result { - // let (_, plugin) = Self::init_inner(precompiled, module, plugin).await?; - // Ok(plugin) - // } - - // async fn init_epoch( - // precompiled: bool, - // module: &[u8], - // plugin: PluginBuilder, - // callback: C, - // ) -> Result { - // let (_, plugin) = Self::init_inner(precompiled, module, plugin).await?; - // Ok(plugin) - // } - /// Attaches a file or directory the the given system path to the runtime. /// Note that the resource must be freed by calling `remove_resource` afterwards. pub fn attach_path>(&mut self, path: T) -> Result { From c956a8866e860f1b4885cb1f4872c4e26101d33d Mon Sep 17 00:00:00 2001 From: Isaac Clayton Date: Wed, 13 Jul 2022 13:21:45 +0200 Subject: [PATCH 08/10] Quick documentation fix --- crates/plugin_runtime/src/plugin.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/plugin_runtime/src/plugin.rs b/crates/plugin_runtime/src/plugin.rs index 410185078f8de77fc961916fecdd36277dcf49e6..153643db66e2995cb7f1d55ea0e60245d3a4244a 100644 --- a/crates/plugin_runtime/src/plugin.rs +++ b/crates/plugin_runtime/src/plugin.rs @@ -345,8 +345,6 @@ impl PluginBuilder { /// Initializes a [`Plugin`] from a given compiled Wasm module. /// Both binary (`.wasm`) and text (`.wat`) module formats are supported. - /// Will panic if this is plugin uses `PluginYield::Epoch`, - /// but an epoch incrementer has not yet been created. pub async fn init>(self, precompiled: bool, module: T) -> Result { Plugin::init(precompiled, module.as_ref(), self).await } From daf1674ca60a7f61df2f569d92d0f6ba60ec1448 Mon Sep 17 00:00:00 2001 From: Isaac Clayton Date: Wed, 13 Jul 2022 13:27:31 +0200 Subject: [PATCH 09/10] Fix failing test --- crates/plugin_runtime/src/plugin.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/plugin_runtime/src/plugin.rs b/crates/plugin_runtime/src/plugin.rs index 153643db66e2995cb7f1d55ea0e60245d3a4244a..988c9263351041cc5d0d1f2c433824db4ee067c2 100644 --- a/crates/plugin_runtime/src/plugin.rs +++ b/crates/plugin_runtime/src/plugin.rs @@ -105,8 +105,6 @@ impl PluginBuilder { fn create_engine(yield_when: &PluginYield) -> Result<(Engine, Linker), Error> { let mut config = Config::default(); config.async_support(true); - let engine = Engine::new(&config)?; - let linker = Linker::new(&engine); match yield_when { PluginYield::Epoch { .. } => { @@ -117,6 +115,8 @@ impl PluginBuilder { } } + let engine = Engine::new(&config)?; + let linker = Linker::new(&engine); Ok((engine, linker)) } From a6edf850786387e06aa0136b84235fecbd1f4184 Mon Sep 17 00:00:00 2001 From: Isaac Clayton Date: Wed, 13 Jul 2022 14:26:52 +0200 Subject: [PATCH 10/10] Use enum to differentiate between normal and precompiled plugins --- crates/plugin_runtime/src/lib.rs | 7 +++---- crates/plugin_runtime/src/plugin.rs | 18 +++++++++++------- crates/zed/src/languages/language_plugin.rs | 9 ++++----- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/crates/plugin_runtime/src/lib.rs b/crates/plugin_runtime/src/lib.rs index b59f03a55f9b5c35f3bdc78dcfe8d2ad1c6495d0..b008a98c287541c2cd73067fee6a59edd4d79fda 100644 --- a/crates/plugin_runtime/src/lib.rs +++ b/crates/plugin_runtime/src/lib.rs @@ -46,10 +46,9 @@ mod tests { .map(|output| output.stdout) }) .unwrap() - .init( - false, - include_bytes!("../../../plugins/bin/test_plugin.wasm"), - ) + .init(PluginBinary::Wasm( + include_bytes!("../../../plugins/bin/test_plugin.wasm").as_ref(), + )) .await .unwrap(); diff --git a/crates/plugin_runtime/src/plugin.rs b/crates/plugin_runtime/src/plugin.rs index 988c9263351041cc5d0d1f2c433824db4ee067c2..2767d9c4b876e0697f817774a155e7aab9a22561 100644 --- a/crates/plugin_runtime/src/plugin.rs +++ b/crates/plugin_runtime/src/plugin.rs @@ -345,8 +345,8 @@ impl PluginBuilder { /// Initializes a [`Plugin`] from a given compiled Wasm module. /// Both binary (`.wasm`) and text (`.wat`) module formats are supported. - pub async fn init>(self, precompiled: bool, module: T) -> Result { - Plugin::init(precompiled, module.as_ref(), self).await + pub async fn init<'a>(self, binary: PluginBinary<'a>) -> Result { + Plugin::init(binary, self).await } } @@ -379,6 +379,11 @@ impl WasiCtxAlloc { } } +pub enum PluginBinary<'a> { + Wasm(&'a [u8]), + Precompiled(&'a [u8]), +} + /// Represents a WebAssembly plugin, with access to the WebAssembly System Inferface. /// Build a new plugin using [`PluginBuilder`]. pub struct Plugin { @@ -406,7 +411,7 @@ impl Plugin { println!(); } - async fn init(precompiled: bool, module: &[u8], plugin: PluginBuilder) -> Result { + async fn init<'a>(binary: PluginBinary<'a>, plugin: PluginBuilder) -> Result { // initialize the WebAssembly System Interface context let engine = plugin.engine; let mut linker = plugin.linker; @@ -422,10 +427,9 @@ impl Plugin { }, ); - let module = if precompiled { - unsafe { Module::deserialize(&engine, module)? } - } else { - Module::new(&engine, module)? + let module = match binary { + PluginBinary::Precompiled(bytes) => unsafe { Module::deserialize(&engine, bytes)? }, + PluginBinary::Wasm(bytes) => Module::new(&engine, bytes)?, }; // set up automatic yielding based on configuration diff --git a/crates/zed/src/languages/language_plugin.rs b/crates/zed/src/languages/language_plugin.rs index 8417f0ff9a6f699878309f406a8ef875f3f76317..db96c6832908b65c181cd0223fef241dff17783f 100644 --- a/crates/zed/src/languages/language_plugin.rs +++ b/crates/zed/src/languages/language_plugin.rs @@ -4,7 +4,7 @@ use client::http::HttpClient; use futures::lock::Mutex; use gpui::executor::Background; use language::{LanguageServerName, LspAdapter}; -use plugin_runtime::{Plugin, PluginBuilder, PluginYield, WasiFn}; +use plugin_runtime::{Plugin, PluginBinary, PluginBuilder, PluginYield, WasiFn}; use std::{any::Any, path::PathBuf, sync::Arc}; use util::ResultExt; @@ -24,10 +24,9 @@ pub async fn new_json(executor: Arc) -> Result { .log_err() .map(|output| output.stdout) })? - .init( - true, - include_bytes!("../../../../plugins/bin/json_language.wasm.pre"), - ) + .init(PluginBinary::Precompiled(include_bytes!( + "../../../../plugins/bin/json_language.wasm.pre" + ))) .await?; PluginLspAdapter::new(plugin, executor).await