syscall/js: allocate makeArgs slices on stack #49799

finnbear · 2021-11-25T18:39:44Z

This change optimizes the syscall/js package such that:

Calling a JavaScript function with 16 arguments or fewer will not induce two additional heap allocations, at least with the current Go compiler
Using syscall/js features with slices and strings of statically-known length will not cause them to be escaped to the heap, at least with the current Go compiler
The reduction in allocations has the additional benefit that the garbage collector runs less often, blocking WebAssembly's one and only thread less often

There should be no observable difference in the syscall/js API.

These changes have been discussed and mostly agreed upon in the linked issue.

I am aware that this change falls outside a release cycle and during a development freeze. I will leave it up to maintainers to decide whether this qualifies for a freeze exception.

finnbear · 2021-11-25T18:42:22Z

src/syscall/js/js.go

@@ -580,4 +604,5 @@ func CopyBytesToJS(dst Value, src []byte) int {
 	return n
 }

+//go:noescape


Justification: this will affect whether byte slices with dynamic contents but statically known length to not escape anymore. It is correct because the bytes are copied to JS, not referenced by JS

finnbear · 2021-11-25T18:42:52Z

src/syscall/js/js.go

@@ -292,6 +293,7 @@ func (v Value) Get(p string) Value {
 	return r
 }

+//go:noescape


Justification: this (and the one on valueSet and valueDelete) will affect whether strings with dynamic contents but statically known length to not escape anymore. It is correct, because the Go memory is only accessed for the duration of the call.

finnbear · 2021-11-25T18:42:58Z

src/syscall/js/js.go

@@ -209,6 +209,7 @@ func ValueOf(x interface{}) Value {
 	}
 }

+//go:noescape


Justification: this will affect whether strings with dynamic contents but statically known length to not escape anymore. It is correct, because the string is immediately copied away from Go memory on the JS side.

gopherbot · 2021-11-25T18:44:16Z

This PR (HEAD: 8c960c1) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/367045 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

finnbear · 2021-11-25T18:44:25Z

src/syscall/js/js.go

@@ -391,13 +410,15 @@ func (v Value) Call(m string, args ...interface{}) Value {
 	return makeValue(res)
 }

+//go:noescape


Justification: this (and the one on valueInvoke and valueNew) is essential for the slice optimization. It is correct because the slices are not kept around.

gopherbot · 2021-11-25T18:46:51Z

Message from Go Bot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
A maintainer will review your change and provide feedback. See
https://golang.org/doc/contribute.html#review for more info and tips to get your
patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://golang.org/s/release
for more details.