From 3fb1cd07260afa2ebcf0aadd1dd96a87b51adb5d Mon Sep 17 00:00:00 2001 From: Isaac Clayton Date: Wed, 13 Jul 2022 16:31:47 +0200 Subject: [PATCH 1/4] Fix issue where precompiled plugins were compiled with the wrong settings --- crates/plugin_runtime/build.rs | 24 +++++++++++++++------ crates/zed/src/languages/language_plugin.rs | 2 +- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/crates/plugin_runtime/build.rs b/crates/plugin_runtime/build.rs index d1b1b58411..b323bdc34e 100644 --- a/crates/plugin_runtime/build.rs +++ b/crates/plugin_runtime/build.rs @@ -43,7 +43,8 @@ fn main() { assert!(build_successful); // Find all compiled binaries - let engine = create_default_engine(); + let epoch_engine = create_epoch_engine(); + let fuel_engine = create_fuel_engine(); let binaries = std::fs::read_dir(base.join("target/wasm32-wasi").join(profile_target)) .expect("Could not find compiled plugins in target"); @@ -61,26 +62,35 @@ fn main() { if let Some(path) = is_wasm() { let out_path = base.join("bin").join(path.file_name().unwrap()); std::fs::copy(&path, &out_path).expect("Could not copy compiled plugin to bin"); - precompile(&out_path, &engine); + precompile(&out_path, &epoch_engine, "epoch"); + precompile(&out_path, &fuel_engine, "fuel"); } } } -/// Creates a default engine for compiling Wasm. -fn create_default_engine() -> Engine { +fn create_epoch_engine() -> Engine { let mut config = Config::default(); config.async_support(true); + config.epoch_interruption(true); Engine::new(&config).expect("Could not create engine") } -fn precompile(path: &Path, engine: &Engine) { +fn create_fuel_engine() -> Engine { + let mut config = Config::default(); + config.async_support(true); + config.consume_fuel(true); + Engine::new(&config).expect("Could not create engine") +} + +fn precompile(path: &Path, engine: &Engine, engine_name: &str) { let bytes = std::fs::read(path).expect("Could not read wasm module"); let compiled = engine .precompile_module(&bytes) .expect("Could not precompile module"); let out_path = path.parent().unwrap().join(&format!( - "{}.pre", - path.file_name().unwrap().to_string_lossy() + "{}.{}.pre", + path.file_name().unwrap().to_string_lossy(), + engine_name, )); let mut out_file = std::fs::File::create(out_path) .expect("Could not create output file for precompiled module"); diff --git a/crates/zed/src/languages/language_plugin.rs b/crates/zed/src/languages/language_plugin.rs index 6f4bdab78e..690e6d385c 100644 --- a/crates/zed/src/languages/language_plugin.rs +++ b/crates/zed/src/languages/language_plugin.rs @@ -26,7 +26,7 @@ pub async fn new_json(executor: Arc) -> Result { .map(|output| output.stdout) })? .init(PluginBinary::Precompiled(include_bytes!( - "../../../../plugins/bin/json_language.wasm.pre" + "../../../../plugins/bin/json_language.wasm.epoch.pre" ))) .await?; From 562e22814f0920ee0e2928b3d317abeafe98103d Mon Sep 17 00:00:00 2001 From: Isaac Clayton Date: Wed, 13 Jul 2022 17:08:43 +0200 Subject: [PATCH 2/4] Remove .pre suffix use .epoch and .fuel instead --- crates/plugin_runtime/README.md | 2 ++ crates/plugin_runtime/build.rs | 2 +- crates/zed/src/languages/language_plugin.rs | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/plugin_runtime/README.md b/crates/plugin_runtime/README.md index 8ac843cb02..4088041fe6 100644 --- a/crates/plugin_runtime/README.md +++ b/crates/plugin_runtime/README.md @@ -154,6 +154,8 @@ Plugins in the `plugins` directory are automatically recompiled and serialized t - `plugin.wasm.pre` is the plugin compiled to Wasm *and additionally* precompiled to host-platform-agnostic cranelift-specific IR. This should be about 700KB for debug builds and 500KB in release builds. Each plugin takes about 1 or 2 seconds to compile to native code using cranelift, so precompiling plugins drastically reduces the startup time required to begin to run a plugin. +> TODO: Rework precompiled plugins. + For all intents and purposes, it is *highly recommended* that you use precompiled plugins where possible, as they are much more lightweight and take much less time to instantiate. ### Instantiating a plugin diff --git a/crates/plugin_runtime/build.rs b/crates/plugin_runtime/build.rs index b323bdc34e..8803cb2fb7 100644 --- a/crates/plugin_runtime/build.rs +++ b/crates/plugin_runtime/build.rs @@ -88,7 +88,7 @@ fn precompile(path: &Path, engine: &Engine, engine_name: &str) { .precompile_module(&bytes) .expect("Could not precompile module"); let out_path = path.parent().unwrap().join(&format!( - "{}.{}.pre", + "{}.{}", path.file_name().unwrap().to_string_lossy(), engine_name, )); diff --git a/crates/zed/src/languages/language_plugin.rs b/crates/zed/src/languages/language_plugin.rs index 690e6d385c..090dc6b313 100644 --- a/crates/zed/src/languages/language_plugin.rs +++ b/crates/zed/src/languages/language_plugin.rs @@ -26,7 +26,7 @@ pub async fn new_json(executor: Arc) -> Result { .map(|output| output.stdout) })? .init(PluginBinary::Precompiled(include_bytes!( - "../../../../plugins/bin/json_language.wasm.epoch.pre" + "../../../../plugins/bin/json_language.wasm.epoch" ))) .await?; From 8bb8e851df5ca7dc13f38372cfbf367056bf5f0a Mon Sep 17 00:00:00 2001 From: Isaac Clayton Date: Wed, 13 Jul 2022 18:03:14 +0200 Subject: [PATCH 3/4] Remove epoch-based metering --- crates/plugin_runtime/README.md | 2 - crates/plugin_runtime/build.rs | 25 ++-- crates/plugin_runtime/src/lib.rs | 2 +- crates/plugin_runtime/src/plugin.rs | 154 ++++---------------- crates/zed/src/languages/language_plugin.rs | 10 +- 5 files changed, 41 insertions(+), 152 deletions(-) diff --git a/crates/plugin_runtime/README.md b/crates/plugin_runtime/README.md index 4088041fe6..8ac843cb02 100644 --- a/crates/plugin_runtime/README.md +++ b/crates/plugin_runtime/README.md @@ -154,8 +154,6 @@ Plugins in the `plugins` directory are automatically recompiled and serialized t - `plugin.wasm.pre` is the plugin compiled to Wasm *and additionally* precompiled to host-platform-agnostic cranelift-specific IR. This should be about 700KB for debug builds and 500KB in release builds. Each plugin takes about 1 or 2 seconds to compile to native code using cranelift, so precompiling plugins drastically reduces the startup time required to begin to run a plugin. -> TODO: Rework precompiled plugins. - For all intents and purposes, it is *highly recommended* that you use precompiled plugins where possible, as they are much more lightweight and take much less time to instantiate. ### Instantiating a plugin diff --git a/crates/plugin_runtime/build.rs b/crates/plugin_runtime/build.rs index 8803cb2fb7..93ba175c18 100644 --- a/crates/plugin_runtime/build.rs +++ b/crates/plugin_runtime/build.rs @@ -43,8 +43,7 @@ fn main() { assert!(build_successful); // Find all compiled binaries - let epoch_engine = create_epoch_engine(); - let fuel_engine = create_fuel_engine(); + let engine = create_default_engine(); let binaries = std::fs::read_dir(base.join("target/wasm32-wasi").join(profile_target)) .expect("Could not find compiled plugins in target"); @@ -62,35 +61,29 @@ fn main() { if let Some(path) = is_wasm() { let out_path = base.join("bin").join(path.file_name().unwrap()); std::fs::copy(&path, &out_path).expect("Could not copy compiled plugin to bin"); - precompile(&out_path, &epoch_engine, "epoch"); - precompile(&out_path, &fuel_engine, "fuel"); + precompile(&out_path, &engine); } } } -fn create_epoch_engine() -> Engine { - let mut config = Config::default(); - config.async_support(true); - config.epoch_interruption(true); - Engine::new(&config).expect("Could not create engine") -} - -fn create_fuel_engine() -> Engine { +/// Creates an engine with the default configuration. +/// N.B. This must create an engine with the same config as the one +/// in `plugin_runtime/build.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") + Engine::new(&config).expect("Could not create precompilation engine") } -fn precompile(path: &Path, engine: &Engine, engine_name: &str) { +fn precompile(path: &Path, engine: &Engine) { let bytes = std::fs::read(path).expect("Could not read wasm module"); let compiled = engine .precompile_module(&bytes) .expect("Could not precompile module"); let out_path = path.parent().unwrap().join(&format!( - "{}.{}", + "{}.pre", path.file_name().unwrap().to_string_lossy(), - engine_name, )); let mut out_file = std::fs::File::create(out_path) .expect("Could not create output file for precompiled module"); diff --git a/crates/plugin_runtime/src/lib.rs b/crates/plugin_runtime/src/lib.rs index b008a98c28..8665a108c9 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_fuel_with_default_ctx(PluginYield::default_fuel()) + let mut runtime = PluginBuilder::new_default() .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 2767d9c4b8..2748a3f3f7 100644 --- a/crates/plugin_runtime/src/plugin.rs +++ b/crates/plugin_runtime/src/plugin.rs @@ -1,6 +1,5 @@ use std::future::Future; -use std::time::Duration; use std::{fs::File, marker::PhantomData, path::Path}; use anyhow::{anyhow, Error}; @@ -55,34 +54,14 @@ impl Clone for WasiFn { } } -pub struct PluginYieldEpoch { - delta: u64, - epoch: std::time::Duration, -} - -pub struct PluginYieldFuel { +pub struct Metering { initial: u64, refill: u64, } -pub enum PluginYield { - Epoch { - yield_epoch: PluginYieldEpoch, - initialize_incrementer: Box () + Send>, - }, - Fuel(PluginYieldFuel), -} - -impl PluginYield { - pub fn default_epoch() -> PluginYieldEpoch { - PluginYieldEpoch { - delta: 1, - epoch: Duration::from_millis(1), - } - } - - pub fn default_fuel() -> PluginYieldFuel { - PluginYieldFuel { +impl Default for Metering { + fn default() -> Self { + Metering { initial: 1000, refill: 1000, } @@ -97,110 +76,44 @@ pub struct PluginBuilder { wasi_ctx: WasiCtx, engine: Engine, linker: Linker, - yield_when: PluginYield, + metering: Metering, +} + +/// Creates an engine with the default configuration. +/// N.B. This must create an engine with the same config as the one +/// in `plugin_runtime/build.rs`. +fn create_default_engine() -> Result { + let mut config = Config::default(); + config.async_support(true); + config.consume_fuel(true); + Engine::new(&config) } 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); - - 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((engine, linker)) - } - - /// 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, - spawn_detached_future: C, - ) -> Result - where - C: FnOnce(std::pin::Pin + Send + 'static>>) -> () - + Send - + 'static, - { - // we can't create the future until after initializing - // because we need the engine to load the plugin - let epoch = yield_epoch.epoch; - let initialize_incrementer = Box::new(move |engine: Engine| { - spawn_detached_future(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, - engine, - linker, - yield_when, - }) - } - /// 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)?; + pub fn new(wasi_ctx: WasiCtx, metering: Metering) -> Result { + let engine = create_default_engine()?; + let linker = Linker::new(&engine); Ok(PluginBuilder { wasi_ctx, engine, linker, - yield_when, + metering, }) } - /// Create a new `WasiCtx` that inherits the - /// host processes' access to `stdout` and `stderr`. - fn default_ctx() -> WasiCtx { - WasiCtxBuilder::new() + /// 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_default() -> Result { + let default_ctx = WasiCtxBuilder::new() .inherit_stdout() .inherit_stderr() - .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, - spawn_detached_future: C, - ) -> Result - where - C: FnOnce(std::pin::Pin + Send + 'static>>) -> () - + Send - + 'static, - { - 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) + .build(); + let metering = Metering::default(); + Self::new(default_ctx, metering) } /// Add an `async` host function. See [`host_function`] for details. @@ -433,19 +346,8 @@ impl Plugin { }; // set up automatic yielding based on configuration - match plugin.yield_when { - 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(); - store.out_of_fuel_async_yield(u64::MAX, refill); - } - } + store.add_fuel(plugin.metering.initial).unwrap(); + store.out_of_fuel_async_yield(u64::MAX, plugin.metering.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 090dc6b313..e5513e97bf 100644 --- a/crates/zed/src/languages/language_plugin.rs +++ b/crates/zed/src/languages/language_plugin.rs @@ -5,16 +5,12 @@ use collections::HashMap; use futures::lock::Mutex; use gpui::executor::Background; use language::{LanguageServerName, LspAdapter}; -use plugin_runtime::{Plugin, PluginBinary, PluginBuilder, PluginYield, WasiFn}; +use plugin_runtime::{Plugin, PluginBinary, PluginBuilder, WasiFn}; 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(), move |future| { - executor_ref.spawn(future).detach() - })? + let plugin = PluginBuilder::new_default()? .host_function_async("command", |command: String| async move { let mut args = command.split(' '); let command = args.next().unwrap(); @@ -26,7 +22,7 @@ pub async fn new_json(executor: Arc) -> Result { .map(|output| output.stdout) })? .init(PluginBinary::Precompiled(include_bytes!( - "../../../../plugins/bin/json_language.wasm.epoch" + "../../../../plugins/bin/json_language.wasm.pre" ))) .await?; From 5e7456df4ea49f45cae4620ebda4bc0390e62938 Mon Sep 17 00:00:00 2001 From: Isaac Clayton Date: Wed, 13 Jul 2022 20:19:56 +0200 Subject: [PATCH 4/4] Fix docs --- crates/plugin_runtime/build.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/plugin_runtime/build.rs b/crates/plugin_runtime/build.rs index 93ba175c18..8a0c5c57de 100644 --- a/crates/plugin_runtime/build.rs +++ b/crates/plugin_runtime/build.rs @@ -68,7 +68,7 @@ fn main() { /// Creates an engine with the default configuration. /// N.B. This must create an engine with the same config as the one -/// in `plugin_runtime/build.rs`. +/// in `plugin_runtime/src/plugin.rs`. fn create_default_engine() -> Engine { let mut config = Config::default(); config.async_support(true);