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

Use obj.func.Invoke() instead of obj.Call() #12

Open
deferred-impact opened this issue Nov 14, 2023 · 5 comments
Open

Use obj.func.Invoke() instead of obj.Call() #12

deferred-impact opened this issue Nov 14, 2023 · 5 comments

Comments

@deferred-impact
Copy link

deferred-impact commented Nov 14, 2023

Call() has terrible performance (for a certain definition of "terrible" - i'm writing a WebGL thing and it's bad enough to matter). It has to pass the method name from wasm to JS each time, and decode it from bytes into a UTF8 string each time: golang/go#32591 (comment)

A potential solution would be to cache functions for each object (either on creation or on invocation). Properties referencing functions in JS web API objects tend to be readonly (in fact i'm fairly sure they always are, would love to find a counterexample), so there shouldn't be any issues with an outdated value in cache. Then instead of using Call() on the object, use an Invoke() on the cached function.

(Don't forget to Bind() the function first, like i just did)

@janpfeifer
Copy link
Collaborator

That makes lots of sense. If I understood correctly (it has been a while since I looked at the code) the issue of caching is that in some rare (?) scenarios, I suppose some end-user code may be dynamically changing the JS function, and the cache would hit an older definition, breaking the code (and compatibility). But if the gains are that significant, probably worth it. There should be a way to deliberate bypass the cache -- so if anyone bumps into this issue, at least it is fixable.

Would you like to take a stab at creating a PR for this ?

ps.: Btw, I use gowebapi with WebGL to power a small game (prototype) with thousands of sprites, at a decent FPS. But I did manage to make it in very few calls, passing very large arrays -- fewer calls made a huge difference, I wonder if this had something to do with it.

@deferred-impact
Copy link
Author

If I understood correctly (it has been a while since I looked at the code) the issue of caching is that in some rare (?) scenarios, I suppose some end-user code may be dynamically changing the JS function, and the cache would hit an older definition, breaking the code (and compatibility).

Yes, that is the concern with caching. But gowebapi is not wrapping arbitrary end-user code, it's wrapping the web API, which never(?) dynamically changes functions on objects. So it should be fine to use it here. Perhaps we could restrict this optimization for WebGL and other things that have to be called thousands of times per second

Would you like to take a stab at creating a PR for this ?

Sure, i will try to implement it sometime this week

Btw, I use gowebapi with WebGL to power a small game (prototype) with thousands of sprites, at a decent FPS. But I did manage to make it in very few calls, passing very large arrays -- fewer calls made a huge difference, I wonder if this had something to do with it.

Indeed, i just spent this morning looking into ways to boost performance. I had batching for shaders but not for meshes and materials - implementing it for the latter gave a massive performance boost for this scene. But there could be situations where batching doesn't do enough, so this optimization is still worthwhile, i think

@janpfeifer
Copy link
Collaborator

For what it's worth, I recall having to do something specialized for writing those JS arrays with the batched everything. That also made a large difference (this was a few years ago).

Also, I recall converting arrays from Go to JS had to be done careful. I think we fixed it in gowebui though, although the Float32ToJs still uses js.Global().Get("Float32Array") which I assume could be cached as well ? (if this becomes an issue to you)

Apologies, I intend to get back to it another time, but I can't right now.

@deferred-impact
Copy link
Author

deferred-impact commented Nov 14, 2023

I rewrote several functions but the array conversion functions managed to evade my attention so far. In theory it would help a lot as well, since these do a lot of dynamic string accesses. Sadly caching doesn't quite work here due to how they are written, so i ended up with a slightly different approach:

func perf_SliceToJs[T ~uint16 | ~float32](r *WebGLRenderer, src []T) js.Value {
	var s T
	size := binary.Size(s)
	name := "Uint8Array"
	array := r.getCachedJsFn(js.Global(), name).New(len(src) * size)
	head := (*reflect.SliceHeader)(unsafe.Pointer(&src))
	head.Len *= size
	head.Cap *= size
	data := *(*[]byte)(unsafe.Pointer(head))
	js.CopyBytesToJS(array, data)
	runtime.KeepAlive(src)

	fnName := ""
	switch any(s).(type) {
	case uint16:
		fnName = "toUint16Array"
	case float32:
		fnName = "toFloat32Array"
	default:
		panic(fmt.Sprintf("Unknown type %T", s))
	}
	return r.getCachedJsFn(js.Global(), fnName).Invoke(array)
}
globalThis.toUint16Array = (arr) => {
  return new Uint16Array(arr.buffer, arr.byteOffset, arr.byteLength / 2)
}
globalThis.toFloat32Array = (arr) => {
  return new Float32Array(arr.buffer, arr.byteOffset, arr.byteLength / 4)
}

In practice however, did not see any meaningful change in performance. Still going to keep these changes, though, even the smallest improvement helps

What we really need, though, is for the language spec to support some kind of static_string type, which it could then use for optimizing JS calls by storing all these strings on JS side, and instead of passing method names as strings, it could then simply pass an index for this static string.

Also speaking of arrays, i've been thinking: in the context of WebGL at least, nothing really stops us from just... taking a DataView of a part of the go wasm memory, and then passing that directly to WebGL, without the js.CopyBytesToJS() call. It would only work for specific functions (those which already copy data as part of their spec - the way most WebGL functions do) but it could be a big improvement.

Obviously though both of these would require changes to the compiler, so they're way out of scope of gowebapi. Just sharing some ideas.

@janpfeifer
Copy link
Collaborator

Agreed, wherever there are string hashing/comparison involved, there goes some "friction" time ...

When I come back to my project, I'll benchmark your implementation. If mine is faster, I'll copy&paste it here.

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