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

Extend precompile to support a DAG #792

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
22 changes: 18 additions & 4 deletions crates/containerd-shim-wasm/src/container/engine.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use std::collections::BTreeSet;
use std::fs::File;
use std::future::Future;
use std::io::Read;

use anyhow::{bail, Context, Result};
Expand Down Expand Up @@ -60,10 +62,11 @@ pub trait Engine: Clone + Send + Sync + 'static {
/// This is used to precompile the layers before they are run and will be called if `can_precompile` returns `true`.
/// It is called only the first time a module is run and the resulting bytes will be cached in the containerd content store.
/// The cached, precompiled layers will be reloaded on subsequent runs.
/// The runtime is expected to return the same number of layers passed in, if the layer cannot be precompiled it should return `None` for that layer.
/// In some edge cases it is possible that the layers may already be precompiled and None should be returned in this case.
fn precompile(&self, _layers: &[WasmLayer]) -> Result<Vec<Option<Vec<u8>>>> {
bail!("precompile not supported");
fn precompile(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: why don't we define this fucntion to be async?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using async function in public traits is discouraged.
The reason is that usually you want your future to be Send so that you can execute it in a multithreaded runtime.
The async fn syntax doesn't enforce the Send bound, so the suggested approach is to desugar the async fn into an impl Future + Send.

&self,
_layers: &[WasmLayer],
) -> impl Future<Output = Result<Vec<PrecompiledLayer>>> + Send {
async move { bail!("precompile not supported") }
}

/// Can_precompile lets the shim know if the runtime supports precompilation.
Expand All @@ -81,3 +84,14 @@ pub trait Engine: Clone + Send + Sync + 'static {
None
}
}

/// A `PrecompiledLayer` represents the precompiled bytes of a layer and the digests of parent layers (if any) used to process it.
#[derive(Clone)]
pub struct PrecompiledLayer {
/// The media type this layer represents.
pub media_type: String,
/// The bytes of the precompiled layer.
pub bytes: Vec<u8>,
/// Digests of this layers' parents.
pub parents: BTreeSet<String>,
}
2 changes: 1 addition & 1 deletion crates/containerd-shim-wasm/src/container/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ mod wasm;

pub(crate) use context::WasiContext;
pub use context::{Entrypoint, RuntimeContext, Source};
pub use engine::Engine;
pub use engine::{Engine, PrecompiledLayer};
pub use instance::Instance;
pub use path::PathResolve;
pub use wasm::WasmBinaryType;
Expand Down
Loading
Loading