Skip to content

Commit

Permalink
Remove some unused memory management code in the adapter (#398)
Browse files Browse the repository at this point in the history
After #394 there were some unused functions in the adapter that didn't fail in
CI as we don't currently build the adapter in CI. This PR removes the unused
code, and also builds the adapter in CI.
  • Loading branch information
elliottt authored Jun 28, 2024
1 parent a4bc8c1 commit b12dbfb
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 55 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ jobs:
- name: test
run: make ci
shell: bash
- name: adapter
run: cargo build --release -p viceroy-component-adapter --target wasm32-unknown-unknown

# Run the trap test in an isolated job. It needs different cargo features than the usual build, so
# it entails rebuilding the whole workspace if we combine them in a single job. This way, we
Expand Down
14 changes: 4 additions & 10 deletions crates/adapter/src/descriptors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,7 @@ impl Streams {

pub enum StreamType {
/// Streams for implementing stdio.
Stdio(Stdio),
}

pub enum Stdio {
Stdin,
Stdout,
Stderr,
Stdio,
}

#[repr(C)]
Expand Down Expand Up @@ -94,19 +88,19 @@ impl Descriptors {
d.push(Descriptor::Streams(Streams {
input: new_once(stdin::get_stdin()),
output: OnceCell::new(),
type_: StreamType::Stdio(Stdio::Stdin),
type_: StreamType::Stdio,
}))
.trapping_unwrap();
d.push(Descriptor::Streams(Streams {
input: OnceCell::new(),
output: new_once(stdout::get_stdout()),
type_: StreamType::Stdio(Stdio::Stdout),
type_: StreamType::Stdio,
}))
.trapping_unwrap();
d.push(Descriptor::Streams(Streams {
input: OnceCell::new(),
output: new_once(stderr::get_stderr()),
type_: StreamType::Stdio(Stdio::Stderr),
type_: StreamType::Stdio,
}))
.trapping_unwrap();

Expand Down
50 changes: 5 additions & 45 deletions crates/adapter/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Promote warnings into errors, when building in release mode.
#![cfg_attr(not(debug_assertions), deny(warnings))]

use crate::bindings::wasi::clocks::{monotonic_clock, wall_clock};
use crate::bindings::wasi::io::poll;
use crate::bindings::wasi::io::streams;
Expand Down Expand Up @@ -219,22 +222,6 @@ enum ImportAlloc {
pointers: BumpAlloc,
},

/// An allocator specifically for getting the nth string allocation used
/// for preopens.
///
/// This will allocate everything into `alloc`. All strings other than the
/// `nth` string, however, will be discarded (the allocator's state is reset
/// after the allocation). This means that the pointer returned for the
/// `nth` string will be retained in `alloc` while all others will be
/// discarded.
///
/// The `cur` count starts at 0 and counts up per-string.
GetPreopenPath {
cur: u32,
nth: u32,
alloc: BumpAlloc,
},

/// No import allocator is configured and if an allocation happens then
/// this will abort.
None,
Expand Down Expand Up @@ -301,12 +288,6 @@ impl ImportAlloc {
// WASI doesn't say all the strings have to be adjacent, so this
// should work out in practice.
//
// * Finally for `GetPreopenPath` this works out only insofar that the
// `State::temporary_alloc` space is used to store the path. The
// WASI-provided buffer is precisely sized, not overly large, meaning
// that we're forced to copy from `temporary_alloc` into the
// destination buffer for this WASI call.
//
// Basically it's a case-by-case basis here that enables ignoring
// shrinking return calls here. Not robust.
if !old_ptr.is_null() {
Expand Down Expand Up @@ -338,18 +319,6 @@ impl ImportAlloc {
alloc.alloc(align, size)
}
}
ImportAlloc::GetPreopenPath { cur, nth, alloc } => {
if align == 1 {
let real_alloc = *nth == *cur;
if real_alloc {
alloc.alloc(align, size)
} else {
alloc.clone().alloc(align, size)
}
} else {
alloc.alloc(align, size)
}
}
ImportAlloc::None => {
unreachable!("no allocator configured")
}
Expand Down Expand Up @@ -1181,7 +1150,7 @@ pub unsafe extern "C" fn poll_oneoff(
.trapping_unwrap();
match desc {
Descriptor::Streams(streams) => match &streams.type_ {
StreamType::Stdio(_) => (ERRNO_SUCCESS, 1, 0),
StreamType::Stdio => (ERRNO_SUCCESS, 1, 0),
},
_ => unreachable!(),
}
Expand All @@ -1194,7 +1163,7 @@ pub unsafe extern "C" fn poll_oneoff(
.trapping_unwrap();
match desc {
Descriptor::Streams(streams) => match &streams.type_ {
StreamType::Stdio(_) => (ERRNO_SUCCESS, 1, 0),
StreamType::Stdio => (ERRNO_SUCCESS, 1, 0),
},
_ => unreachable!(),
}
Expand Down Expand Up @@ -1614,15 +1583,6 @@ impl State {
}
}

/// Configure that `cabi_import_realloc` will allocate once from
/// `self.temporary_data` for the duration of the closure `f`.
///
/// Panics if the import allocator is already configured.
fn with_one_temporary_alloc<T>(&self, f: impl FnOnce() -> T) -> T {
let alloc = unsafe { self.temporary_alloc() };
self.with_import_alloc(ImportAlloc::OneAlloc(alloc), f).0
}

/// Configure that `cabi_import_realloc` will allocate once from
/// `base` with at most `len` bytes for the duration of `f`.
///
Expand Down
Binary file modified lib/data/viceroy-component-adapter.wasm
Binary file not shown.

0 comments on commit b12dbfb

Please sign in to comment.