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

Add API that uses WebAssembly ValueTypes, such as floats or reference types #532

Closed
inkeliz opened this issue May 7, 2022 · 6 comments
Closed

Comments

@inkeliz
Copy link
Contributor

inkeliz commented May 7, 2022

I notice that Call only returns []Uint64, however other types are supported, such as f32.

In that case I need to use:

ret = w.functions[s].Call(context.Background(), v...)
*(*float32)(unsafe.Pointer(&ret[0]))

Should be possible to return based on Generics or create another Value type, which have options to unwrap the value (such as Value.Int(), Value.Int32(), Value.Float32). Behind the scenes it will use unsafe, but the public-API will be easier to use.

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented May 7, 2022

Agreed. There's a lot to think about on this one because it also affects anything with ValueType support, ex Globals. Generics are a neat idea, and this also applies to a codegen solution. Meanwhile, when people start thinking about this, value binders aren't far off.

Currently, we have utilities like https://pkg.go.dev/github.com/tetratelabs/wazero/api#DecodeF64 which are documented here https://pkg.go.dev/github.com/tetratelabs/wazero/api#ValueType

These utilities can also work with globals, and buy time for a layer to be added to do things naturally.

floatG := api.DecodeF32(global.Get(ctx))

One reason we delayed doing this is seeing the bad effects of the naive approach. Ex other libraries using interface{} and using documentation or shared knowledge to know for example int32 is allowed, but not uint32 etc. Meanwhile runtime failures on things like passing strings etc. We want to make a good api in context of the main use cases and keep things as safe as possible especially to the expanding value types.

Notably, there's a long summary about things like Reference Type and v128, etc. and decide to not break the current API meanwhile #484 (comment) After a lot of thinking, we are confident we can do one of the following without breaking the current API:

  • Make a Values kind of type that can allow inputs of natural types and hook into Call via .ToUint64s or something.
  • Make a layer that internally invokes Call or Global APIs instead of routing through uint64s in a visible way
  • Add a second invocation form to function call or globals that takes a sort of Values api

Let's leave this issue open to track doing one of the above. We need to land the more complex times before solidifying an approach and also need a bit more experience with common bindings and what if anything to do about them (ex wasm-bindgen sortof stuff). It would be awesome for us to have an api that doesn't punt to things like interface{}, supports reference types etc.. I think with some thinking and experience like yours, we can work it out.

Generics is interesting, we'll have to see about how split builds work in Go, maybe a second go.mod file I guess until our floor version is 1.18. In any case, let's continue the discussion here. Meanwhile, if you want to do a playground in another repo that dispatches to current globals and function.call, that would be cool experience to help shape things.

@codefromthecrypt codefromthecrypt changed the title Calling functions with float as return Add API that uses WebAssembly ValueTypes, such as floats or reference types May 7, 2022
@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented May 7, 2022

For evaluation of whatever next API, clarifying the known consumers of our APIs:

  • low level SDK or power user: uint64s with codec functions are tolerable, but another API better
  • pure wasi: doesn't matter as "_start" has no parameters or results
  • codegen: uint64s with codec functions should be ideal as they know the value types
  • binder library: uint64s with codec functions should be ideal, if it isn't a reflective library
  • ad-hoc high level use: uint64s are only tolerable when dealing exclusively with unsigned integer types

When we do things, we'll remember that ad-hoc use shouldn't require things like codegen or a binder library, though some of those doing ad-hoc high level use may be better off using one of these.

I'm not entirely sure which of these would be Generics, so that's where I'd love to get some feedback from some experiments on this. Probably more than one..

If I left out important things, please comment back!

@inkeliz
Copy link
Contributor Author

inkeliz commented May 7, 2022

What do you think about something like:

type Values []Value

func (v Values) Scan(out ...any) {
	if len(out) > len(v) {
		return // Too much outputs
	}
	for i, o := range out {
		switch o := o.(type) {
		case *int64:
			*o = v[i].Int64()
		case *uint64:
			*o = v[i].Uint64()
		case *float32:
			*o = v[i].Float32()
		case *float64:
			*o = v[i].Float64()
		}
	}
}

type Value uint64

func (v Value) Uint64() uint64 { return uint64(v) }

func (v Value) Int64() int64 { return int64(v) }

func (v *Value) Float32() float32 { return *(*float32)(unsafe.Pointer(v)) }

func (v *Value) Float64() float64 { return *(*float64)(unsafe.Pointer(v)) }

The function will return Values instead of []uint64. Actually, we can use *(*[]Values)(unsafe.Pointer(&r)) assuming r is []uint64, so everything internal will remain unchanged. It only changes the "front-end" of Wazero (the Call function).


That construction can be used in two ways:

ret, _ := w.functions[s].Call(context.Background(), v...)

// Get single value:
myFloat = ret[1].Float32()

// Retrieve everything:
ret, _ := w.functions[s].Call(context.Background(), v...)

var myFloatA, myFloatB = float32(0), float32(0)
ret.Scan(&myFloatA, &myFloatB)

In the case of Scan()function, would be better if Call suppresses the error, and give only Values. Something like:

var myFloatA, myFloatB = float32(0), float32(0)
if err := w.functions[s].Call(context.Background(), v...).Scan(&myFloatA, myFloatB) {
}

So, it could be:

type Values struct {
	values []Value
	err error
}

func (r *Values) Values() ([]Value, error) {
	if r.err != nil {
		return nil, r.err
	}
	return r.values, nil
}

func (r *Values) Scan(out ...any) error {
	if r.err != nil {
		return r.err
	}
	if len(out) > len(r.values) {
		return errors.New("function signature not match") 
	}
	for i, o := range out {
		switch o := o.(type) {
		case *int64:
			*o = r.values[i].Int64()
		case *uint64:
			*o = r.values[i].Uint64()
		case *float32:
			*o = r.values[i].Float32()
		case *float64:
			*o = r.values[i].Float64()
		}
	}
	return nil
}

The issue if this last method is for "void" functions, since calling Values() for void is counter-intuitive, and that is the only way to get errors. Anyway, just ideas. 🧐

@codefromthecrypt
Copy link
Contributor

this is an interesting view. bear in mind, we deferred doing things like this because 128 is two uint64s. that said in the approach you are suggesting above, the caller isn't passing a type like that. It feels like when someone does this, they have a signature in mind (and don't want or don't yet have codegen).

this #264 was an attempt to be less squishy, ex defining before a function is called, its parameters, vs possibly failing at runtime. Since the function exposes its param and result types, you can imagine the overhead of scanning is somewhat fixed. OTOH there's some glitches mentioned in the issue, hence it being punted.

In any case any of these approaches can be done externally as long as we don't try to change the uint64 to another type of uint64 (I think the above in the example isn't necessary to define a new type as you still need the function sig. It is mainly syntactic sugar right? So, I was hoping someone can try on own vs needing an internal change a while as usually there are glitches that come out when doing that.

@codefromthecrypt
Copy link
Contributor

food for thought: while we're discussing is the caller side, we've already implemented the callee side (host functions). This code will change for v128 for example (consuming 2 64bit blocks for a i128). When we end up baking in a callee-side means to get go values, we'd likely only support the exact same type mappings that exist on the host side. That makes docs easier to reason with and intuition possible from knowing one side or the other https://github.com/tetratelabs/wazero/blob/main/internal/wasm/gofunc.go#L76-L133

@codefromthecrypt
Copy link
Contributor

https://github.com/stealthrocket/wazergo is where to go for ergonomics now

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

No branches or pull requests

2 participants