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

Complete minimum viable WASI wasi_snapshot_preview1 #271

Closed
11 of 17 tasks
codefromthecrypt opened this issue Feb 21, 2022 · 13 comments · Fixed by #389
Closed
11 of 17 tasks

Complete minimum viable WASI wasi_snapshot_preview1 #271

codefromthecrypt opened this issue Feb 21, 2022 · 13 comments · Fixed by #389

Comments

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Feb 21, 2022

Here is the status of our implementation of wasi_snapshot_preview1

  • args_get
  • args_sizes_get
  • environ_get
  • environ_sizes_get
  • clock_time_get (partially implemented)
  • poll_oneoff (TinyGo needs this)
  • fd_close
  • fd_fdstat_get (partially implemented)
  • fd_prestat_get
  • fd_prestat_dir_name
  • fd_read
  • fd_seek @nullpo-head working on
  • fd_write
  • path_open (partially implemented)
  • fd_write
  • random_get
  • proc_exit

The above should only be checked when all parts of the API are implemented or we know what parts we won't implement. For example, don't tick a box if an API is skipping parameters unless we actively rule out a parameter. Definitely no box can be ticked until it is properly documented and tested in wasi.go and wasi_test.go

Notes: Per #263 there are some problems in wasi_snapshot_preview1, so it may not make sense to burn time on implementing all features and constants of it. That said, there are code using it, notably TinyGo, and we should complete a minimum viable subset of the API and its features based on what users want.

@pims
Copy link
Contributor

pims commented Feb 21, 2022

proc_raise would be nice to be able to use Grain Lang compiled webassembly.

@nullpo-head
Copy link
Contributor

proc_raise would be nice to be able to use Grain Lang compiled webassembly.

Hi @pims, thanks for the feedback! Could you share some references on how Gain Lang uses proc_raise, please? I personally tried Grain but I couldn't see the compiled webassembly import proc_raise, at least for hello-world.

@pims
Copy link
Contributor

pims commented Feb 24, 2022

@nullpo-head

Given the following grain program:

import Process from "sys/process"
import Array from "array"

match (Process.env()) {
 Ok(arr) => Array.forEach(print, arr),
 Err(err) => throw err
}

The following go code:

store, err := wazero.NewStoreWithConfig(&wazero.StoreConfig{
		ModuleToHostFunctions: map[string]*wazero.HostFunctions{
			wasi.ModuleSnapshotPreview1: wazero.WASISnapshotPreview1WithConfig(wasiConfig),
		},
	})
	if err != nil {
		log.Fatal(err)
	}
	// StartWASICommand runs the "_start" function which is what TinyGo compiles "main" to
	_, err = wazero.StartWASICommand(store, s.mod)
	if err != nil {
		log.Fatal(err)
	}

outputs:

2022/02/24 14:24:08 resolve imports: proc_raise: "proc_raise" is not exported in module "wasi_snapshot_preview1"
exit status 1

Environment:

grain --version
Grain cli 0.4.7
Grain compiler 0.4.6

go 1.17
github.com/tetratelabs/wazero v0.0.0-20220224115833-c09d4eef3f94

Hopefully this helps! Let me know if you'd like me to do more testing. Thanks!

@codefromthecrypt
Copy link
Contributor Author

thanks @pims. are your functions like this running in a single process or shared within a process. If the latter, I suspect we would need to implement #293 to allow safe deinstantiation (without clobbering the host process). We would support both, but curious if you immediately need process-scoped or module-scoped

@pims
Copy link
Contributor

pims commented Feb 24, 2022

@codefromthecrypt right now, I parse the wasm binary file once, and instantiate a new store for each incoming nats.io message I receive. Environment and Args being different for each message.

@codefromthecrypt
Copy link
Contributor Author

thanks @pims, that helps prioritize!

@codefromthecrypt
Copy link
Contributor Author

@nullpo-head
Copy link
Contributor

Grain seems to import all WASI APIs regardless of them being used or not, so, implementing proc_raise is not the solution to support Grain apps in wazero. Instead, I opened an issue on Grain side to reduce the unused imports.
grain-lang/grain#1146

I've also found that wasmtime nor wasmer implement proc_raise. Because of those reasons, now I think we can remove proc_raise from the minimum viable list.

@nullpo-head
Copy link
Contributor

Oh I missed that there are new comment about AssemblyScript on proc_raise. I still think proc_raise is unnecessary for AssemblyScript either. If I understand correctly, AssemblyScript's abort itself is their custom runtime API, and unrelated to WASI's abort signal. abort has platform-dependent implementations (e.g for JS, for WASI). And abort implementation for WASI doesn't use proc_raise, but does proc_exit.

@pims
Copy link
Contributor

pims commented Mar 2, 2022

@nullpo-head would it make sense to at least have a stub for all wasi preview snapshot_1 functions so Grain/Other compiled wasm files run in wazero?

The same Grain program runs fine in wasmtime for instance.

@codefromthecrypt
Copy link
Contributor Author

@nullpo-head would it make sense to at least have a stub for all wasi preview snapshot_1 functions so Grain/Other compiled wasm files run in wazero?

The same Grain program runs fine in wasmtime for instance.

My 2p is that regardless of whether other compilers actually claim functions they don't use, we can do this for you even if grain addresses the issue. I'll offer the labor. Through this, we may find other users who need something more than stubbing!

@codefromthecrypt
Copy link
Contributor Author

As of last night, all WASI functions are stubbed.

codefromthecrypt pushed a commit that referenced this issue Mar 17, 2022
This formalizes that completing all WASI APIs is an anti-goal as the
abstraction includes abandoned features.

Fixes #271

Signed-off-by: Adrian Cole <adrian@tetrate.io>
codefromthecrypt added a commit that referenced this issue Mar 17, 2022
)

This formalizes that completing all WASI APIs is an anti-goal as the
abstraction includes abandoned features.

Fixes #271

Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt
Copy link
Contributor Author

Closing this to the top-level README as it is a non-goal to complete all WASI snapshot APIs. Any APIs folks need should become new issues! Thanks for all the feedback.

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 a pull request may close this issue.

3 participants