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.

Sign up for GitHub

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

Jump to bottom

feat: Add features for dev and prod wasmer configurations #120

Merged
merged 7 commits into from
Oct 31, 2024
Merged

feat: Add features for dev and prod wasmer configurations #120

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4d4618d
feat: Add features for dev and prod wasmer configurations
ThetaSinner Oct 22, 2024
8c6751e
Fix fuzz scripts, although fuzzing doesn't just work
ThetaSinner Oct 25, 2024
723295d
Format check, rather than actually formatting
ThetaSinner Oct 30, 2024
abff424
Fix lint checking and then lint issues
ThetaSinner Oct 30, 2024
0d499c8
Fix another lint issue
ThetaSinner Oct 30, 2024
f9d4542
Update changelog
ThetaSinner Oct 30, 2024
1f6cb88
Update CHANGELOG.md
ThetaSinner Oct 31, 2024
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
27 changes: 21 additions & 6 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
matrix:
script: ["test", "bench"]
os: ["ubuntu-latest", "macos-latest"]
wasmer-feature: ["wasmer_sys", "wasmer_wamr"]
wasmer-feature: ["wasmer_sys_dev", "wasmer_sys_prod", "wasmer_wamr"]
exclude:
# TODO bench suite on macos-latest is killed by system due to running out of swap space
# All benches run fine individually
Expand All @@ -33,9 +33,9 @@ jobs:
size: 15G
- uses: actions/checkout@v4
- name: Install nix
uses: cachix/install-nix-action@v26
uses: cachix/install-nix-action@v30
- name: Setup cachix
uses: cachix/cachix-action@v14
uses: cachix/cachix-action@v15
if: ${{ ! contains(matrix.platform.runs-on, 'self-hosted') }}
with:
name: holochain-ci
Expand All @@ -47,7 +47,8 @@ jobs:
fail-fast: false
matrix:
wasmer-feature:
- "wasmer_sys"
- "wasmer_sys_dev"
- "wasmer_sys_prod"
# TODO Building with wasmer_wamr feature flag on windows is not currently working.
# See https://github.com/holochain/holochain-wasmer/issues/117
# - "wasmer_wamr"
Expand All @@ -57,7 +58,21 @@ jobs:
- uses: dtolnay/rust-toolchain@stable
with:
targets: wasm32-unknown-unknown
- name: Install LLVM
env:
LLVM_DIR: .llvm
shell: bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this also be powershell like the others?

Copy link
Member Author

Choose a reason for hiding this comment

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

That would involve me figuring out the black magic bash command that I copied from the wasmer project's CI :)

Given that we're pulling their prebuilt binary, I'd prefer to just use their command to unpack it too

https://github.com/wasmerio/wasmer/blob/main/.github/workflows/build.yml#L270

run: |
LLVM_DIR=$(pwd)/${{ env.LLVM_DIR }}
mkdir -p ${LLVM_DIR}
curl --proto '=https' --tlsv1.2 -sSf "https://github.com/wasmerio/llvm-custom-builds/releases/download/15.x/llvm-windows-amd64.tar.xz" -L -o - | tar xJv -C ${LLVM_DIR}
- name: test root
run: cargo test --release --no-default-features --features error_as_host,${{ matrix.wasmer-feature }} -- --nocapture
shell: pwsh
Copy link
Contributor

Choose a reason for hiding this comment

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

PowerShell? Why?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because this is running on Windows only and because its easier to work with than batch on Windows. With the distinction between traditional powershell and powershell core (pwsh), there is now a cross-platform and very capable scripting language option for Windows.

run: |
$env:LLVM_SYS_150_PREFIX="$(pwd)/.llvm"
cargo test --release --no-default-features --features error_as_host,${{ matrix.wasmer-feature }} -- --nocapture
- name: test
run: cargo test --release --manifest-path test/Cargo.toml --no-default-features --features ${{ matrix.wasmer-feature }} -- --nocapture
shell: pwsh
run: |
$env:LLVM_SYS_150_PREFIX="$(pwd)/.llvm"
cargo test --release --manifest-path test/Cargo.toml --no-default-features --features ${{ matrix.wasmer-feature }} -- --nocapture
128 changes: 109 additions & 19 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 11 additions & 3 deletions crates/host/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ authors = ["thedavidmeister", "[email protected]"]
edition = "2021"

[dependencies]
wasmer = { version = "=4.3.6", optional = true, default-feature = false }
wasmer-middlewares = { version = "=4.3.6", optional = true, default-feature = false }
wasmer = { version = "=4.3.6", optional = true, default-features = false }
wasmer-middlewares = { version = "=4.3.6", optional = true, default-features = false }
Copy link
Member Author

Choose a reason for hiding this comment

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

This took me way too long to find. This is a horrible little mistake to catch!

Copy link
Contributor

Choose a reason for hiding this comment

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

Why does cargo check not catch that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question! Just put this back and ran cargo check. Doesn't find it.

I guess Cargo is ignoring unknown fields? Which is something I hate with a passion. If people want custom properties, make them use x-property-name and reject all other inputs. We've known this is a good way to design software for 50 years because you can find it in specs from the 70s... /endrant :)

Copy link
Member

Choose a reason for hiding this comment

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

oof my bad! 😬

Copy link
Member Author

Choose a reason for hiding this comment

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

All good, I blame Cargo, not the typo :)


# Temporarily include a fork of wasmer from the git branch 'wamr', until it is officially released in wasmer v5
hc-wasmer = { version="=4.3.6-hc.1", optional = true, default-features = false }
Expand All @@ -30,14 +30,22 @@ crate-type = ["cdylib", "rlib"]
path = "src/lib.rs"

[features]
default = ["error_as_host", "wasmer_sys"]
default = ["error_as_host", "wasmer_sys_dev"]
debug_memory = []
error_as_host = ["holochain_wasmer_common/error_as_host"]
wasmer_sys = [
"dep:wasmer",
"dep:wasmer-middlewares",
"wasmer/sys",
]
wasmer_sys_dev = [
"wasmer_sys",
"wasmer/default",
]
wasmer_sys_prod = [
"wasmer_sys",
"wasmer/llvm",
]
wasmer_wamr = [
"dep:hc-wasmer",
"hc-wasmer/wamr"
Expand Down
11 changes: 8 additions & 3 deletions crates/host/src/module/wasmer_sys.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use wasmer::sys::BaseTunables;
use wasmer::sys::CompilerConfig;
use wasmer::wasmparser;
use wasmer::CompileError;
use wasmer::Cranelift;
use wasmer::DeserializeError;
use wasmer::Engine;
use wasmer::Module;
Expand All @@ -29,9 +28,15 @@ pub fn make_engine() -> Engine {
// @todo 100 giga-ops is totally arbitrary cutoff so we probably
// want to make the limit configurable somehow.
let metering = Arc::new(Metering::new(WASM_METERING_LIMIT, cost_function));

// the only place where the wasm compiler engine is set
let mut compiler = Cranelift::default();
compiler.canonicalize_nans(true).push_middleware(metering);
#[cfg(feature = "wasmer_sys_dev")]
let mut compiler = wasmer::Cranelift::default();
#[cfg(feature = "wasmer_sys_prod")]
let mut compiler = wasmer::LLVM::default();

compiler.canonicalize_nans(true);
compiler.push_middleware(metering);

// Workaround for invalid memory access on iOS.
// https://github.com/holochain/holochain/issues/3096
Expand Down
23 changes: 18 additions & 5 deletions flake.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,26 @@
inputs.holochain-flake.devShells.${system}.rustDev
];

# These packages and env vars are required to build wasmer on the 'wamr' branch (i.e. the hc-wasmer dependency)
packages = [
pkgs.cmake
pkgs.clang
pkgs.llvmPackages.libclang.lib
packages = with pkgs; [
# These packages and env vars are required to build Wasmer with the 'wamr' feature (i.e. the hc-wasmer dependency)
cmake
clang
llvmPackages.libclang.lib
# These packages are required to build Wasmer with the production config
llvm_15
libffi
libxml2
zlib
ncurses
];
# Used by `wamr`
LIBCLANG_PATH="${pkgs.llvmPackages.libclang.lib}/lib";
# Used by wasmer production config
shellHook = ''
# This binary lives in a different derivation to `llvm_15` and isn't re-exported through that derivation
export LLVM_SYS_150_PREFIX=$(which llvm-config | xargs dirname | xargs dirname)
export LD_LIBRARY_PATH="${pkgs.stdenv.cc.cc.lib}/lib:${pkgs.libffi}/lib:${pkgs.zlib}/lib:${pkgs.ncurses}/lib"
'';
};
};
};
Expand Down
Loading