internal/wasmtools: run wasm-tools.wasm in embedded Wazero runtime by Mossaka · Pull Request #252 · bytecodealliance/go-modules

Mossaka · 2024-11-28T21:06:39Z

add wasm-tools as git submodule
update Makefile to compile wit/wasm-tools.wasm
use wazero to execute wasm-tools.wasm

what I did to optmize the wasm-tools Wasm binary size

The current wasm-tools.wasm size is 3.5M

Cargo.toml setup

[profile.release]
lto = true # compile with link-time optimization
codegen-units = 1 # compile with a single codegen unit
opt-level = "z" # optimize for size
panic = "abort" # panic=abort will remove the panic code
strip = true # strip the debug information

disable default features and only added "component" feature

wasm-tools = { version = "1.221.0", default-features = false, features = [
	"component",
]}

gzip compress it

Let me know if you know more ways to cut down the size of the binary!

ydnar · 2024-11-28T21:26:10Z

 .PHONY: $(wit_files)
-$(wit_files):
-	wasm-tools component wit -j --all-features $@ > $@.json
+$(wit_files): wit/wasm-tools.wasm


Let's put the Wazero and wasm-tools dependency into internal/ or x/.

As it stands, this creates a new problem where consumers of package cm will have an implicit dependency on Wazero. We need to figure out a way around this.

ydnar · 2024-11-29T01:21:01Z

+
+// Executor is an interface for running Wasm modules.
+type Executor interface {
+	Run(ctx context.Context, args []string, stdin io.Reader, fsMap map[string]string) (stdout *bytes.Buffer, stderr *bytes.Buffer, err error)


Can this be an fs.FS?

Are you referring to the fsMap? yes it can be defined as map[fs.FS]string

ydnar · 2024-12-11T01:09:53Z

+	}
+	config = config.WithFSConfig(fsConfig)
+
+	_, err = w.runtime.InstantiateModule(ctx, w.module, config)


Does this run in a goroutine?

I don't think so. I've added a 10s timeout to it so that it won't run forever.

ydnar · 2024-12-11T01:11:17Z

+		WithSysWalltime().
+		WithArgs(append([]string{"wasm-tools.wasm"}, args...)...)
+	if name != nil {
+		config = config.WithName(*name)


Is the name arg necessary?

It is used in the test.

What is the purpose of the name? Does it matter to the caller?

If you want to run wasm-tools.wasm multiple times, the Run() would fail without the name parameter because the wazero module configuration defaults the "name" to what was decoded from the name section and duplicate names are not allowed in a single Runtime.

ydnar · 2024-12-11T01:12:23Z

+	return w.runtime.Close(ctx)
+}
+
+func (w *WasmTools) Run(ctx context.Context, args []string, stdin io.Reader, fsMap map[string]string, name *string) (stdout *bytes.Buffer, stderr *bytes.Buffer, err error) {


Instead of fsMap map[string]string, how about a varadic dirs ...string at the end of the argument list?

Do you not want to distinguish host and guest paths?

If we use dirs ...string we will have to set the guest path as the same as the host path. i.e. fsConfig.WithDirMount(dir, dir)

ydnar · 2024-12-11T01:14:38Z

I think we’ll need to add build tags to disable this functionality when run under TinyGo (e.g. have Run always return nil, nil, error).

ydnar · 2024-12-15T00:46:59Z

@Mossaka can you rebase and squash some commits?

* 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) <duibao55328@gmail.com>

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) <duibao55328@gmail.com>

Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

…map[fs.FS]string Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

…ve 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 <duibao55328@gmail.com>

… in TinyGo Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

…nce API Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

Mossaka · 2024-12-15T06:46:59Z

Added more docs and updated CHANGELOG

…m 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 <duibao55328@gmail.com>

Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

ydnar

What's the propose of the gzip code? Groundwork for gzipping the binary output?

ydnar · 2024-12-15T08:03:39Z

          go-version-file: go.mod

+      - name: Set up wams-tools.wasm
+        run: make internal/wasmtools/wasm-tools.wasm


Do we want to do this?

The artifact should already be committed, and this adds a Rust dependency / build step to the release workflow.

ydnar · 2024-12-15T09:29:27Z

+)
+
+//go:embed wasm-tools.wasm
+var wasmTools []byte


Can this be the gz file?

ydnar · 2024-12-15T09:30:34Z

          version: ${{ env.wasm-tools-version }}

+      - name: Set up wams-tools.wasm
+        run: make internal/wasmtools/wasm-tools.wasm


Is this necessary?

Should we delete the step above?

ydnar · 2024-12-15T09:31:01Z

        with:
          version: ${{ env.wasm-tools-version }}

+      - name: Set up wams-tools.wasm


Necessary? Delete this and the step above?

ydnar · 2024-12-15T22:09:39Z

Had an idea: internal/cmd/wasm-tools-go that wraps internal/wasmtools with a CLI. No need to have Wasmtime installed.

ydnar · 2024-12-18T05:48:07Z

Implemented in #272

Mossaka · 2024-12-18T06:30:06Z

Close for #272

ydnar reviewed Nov 28, 2024

View reviewed changes

Mossaka commented Nov 28, 2024

View reviewed changes

Comment thread wit/wasm-tools.wasm Resolved

ydnar reviewed Nov 28, 2024

View reviewed changes

Comment thread wit/testdata_test.go Resolved

ydnar mentioned this pull request Dec 8, 2024

Discussion: Strategy to remove shell-exec from compilation path #202

Open

ydnar reviewed Dec 9, 2024

View reviewed changes

Mossaka mentioned this pull request Dec 9, 2024

proposal: put package cm in its own module #261

Closed

ydnar reviewed Dec 11, 2024

View reviewed changes

Comment thread internal/wasmtools/wasmtools.go Resolved

ydnar reviewed Dec 11, 2024

View reviewed changes

Comment thread internal/wasmtools/wasmtools.go Resolved

ydnar reviewed Dec 11, 2024

View reviewed changes

Comment thread internal/wasmtools/wasmtools.go Resolved

ydnar changed the title ~~Run wasm-tools as Wasm file in embedded wazero runtime~~ internal/wasmtools: run wasm-tools.wasm in embedded Wazero runtime Dec 11, 2024

Mossaka marked this pull request as ready for review December 11, 2024 06:48

ydnar reviewed Dec 11, 2024

View reviewed changes

Comment thread wit/wit_test.go Resolved

Mossaka force-pushed the mossaka/wasmTools branch from 13978f5 to c41c661 Compare December 12, 2024 00:49

Mossaka requested a review from ydnar December 12, 2024 05:43

ydnar reviewed Dec 12, 2024

View reviewed changes

Comment thread internal/wasmtools/types.go Resolved

Mossaka force-pushed the mossaka/wasmTools branch from dc61352 to aba1c33 Compare December 15, 2024 06:35

Mossaka added 7 commits December 14, 2024 22:36

internal/wasmtools: optimize for size

787ab7e

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) <duibao55328@gmail.com>

wit/testdata_test: implement code review comment

0e1c947

Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

internal/wasmtools, wit: change the type of fsMap in Executor.Run to …

1caa745

…map[fs.FS]string Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

Makefile: change dir to '.'

10f2270

Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

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

ff76f42

… in TinyGo Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

Mossaka force-pushed the mossaka/wasmTools branch from aba1c33 to ff76f42 Compare December 15, 2024 06:37

internal/wasmtools: add comments to improve readability for the Insta…

b5ea977

…nce API Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

CHANGELOG: Update with wasm-tools integration details

8f0c740

Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

Mossaka added 2 commits December 14, 2024 23:00

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

1aa6f6e

Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

ydnar reviewed Dec 15, 2024

View reviewed changes

ydnar mentioned this pull request Dec 15, 2024

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

Merged

Mossaka closed this Dec 18, 2024

Conversation

Mossaka commented Nov 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

what I did to optmize the wasm-tools Wasm binary size

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ydnar commented Dec 11, 2024

Uh oh!

Uh oh!

Uh oh!

ydnar commented Dec 15, 2024

Uh oh!

Mossaka commented Dec 15, 2024

Uh oh!

ydnar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ydnar commented Dec 15, 2024

Uh oh!

ydnar commented Dec 18, 2024

Uh oh!

Mossaka commented Dec 18, 2024

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Mossaka commented Nov 28, 2024 •

edited

Loading