Skip to content

syscall/js: allocate makeArgs slices on stack #49799

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

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 31 additions & 6 deletions src/syscall/js/js.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ func ValueOf(x interface{}) Value {
}
}

//go:noescape
Copy link
Author

@finnbear finnbear Nov 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

func stringVal(x string) ref

// Type represents the JavaScript type of a Value.
Expand Down Expand Up @@ -292,6 +293,7 @@ func (v Value) Get(p string) Value {
return r
}

//go:noescape
Copy link
Author

@finnbear finnbear Nov 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

func valueGet(v ref, p string) ref

// Set sets the JavaScript property p of value v to ValueOf(x).
Expand All @@ -306,6 +308,7 @@ func (v Value) Set(p string, x interface{}) {
runtime.KeepAlive(xv)
}

//go:noescape
func valueSet(v ref, p string, x ref)

// Delete deletes the JavaScript property p of value v.
Expand All @@ -318,6 +321,7 @@ func (v Value) Delete(p string) {
runtime.KeepAlive(v)
}

//go:noescape
func valueDelete(v ref, p string)

// Index returns JavaScript index i of value v.
Expand Down Expand Up @@ -347,15 +351,29 @@ func (v Value) SetIndex(i int, x interface{}) {

func valueSetIndex(v ref, i int, x ref)

func makeArgs(args []interface{}) ([]Value, []ref) {
argVals := make([]Value, len(args))
argRefs := make([]ref, len(args))
func makeArgs(args []interface{}) (argVals []Value, argRefs []ref) {
// value chosen for being power of two, and enough to handle all web APIs
// in particular, note that WebGL2's texImage2D takes up to 10 arguments
const maxStackArgs = 16
if len(args) <= maxStackArgs {
// as long as makeArgs is inlined, these will be stack-allocated
argVals = make([]Value, len(args), maxStackArgs)
argRefs = make([]ref, len(args), maxStackArgs)
} else {
// allocates on the heap, but exceeding maxStackArgs should be rare
argVals = make([]Value, len(args))
argRefs = make([]ref, len(args))
}
return
}

func storeArgs(args []interface{}, argValsDst []Value, argRefsDst []ref) {
// would go in makeArgs if the combined func was simple enough to inline
for i, arg := range args {
v := ValueOf(arg)
argVals[i] = v
argRefs[i] = v.ref
argValsDst[i] = v
argRefsDst[i] = v.ref
}
return argVals, argRefs
}

// Length returns the JavaScript property "length" of v.
Expand All @@ -376,6 +394,7 @@ func valueLength(v ref) int
// The arguments get mapped to JavaScript values according to the ValueOf function.
func (v Value) Call(m string, args ...interface{}) Value {
argVals, argRefs := makeArgs(args)
storeArgs(args, argVals, argRefs)
res, ok := valueCall(v.ref, m, argRefs)
runtime.KeepAlive(v)
runtime.KeepAlive(argVals)
Expand All @@ -391,13 +410,15 @@ func (v Value) Call(m string, args ...interface{}) Value {
return makeValue(res)
}

//go:noescape
Copy link
Author

@finnbear finnbear Nov 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

func valueCall(v ref, m string, args []ref) (ref, bool)

// Invoke does a JavaScript call of the value v with the given arguments.
// It panics if v is not a JavaScript function.
// The arguments get mapped to JavaScript values according to the ValueOf function.
func (v Value) Invoke(args ...interface{}) Value {
argVals, argRefs := makeArgs(args)
storeArgs(args, argVals, argRefs)
res, ok := valueInvoke(v.ref, argRefs)
runtime.KeepAlive(v)
runtime.KeepAlive(argVals)
Expand All @@ -410,13 +431,15 @@ func (v Value) Invoke(args ...interface{}) Value {
return makeValue(res)
}

//go:noescape
func valueInvoke(v ref, args []ref) (ref, bool)

// New uses JavaScript's "new" operator with value v as constructor and the given arguments.
// It panics if v is not a JavaScript function.
// The arguments get mapped to JavaScript values according to the ValueOf function.
func (v Value) New(args ...interface{}) Value {
argVals, argRefs := makeArgs(args)
storeArgs(args, argVals, argRefs)
res, ok := valueNew(v.ref, argRefs)
runtime.KeepAlive(v)
runtime.KeepAlive(argVals)
Expand All @@ -429,6 +452,7 @@ func (v Value) New(args ...interface{}) Value {
return makeValue(res)
}

//go:noescape
func valueNew(v ref, args []ref) (ref, bool)

func (v Value) isNumber() bool {
Expand Down Expand Up @@ -580,4 +604,5 @@ func CopyBytesToJS(dst Value, src []byte) int {
return n
}

//go:noescape
Copy link
Author

@finnbear finnbear Nov 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

func copyBytesToJS(dst ref, src []byte) (int, bool)