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

internal/wasmtools: vendor wasm-tools with WebAssembly #272

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ydnar
Copy link
Collaborator

@ydnar ydnar commented Dec 15, 2024

Iteration of #252, squashed and rebased.

  • Embeds gzipped wasm file, with lazy decompression
  • Uses Wazero compilation cache (first tries on-disk, then falls back to in-memory)
  • Adds internal/cmd/wasm-tools as a thin CLI wrapper around package internal/wasmtools

@ydnar ydnar requested a review from Mossaka December 15, 2024 18:11
@ydnar ydnar self-assigned this Dec 16, 2024
@ydnar
Copy link
Collaborator Author

ydnar commented Dec 16, 2024

Cool. Time for rebase and squash!

@ydnar ydnar changed the title internal/wasmtools: try gzipped wasm-tools.wasm internal/wasmtools: use wasm-tools.wasm.gz Dec 17, 2024
Copy link
Member

@Mossaka Mossaka left a comment

Choose a reason for hiding this comment

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

LGTM!

@ydnar ydnar changed the title internal/wasmtools: use wasm-tools.wasm.gz internal/wasmtools: vendor and run wasm-tools with WebAssembly Dec 18, 2024
@ydnar ydnar changed the title internal/wasmtools: vendor and run wasm-tools with WebAssembly internal/wasmtools: vendor wasm-tools with WebAssembly Dec 18, 2024
@ydnar ydnar marked this pull request as ready for review December 19, 2024 03:19
all: replace submodule with Cargo-managed wasm-tools CLI

* add wasm-tools as git submodule
* update Makefile to compile wit/wasm-tools.wasm
* use wazero to execute wasm-tools.wasm
* Makefile: add a build command
* Commit the wasm-tools.wasm file
* Makefile: replace path with a variable for the name of the target
* wit, Makefile: move wasmtools.go to internal/wasmtools
* Remove wasmtools submodule

Signed-off-by: Jiaxiao Zhou (Mossaka) <[email protected]>

internal/wasmtools: optimize for size

Adding lto, opt-level=z and other options to the cargo.toml reduces the wasm-tools.wasm binary size from 7.8 MBi to 3.5 MBi

Signed-off-by: Jiaxiao Zhou (Mossaka) <[email protected]>

wit/testdata_test: implement code review comment

Signed-off-by: Jiaxiao Zhou <[email protected]>

internal/wasmtools, wit: change the type of fsMap in Executor.Run to map[fs.FS]string

Signed-off-by: Jiaxiao Zhou <[email protected]>

Makefile: change dir to '.'

Signed-off-by: Jiaxiao Zhou <[email protected]>

internal/wasmtools, wit: enhance API, disable TinyGo tests, and improve naming

* Rename to WasmTools to Instance
* Add build tags to disable wasmtools when run under TinyGo
* wasmtools_tinygo: New returns a new error
* Rename Executor to Runner

Signed-off-by: Jiaxiao Zhou <[email protected]>

wit/testdata_test: change t.Skip to t.Log since Skip is not supported in TinyGo

Signed-off-by: Jiaxiao Zhou <[email protected]>

internal/wasmtools: add comments to improve readability for the Instance API

Signed-off-by: Jiaxiao Zhou <[email protected]>

CHANGELOG: Update with wasm-tools integration details

Signed-off-by: Jiaxiao Zhou <[email protected]>

internal/wasmtools, Makefile: add gzip compression for wasm-tools.wasm and update Makefile

This adds gzip compression for the wasm-tools.wasm to further reduce the size to 1.2M from 3.5M.
Added the wasm file to the .gitignore and thus the remote repo will only have the gzip file.
Updated Makefile to zip and unzip the Wasm module.

Signed-off-by: Jiaxiao Zhou <[email protected]>

.github: Add set-up wasm-tools.wasm to the test.yaml and release.yaml

Signed-off-by: Jiaxiao Zhou <[email protected]>
.github/workflows: remove some (now) unneeded steps

.gitignore: condense internal/wasmtools/.gitignore

internal/wasmtools: use single quotes, add Rust edition

Makefile: update order of wasm-tools.wasm.\* targets)

internal/wasmtools: use gzipped wasm-tools.wasm.gz with sync.Once

internal/wasmtools: rebuild wasm-tools.wasm.gz

CHANGELOG: wordsmith the wasm-tools and Wazero update

internal/wasmtools, wit: remove optional name arg

Removing this did not seem to affect tests.

internal/wasmtools: leave timeout to the caller

Makefile: reorder gzip targets

internal/wasmtools: oops

Makefile: next try

wit: remove check for wasm-tools in PATH

internal/wasmtools, wit: swap key and value types for fsMap

Not all fs.FS are hashable, but all strings are, so use map[string]fs.FS instead.

wit/bindgen: use internal/wasmtools to run wasm-tools in WebAssembly

internal/wasmtools: use wazero.CompilationCache

This speeds up wit/bindgen tests 10x

internal/wasmtools: fix TinyGo impl

internal/wasmtools: remove Runner interface

This wasn’t used anywhere, so removing.

internal/wasmtools: change Run() to accept optional stdout and stderr

internal/wasmtools: move args to varadic trailing arg in Run

wit: (*testing.common).Errorf does not support error-wrapping directive %w

internal/wasmtools, wit/bindgen: additional cleanups for TinyGo and WASI

.github/workflows/test: TinyGo needs wasm-tools for -target=wasip2

.github/dependabot: add internal/wasmtools for Cargo updates

cmd/wit-bindgen-go, internal/{module,witcli}: extract module helper into new package

internal/wasmtools: try to use disk-based compilation cache in TMPDIR

internal/cmd/wasm-tools: add Wazero-based wasm-tools executable

This removes the requirement for wasmtime in the Makefile

Makefile: use Wazero-based internal/cmd/wasm-tools executable instead of Wasmtime

.github/workflows/test: ignore all Markdown files

internal/wasmtools: rebuild wasm-tools.wasm.gz

internal/{cmd/wasm-tools,wasmtools}, wit: default mapping of ./ and / directories

.github/workflows: install wasm wrapped in Go version of wasm-tools

all: revert 26c1e72; Wazero directory handling tricky to manage

Wazero flattens ./ into '', which is treated as /, overriding the / mapping.

Makefile: just use wasm-tools

It’s faster, and works with arbitrary paths.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants