chore(development): use singlepass in debug mode (#2134)

* Add new feature flags

* Use singlepass in debug mode

* Use Cow to avoid unnecessary copies

- Instead of removing and reinserting into the memory cache, use Cow to
  model both owned an borrowed data
- Log at debug-level the time to compile/load a wasm module
- A little clippy drive-by on touched files

* Satisfy the assumption from zellij-utils/src/consts.rs for target-dir

* Allow forcing cranlift in debug mode

* Remove deprecated comments

* PR comment: typo

* Remove extras
This commit is contained in:
m-lima 2023-02-07 17:55:37 +01:00 committed by GitHub
parent e3981283a9
commit 5b3e1ecacd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 93 additions and 14 deletions

View file

@ -85,6 +85,14 @@ Note that the output is truncated at 100KB. This can be adjusted for the purpose
When running Zellij with the `--debug` flag, Zellij will dump a copy of all bytes received over the pty for each pane in: `/$temp_dir/zellij-<UID>/zellij-log/zellij-<pane_id>.log`. These might be useful when troubleshooting terminal issues. When running Zellij with the `--debug` flag, Zellij will dump a copy of all bytes received over the pty for each pane in: `/$temp_dir/zellij-<UID>/zellij-log/zellij-<pane_id>.log`. These might be useful when troubleshooting terminal issues.
## Testing plugins
Zellij by default uses a fast, non-optimized, compiler for WASM when running in debug mode. This can be overriden by using the `force_cranelift` feature flag, if you wish to reproduce the behavior of release mode.
To enable the flag, run:
```sh
cargo xtask run --cranelift
```
## How we treat clippy lints ## How we treat clippy lints
We currently use clippy in [GitHub Actions](https://github.com/zellij-org/zellij/blob/main/.github/workflows/rust.yml) with the default settings that report only [`clippy::correctness`](https://github.com/rust-lang/rust-clippy#readme) as errors and other lints as warnings because Zellij is still unstable. This means that all warnings can be ignored depending on the situation at that time, even though they are also helpful to keep the code quality. We currently use clippy in [GitHub Actions](https://github.com/zellij-org/zellij/blob/main/.github/workflows/rust.yml) with the default settings that report only [`clippy::correctness`](https://github.com/rust-lang/rust-clippy#readme) as errors and other lints as warnings because Zellij is still unstable. This means that all warnings can be ignored depending on the situation at that time, even though they are also helpful to keep the code quality.

46
Cargo.lock generated
View file

@ -836,6 +836,32 @@ version = "1.0.4"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "212d0f5754cb6769937f4501cc0e67f4f4483c8d2c3e1e922ee9edbe4ab4c7c0" checksum = "212d0f5754cb6769937f4501cc0e67f4f4483c8d2c3e1e922ee9edbe4ab4c7c0"
[[package]]
name = "dynasm"
version = "1.2.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "add9a102807b524ec050363f09e06f1504214b0e1c7797f64261c891022dce8b"
dependencies = [
"bitflags",
"byteorder",
"lazy_static",
"proc-macro-error",
"proc-macro2",
"quote",
"syn",
]
[[package]]
name = "dynasmrt"
version = "1.2.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "64fba5a42bd76a17cad4bfa00de168ee1cbfa06a5e8ce992ae880218c05641a9"
dependencies = [
"byteorder",
"dynasm",
"memmap2",
]
[[package]] [[package]]
name = "either" name = "either"
version = "1.6.1" version = "1.6.1"
@ -3412,6 +3438,7 @@ dependencies = [
"wasmer-artifact", "wasmer-artifact",
"wasmer-compiler", "wasmer-compiler",
"wasmer-compiler-cranelift", "wasmer-compiler-cranelift",
"wasmer-compiler-singlepass",
"wasmer-derive", "wasmer-derive",
"wasmer-engine", "wasmer-engine",
"wasmer-engine-dylib", "wasmer-engine-dylib",
@ -3473,6 +3500,25 @@ dependencies = [
"wasmer-types", "wasmer-types",
] ]
[[package]]
name = "wasmer-compiler-singlepass"
version = "2.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "29ca2a35204d8befa85062bc7aac259a8db8070b801b8a783770ba58231d729e"
dependencies = [
"byteorder",
"dynasm",
"dynasmrt",
"gimli",
"lazy_static",
"loupe",
"more-asserts",
"rayon",
"smallvec",
"wasmer-compiler",
"wasmer-types",
]
[[package]] [[package]]
name = "wasmer-derive" name = "wasmer-derive"
version = "2.3.0" version = "2.3.0"

View file

@ -73,3 +73,4 @@ pkg-fmt = "tgz"
default = [ "zellij-utils/plugins_from_target" ] default = [ "zellij-utils/plugins_from_target" ]
disable_automatic_asset_installation = [ "zellij-utils/disable_automatic_asset_installation" ] disable_automatic_asset_installation = [ "zellij-utils/disable_automatic_asset_installation" ]
unstable = [ "zellij-client/unstable", "zellij-utils/unstable" ] unstable = [ "zellij-client/unstable", "zellij-utils/unstable" ]
force_cranelift = [ "zellij-server/force_cranelift" ]

View file

@ -63,6 +63,8 @@ xflags::xflags! {
cmd run { cmd run {
/// Take plugins from here, skip building plugins. Passed to zellij verbatim /// Take plugins from here, skip building plugins. Passed to zellij verbatim
optional --data-dir path: PathBuf optional --data-dir path: PathBuf
/// Force the use of Cranelift for compiling WASM modules
optional --cranelift
/// Arguments to pass after `cargo run --` /// Arguments to pass after `cargo run --`
repeated args: OsString repeated args: OsString
} }
@ -173,6 +175,8 @@ pub struct Run {
pub args: Vec<OsString>, pub args: Vec<OsString>,
pub data_dir: Option<PathBuf>, pub data_dir: Option<PathBuf>,
pub cranelift: bool,
} }
#[derive(Debug)] #[derive(Debug)]

View file

@ -94,6 +94,8 @@ pub fn install(sh: &Shell, flags: flags::Install) -> anyhow::Result<()> {
pub fn run(sh: &Shell, flags: flags::Run) -> anyhow::Result<()> { pub fn run(sh: &Shell, flags: flags::Run) -> anyhow::Result<()> {
let err_context = || format!("failed to run pipeline 'run' with args {flags:?}"); let err_context = || format!("failed to run pipeline 'run' with args {flags:?}");
let cranelift = flags.cranelift.then_some(["--features", "force_cranelift"]);
if let Some(ref data_dir) = flags.data_dir { if let Some(ref data_dir) = flags.data_dir {
let data_dir = sh.current_dir().join(data_dir); let data_dir = sh.current_dir().join(data_dir);
@ -103,6 +105,7 @@ pub fn run(sh: &Shell, flags: flags::Run) -> anyhow::Result<()> {
.args(["--package", "zellij"]) .args(["--package", "zellij"])
.arg("--no-default-features") .arg("--no-default-features")
.args(["--features", "disable_automatic_asset_installation"]) .args(["--features", "disable_automatic_asset_installation"])
.args(cranelift.iter().flatten())
.args(["--", "--data-dir", &format!("{}", data_dir.display())]) .args(["--", "--data-dir", &format!("{}", data_dir.display())])
.args(&flags.args) .args(&flags.args)
.run() .run()
@ -120,7 +123,9 @@ pub fn run(sh: &Shell, flags: flags::Run) -> anyhow::Result<()> {
) )
.and_then(|_| crate::cargo()) .and_then(|_| crate::cargo())
.and_then(|cargo| { .and_then(|cargo| {
cmd!(sh, "{cargo} run --") cmd!(sh, "{cargo} run")
.args(cranelift.iter().flatten())
.args(["--"])
.args(&flags.args) .args(&flags.args)
.run() .run()
.map_err(anyhow::Error::new) .map_err(anyhow::Error::new)
@ -134,7 +139,7 @@ pub fn run(sh: &Shell, flags: flags::Run) -> anyhow::Result<()> {
/// This includes the optimized zellij executable from the [`install`] pipeline, the man page, the /// This includes the optimized zellij executable from the [`install`] pipeline, the man page, the
/// `.desktop` file and the application logo. /// `.desktop` file and the application logo.
pub fn dist(sh: &Shell, _flags: flags::Dist) -> anyhow::Result<()> { pub fn dist(sh: &Shell, _flags: flags::Dist) -> anyhow::Result<()> {
let err_context = || format!("failed to run pipeline 'dist'"); let err_context = || "failed to run pipeline 'dist'";
sh.change_dir(crate::project_root()); sh.change_dir(crate::project_root());
if sh.path_exists("target/dist") { if sh.path_exists("target/dist") {

View file

@ -18,7 +18,7 @@ daemonize = "0.4.1"
serde_json = "1.0" serde_json = "1.0"
unicode-width = "0.1.8" unicode-width = "0.1.8"
url = "2.2.2" url = "2.2.2"
wasmer = "2.3.0" wasmer = { version = "2.3.0", features = ["singlepass"]}
wasmer-wasi = "2.3.0" wasmer-wasi = "2.3.0"
cassowary = "0.3.0" cassowary = "0.3.0"
zellij-utils = { path = "../zellij-utils/", version = "0.34.5" } zellij-utils = { path = "../zellij-utils/", version = "0.34.5" }
@ -36,3 +36,5 @@ semver = "0.11.0"
[dev-dependencies] [dev-dependencies]
insta = "1.6.0" insta = "1.6.0"
[features]
force_cranelift = []

View file

@ -749,7 +749,7 @@ fn init_session(
Some(&to_background_jobs), Some(&to_background_jobs),
None, None,
); );
let store = Store::default(); let store = get_store();
move || { move || {
plugin_thread_main( plugin_thread_main(
@ -818,3 +818,15 @@ fn init_session(
background_jobs_thread: Some(background_jobs_thread), background_jobs_thread: Some(background_jobs_thread),
} }
} }
#[cfg(any(feature = "force_cranelift", not(debug_assertions)))]
fn get_store() -> Store {
log::info!("Compiling plugins using Cranelift");
Store::new(&wasmer::Universal::new(wasmer::Cranelift::default()).engine())
}
#[cfg(not(any(feature = "force_cranelift", not(debug_assertions))))]
fn get_store() -> Store {
log::info!("Compiling plugins using Singlepass");
Store::new(&wasmer::Universal::new(wasmer::Singlepass::default()).engine())
}

View file

@ -30,7 +30,7 @@ use crate::{
}; };
use zellij_utils::{ use zellij_utils::{
consts::{DEBUG_MODE, VERSION, ZELLIJ_CACHE_DIR, ZELLIJ_TMP_DIR}, consts::{VERSION, ZELLIJ_CACHE_DIR, ZELLIJ_TMP_DIR},
data::{Event, EventType, PluginIds}, data::{Event, EventType, PluginIds},
errors::prelude::*, errors::prelude::*,
input::{ input::{
@ -293,13 +293,15 @@ impl WasmBridge {
.collect(); .collect();
let cached_path = ZELLIJ_CACHE_DIR.join(&hash); let cached_path = ZELLIJ_CACHE_DIR.join(&hash);
let timer = std::time::Instant::now();
unsafe { unsafe {
match Module::deserialize_from_file(&self.store, &cached_path) { match Module::deserialize_from_file(&self.store, &cached_path) {
Ok(m) => { Ok(m) => {
log::debug!( log::info!(
"Loaded plugin '{}' from cache folder at '{}'", "Loaded plugin '{}' from cache folder at '{}' in {:?}",
plugin.path.display(), plugin.path.display(),
ZELLIJ_CACHE_DIR.display(), ZELLIJ_CACHE_DIR.display(),
timer.elapsed(),
); );
m m
}, },
@ -313,6 +315,11 @@ impl WasmBridge {
}) })
.and_then(|m| { .and_then(|m| {
m.serialize_to_file(&cached_path).map_err(anyError::new)?; m.serialize_to_file(&cached_path).map_err(anyError::new)?;
log::info!(
"Compiled plugin '{}' in {:?}",
plugin.path.display(),
timer.elapsed()
);
Ok(m) Ok(m)
}) })
.with_context(inner_context) .with_context(inner_context)
@ -442,13 +449,7 @@ impl WasmBridge {
&mut self, &mut self,
mut updates: Vec<(Option<u32>, Option<ClientId>, Event)>, mut updates: Vec<(Option<u32>, Option<ClientId>, Event)>,
) -> Result<()> { ) -> Result<()> {
let err_context = || { let err_context = || "failed to update plugin state".to_string();
if *DEBUG_MODE.get().unwrap_or(&true) {
format!("failed to update plugin state")
} else {
"failed to update plugin state".to_string()
}
};
let mut plugin_bytes = vec![]; let mut plugin_bytes = vec![];
for (pid, cid, event) in updates.drain(..) { for (pid, cid, event) in updates.drain(..) {