syscall/js: remove Wrapper type to avoid extreme allocations and improve performance #44006

finnbear · 2021-01-30T05:22:26Z

Introduction

It is truly remarkable what can be accomplished by compiling Go to WebAssembly (wasm), but virtually all such applications are being held back by the combination of two factors:

Every function call to JS that passes arguments makes multiple, unavoidable Go heap allocations
The Go garbage collector then blocks the main, and only, thread for a large fraction of a second

As the title of this proposal suggests, the performance problems can be solved if the js.Wrapper type is removed. Removal of existing features from the standard library should not be taken lightly, but this particular removal is both allowable (syscall/js is exempt from Go's compatibility promise) and necessary given the data.

Why `Wrapper` matters

At the core of every call to JS, syscall/js uses its ValueOf function to convert Go arguments to their JS equivalents. This conversion is implemented as a switch statement:

func ValueOf(x interface{}) Value {
	switch x := x.(type) { // some cases omitted
	case Value:
		return x
	case Wrapper:
		return x.JSValue() // the problem
	case nil:
		return valueNull
	case bool:
		if x {
			return valueTrue
		} else {
			return valueFalse
		}
	case int:
		return floatValue(float64(x))
	case unsafe.Pointer:
		return floatValue(float64(uintptr(x)))
	case float64:
		return floatValue(x)
	case string:
		return makeValue(stringVal(x))
	default:
		panic("ValueOf: invalid value")
	}
}

One of the cases is the Wrapper interface. This was intended to allow any type implementing the JSValue method to be easily passed as an argument to a JS method. However, as JSValue may consist of an arbitrary pointer receiver, the compiler must assume the worst:

type BadWrapper struct {
    Value js.Value
}

var escapeRoute *BadWrapper

// Implements js.Wrapper
func (this *BadWrapper) JSValue() js.Value {
    escapeRoute = this // escape to heap
    return this.Value
}

This means that the argument to ValueOf, which includes any argument passed to any JS function, is guaranteed to escape to the heap.

Data

Data is reported as number of heap allocations per unit of work.

Optimizations (cumulative)	Frame of game (best case)	Frame of game (worst case)	Typical canvas/WebGL call	`[]byte` copy to JS	`string` manipulation	Garbage collector effect on gameplay
Control (no optimizations)	858	40000	3	1	3	Frequent stutters
`//go:noescape`	829	untested	3	0	3	Frequent stutters
`makeArgs` slices on stack (See #39740)	383	untested	1	0	3	Less-frequent stutters
Unexport `Wrapper.JSValue()`	383	untested	1	0	3	No change
Remove `Wrapper` type	62*	300*	0	0	2	Not bad, especially after other allocations are reduced

*on the order of the number of allocations that happen unrelated to syscall/js

Data collection method

Debug output from my online game, implemented in Go
Micro-benchmarks: main.go
Entire patch to syscall/js/js.go: js.go.patch

Context

Preliminary discussion of this proposal has has already taken place in #39740. The consensus there was that removing Wrapper is the only way to sustainably call JS functions in a performance-critical setting. Given that syscall/js is a low level package, it make sense to prioritize performance over the ease of use afforded by Wrapper.

More context on my actual use case

My game's client is written in Go, compiled to WebAssembly, uses WebGL 2.0 APIs, and is intended to run at 60 frames per second.
The WebGL API requires many function calls, each mainly taking integer and floating point parameters.
I have observed to Go garbage collector running every 3-5 seconds under normal circumstances.
I have observed the Go garbage collector running over 10 times in a single frame in a worst-case-scenario
I have observed the Go garbage collector taking 300+ milliseconds to run

The text was updated successfully, but these errors were encountered:

hajimehoshi · 2021-04-13T06:44:05Z

CC @neelance @agnivade

neelance · 2021-04-18T09:38:54Z

I support this proposal. The Wrapper type was only added for convenience and performance trumps convenience for such a low-level package as syscall/js.

The author of the Wrapper type also supports the removal: #39740 (comment)

@rsc Please consider this proposal for the proposal process.

The existence of the Wrapper interface causes unavoidable heap allocations in syscall/js.ValueOf. See the linked issue for a full explanation. Fixes golang#44006.

awfulcooking · 2021-07-29T20:34:35Z

It is truly remarkable what can be accomplished by compiling Go to WebAssembly

I have a program which animates an LED matrix, and transfers frames @ 60fps, to a JS IDE for rendering.

Receiving binary frames over a WebSocket, the IDE starts to stutter around 9000 - 10,000 pixels.

Receiving binary frames from embedded Go to JS, the server both rendering the animation and copying the bytes to a JS buffer within the context of a requestAnimationFrame, the IDE can handle 10,000 pixels like butter.. without breaking a sweat!

Reading this proposal, and hearing of performance improvements in 1.17, gives hope that even better performance is possible!

Exciting times for the web platform and Go both.

neelance · 2021-09-09T17:47:01Z

Any chance to get this into the proposal process this cycle? 🙂

rsc · 2021-09-15T17:16:07Z

Do we have any numbers about how often Wrapper is used today? Did I miss them above?

rsc · 2021-09-15T18:07:42Z

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

finnbear · 2021-09-15T19:06:38Z

Do we have any numbers about how often Wrapper is used today? Did I miss them above?

You didn't miss anything; the original proposal didn't make an effort to enumerate usage of js.Wrapper, but it probably should have.

Anyway, searching https://github.com/search?q=%22js.Wrapper%22+language%3AGo&type=Code returns 375 code results.

For comparison, the close alternative of js.Value returns 2,790 results and syscall/js returns 16,253 results. Based on these numbers alone, approximately 2-13% of the open-source Go syscall/js ecosystem is affected. I attribute this low percentage to the fact that it is easy to develop applications without js.Wrapper (as an interface, it is mainly a convenience for library developers).
The vast majority of the js.Wrapper results are false positives (i.e. from projects that use JS and "wrappers," but not syscall/js). Many of the code results are in comments, so the actual usage of the js.Wrapper type is less. I made no attempt to check what fraction of the js.Value results are false positives, since there are too many.

Here is a (potentially incomplete) list of projects that would (at least temporarily) break (they reference the js.Wrapper type). The list starts with what seemed to be "the big ones" and then proceeds in no particular order. Of these projects, some rely on js.Wrapper simply to call .JSValue() themselves, instead of actually passing to Call, New, or Invoke (which could be adapted to passing around js.Value instead).

It would be interesting to get the perspectives of the maintainers of these projects.

Finally, since it's been 8 months since this proposal began, I have more context on my particular use cases: Development of the Go version of my online game has stopped, and development on a Rust port has started (but stalled). I also started developing a new online Game based on Rust wasm. Rust seems to be a better fit for performance critical wasm. While I would not personally benefit from this proposal any more, I think it would help close the Go<->Rust wasm gap by eliminating the major performance cliff.

hajimehoshi · 2021-09-16T00:21:10Z

Anyway, searching https://github.com/search?q=%22js.Wrapper%22+language%3AGo&type=Code returns 375 code results.

https://github.com/search?q=%22syscall%2Fjs%22+%22js.Wrapper%22+language%3AGo&type=Code I searched this with syscall/js and the number of results was 55.

As this is an interface, we should find the code that defines a method JSValue() js.Value, as you have already mentioned.

rsc · 2021-09-22T17:30:38Z

It sounds like we need more data about the potential impact here.

rsc · 2021-10-06T21:21:05Z

I would still feel better with more data, but because this is in syscall and in wasm, which is not heavily used, and because literally no one has objected to doing this, it seems okay to do it and see if anyone comes forward.

rsc · 2021-10-06T22:07:08Z

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

cherrymui · 2021-10-06T22:47:35Z

This is not really about removing Wrapper type, which I have no objection. But I don't really understand the escaping part. What exactly escapes, and can be avoided if Wrapper is removed? And if a value is passed to JavaScript how do we know it is safe to not escape? I.e. be sure that JavaScript doesn't hold the value or use it in some asynchronous way? Thanks.

finnbear · 2021-10-07T00:06:48Z

What exactly escapes

From Go's perspective, the parameter x to ValueOf escapes because its data potion can escape. All interfaces in Go operate via indirection. They have one pointer to type information and one pointer to data. The interface itself may be copied by value, but that data must live on the stack or heap. If there is any chance the interface or it's data could escape off the stack, the Go compiler must ensure that the interface's data lives on the heap (by allocating the data that goes into the interface on the heap, e.g. when an argument is passed to a function that takes an interface parameter).

and can be avoided if Wrapper is removed?

As outlined in my original proposal, the case Wrapper is the deciding factor for whether the interface's data can escape. Because Wrapper is, in itself, an interface, its dynamically-dispatched JSValue receiver can run arbitrary code using the interface's data, which may be a pointer (including storing that pointer globally or on the heap, but crucially, off the stack; see BadWrapper). For all other cases in the switch statement, the conversion to JS is statically dispatched so the Go compiler knows that nothing escapes.

And if a value is passed to JavaScript how do we know it is safe to not escape? I.e. be sure that JavaScript doesn't hold the value or use it in some asynchronous way?

It is unsafe for JS to hold pointers into Go's memory, because if Go allocates memory, it may need to expand its WebAssembly linear memory array buffer. At that point, JS would hold stale pointers. I assume it was for this reason that whoever designed syscall/js made it copy all values to JS. For example, when you pass a string to a JS function, the string is copied to JS, and becomes a new JS allocation that is independent from the Go string. Anyway, at least in my understanding, this design decision means that Go values will not escape to the Go or JS heap due to being passed to JS. But they could easily escape with a BadWrapper.

Disclaimer: This is all just my understanding, gathered by optimizing Go and syscall/js code. I'm not a Go compiler developer. Feel free to correct me if I said anything wrong or misleading!

cherrymui · 2021-10-09T00:34:12Z

Thanks. Yeah, I think it makes sense and is safe for ValueOf not escape the interface data at least in some cases (e.g. numbers).

rsc · 2021-10-13T18:00:45Z

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

gopherbot · 2021-10-16T15:24:36Z

Change https://golang.org/cl/356430 mentions this issue: syscall/js: remove Wrapper interface

finnbear · 2021-10-18T21:37:03Z

Change https://golang.org/cl/356430 mentions this issue: syscall/js: remove Wrapper interface

Great! Don't forget to apply the other optimizations (e.g. noescape; none of which are breaking changes; see my patch) so the performance gain will apply 😉

neelance · 2021-10-18T22:29:45Z

@finnbear Let's continue our discussion in #39740.

dmitshur · 2021-10-20T17:51:32Z

Updated the milestone to Go 1.18 since that's the version this proposal was implemented.

This change is significant enough to warrant being mentioned in the 1.18 release notes.

gopherbot added this to the Proposal milestone Jan 30, 2021

gopherbot added the Proposal label Jan 30, 2021

finnbear mentioned this issue Jan 30, 2021

syscall/js: increase performance of Call, Invoke, and New by not allowing new slices to escape onto the heap #39740

Closed

ianlancetaylor added this to Incoming in Proposals (old) Feb 17, 2021

rsc moved this from Incoming to Active in Proposals (old) Sep 15, 2021

rsc moved this from Active to Likely Accept in Proposals (old) Oct 6, 2021

rsc added the Proposal-FinalCommentPeriod label Oct 6, 2021

rsc moved this from Likely Accept to Accepted in Proposals (old) Oct 13, 2021

rsc changed the title ~~proposal: syscall/js: remove Wrapper type to avoid extreme allocations and improve performance~~ syscall/js: remove Wrapper type to avoid extreme allocations and improve performance Oct 13, 2021

rsc added the Proposal-Accepted label Oct 13, 2021

rsc modified the milestones: Proposal, Backlog Oct 13, 2021

gopherbot closed this as completed in 6c0daa7 Oct 18, 2021

dmitshur modified the milestones: Backlog, Go1.18 Oct 20, 2021

janpfeifer mentioned this issue Jan 9, 2022

Go 1.18 drops syscall.js.Wrapper gowebapi/webapi#15

Closed

rsc mentioned this issue Jan 12, 2022

proposal: syscall/js: Reinstate js.Wrapper interface #50310

Closed

mlctrez mentioned this issue Apr 27, 2022

updates required for go 1.18 hack-pad/go-indexeddb#2

Closed

golang locked and limited conversation to collaborators Oct 20, 2022

gopherbot added the FrozenDueToAge label Oct 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

syscall/js: remove Wrapper type to avoid extreme allocations and improve performance #44006

syscall/js: remove Wrapper type to avoid extreme allocations and improve performance #44006

finnbear commented Jan 30, 2021 •

edited

hajimehoshi commented Apr 13, 2021

neelance commented Apr 18, 2021

awfulcooking commented Jul 29, 2021

neelance commented Sep 9, 2021

rsc commented Sep 15, 2021

rsc commented Sep 15, 2021

finnbear commented Sep 15, 2021 •

edited

hajimehoshi commented Sep 16, 2021 •

edited

rsc commented Sep 22, 2021

rsc commented Oct 6, 2021

rsc commented Oct 6, 2021

cherrymui commented Oct 6, 2021

finnbear commented Oct 7, 2021 •

edited

cherrymui commented Oct 9, 2021

rsc commented Oct 13, 2021

gopherbot commented Oct 16, 2021

finnbear commented Oct 18, 2021 •

edited

neelance commented Oct 18, 2021

dmitshur commented Oct 20, 2021

syscall/js: remove Wrapper type to avoid extreme allocations and improve performance #44006

syscall/js: remove Wrapper type to avoid extreme allocations and improve performance #44006

Comments

finnbear commented Jan 30, 2021 • edited

Introduction

Why Wrapper matters

Data

Data collection method

Context

More context on my actual use case

hajimehoshi commented Apr 13, 2021

neelance commented Apr 18, 2021

awfulcooking commented Jul 29, 2021

neelance commented Sep 9, 2021

rsc commented Sep 15, 2021

rsc commented Sep 15, 2021

finnbear commented Sep 15, 2021 • edited

hajimehoshi commented Sep 16, 2021 • edited

rsc commented Sep 22, 2021

rsc commented Oct 6, 2021

rsc commented Oct 6, 2021

cherrymui commented Oct 6, 2021

finnbear commented Oct 7, 2021 • edited

cherrymui commented Oct 9, 2021

rsc commented Oct 13, 2021

gopherbot commented Oct 16, 2021

finnbear commented Oct 18, 2021 • edited

neelance commented Oct 18, 2021

dmitshur commented Oct 20, 2021

finnbear commented Jan 30, 2021 •

edited

Why `Wrapper` matters

finnbear commented Sep 15, 2021 •

edited

hajimehoshi commented Sep 16, 2021 •

edited

finnbear commented Oct 7, 2021 •

edited

finnbear commented Oct 18, 2021 •

edited