Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent redundant merging of the filesystems #5066

Merged
merged 3 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/wasix/src/bin_factory/binary_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ impl BinaryPackageCommand {
#[derivative(Debug)]
pub struct BinaryPackage {
pub id: PackageId,
/// Includes the ids of all the packages in the tree
pub package_ids: Vec<PackageId>,

pub when_cached: Option<u128>,
/// The name of the [`BinaryPackageCommand`] which is this package's
Expand Down
1 change: 1 addition & 0 deletions lib/wasix/src/runners/wasi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ impl WasiRunner {
let container_fs = if let Some(pkg) = pkg {
builder.add_webc(pkg.clone());
builder.set_module_hash(pkg.hash());
builder.include_packages(pkg.package_ids.clone());
Some(Arc::clone(&pkg.webc_fs))
} else {
None
Expand Down
2 changes: 2 additions & 0 deletions lib/wasix/src/runtime/package_loader/load_package_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pub async fn load_package_tree(
) -> Result<BinaryPackage, Error> {
let mut containers = fetch_dependencies(loader, &resolution.package, &resolution.graph).await?;
containers.insert(resolution.package.root_package.clone(), root.clone());
let package_ids = containers.keys().cloned().collect();
let fs = filesystem(&containers, &resolution.package, root_is_local_dir)?;

let root = &resolution.package.root_package;
Expand All @@ -52,6 +53,7 @@ pub async fn load_package_tree(

let loaded = BinaryPackage {
id: root.clone(),
package_ids,
when_cached: crate::syscalls::platform_clock_time_get(
wasmer_wasix_types::wasi::Snapshot0Clockid::Monotonic,
1_000_000,
Expand Down
24 changes: 23 additions & 1 deletion lib/wasix/src/state/builder.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Builder system for configuring a [`WasiState`] and creating it.

use std::{
collections::HashMap,
collections::{HashMap, HashSet},
path::{Path, PathBuf},
sync::Arc,
};
Expand All @@ -10,6 +10,7 @@ use rand::Rng;
use thiserror::Error;
use virtual_fs::{ArcFile, FileSystem, FsError, TmpFileSystem, VirtualFile};
use wasmer::{AsStoreMut, Extern, Imports, Instance, Module, Store};
use wasmer_config::package::PackageId;

#[cfg(feature = "journal")]
use crate::journal::{DynJournal, SnapshotTrigger};
Expand Down Expand Up @@ -69,6 +70,8 @@ pub struct WasiEnvBuilder {
/// List of webc dependencies to be injected.
pub(super) uses: Vec<BinaryPackage>,

pub(super) included_packages: HashSet<PackageId>,

pub(super) module_hash: Option<ModuleHash>,

/// List of host commands to map into the WASI instance.
Expand Down Expand Up @@ -323,6 +326,21 @@ impl WasiEnvBuilder {
self
}

/// Adds a package that is already included in the [`WasiEnvBuilder`] filesystem.
/// These packages will not be merged to the final filesystem since they are already included.
pub fn include_package(&mut self, pkg_id: PackageId) -> &mut Self {
self.included_packages.insert(pkg_id);
self
}

/// Adds packages that is already included in the [`WasiEnvBuilder`] filesystem.
/// These packages will not be merged to the final filesystem since they are already included.
pub fn include_packages(&mut self, pkg_ids: impl IntoIterator<Item = PackageId>) -> &mut Self {
self.included_packages.extend(pkg_ids.into_iter());

self
}

/// Adds a list of other containers this module inherits from.
///
/// This will make all of the container's files and commands available to the
Expand Down Expand Up @@ -841,6 +859,10 @@ impl WasiEnvBuilder {
wasi_fs.set_current_dir(s);
}

for id in &self.included_packages {
wasi_fs.has_unioned.lock().unwrap().insert(id.clone());
}

let state = WasiState {
fs: wasi_fs,
secret: rand::thread_rng().gen::<[u8; 32]>(),
Expand Down
6 changes: 3 additions & 3 deletions lib/wasix/src/state/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1094,9 +1094,9 @@ impl WasiEnv {
tracing::trace!(package=%pkg.id, "merging package dependency into wasi environment");
let root_fs = &self.state.fs.root_fs;

// We first need to copy any files in the package over to the
// main file system
if let Err(e) = InlineWaker::block_on(root_fs.merge(&pkg.webc_fs)) {
// We first need to merge the filesystem in the package into the
// main file system, if it has not been merged already.
if let Err(e) = InlineWaker::block_on(self.state.fs.conditional_union(pkg)) {
tracing::warn!(
error = &e as &dyn std::error::Error,
"Unable to merge the package's filesystem into the main one",
Expand Down
Loading