proposal: cmd/cgo: add "//go:cgo_unsafe_stack_pointer" to avoid escaping

[doujiang24]

This is for better performance, which will avoid escaping.

example:

//go:cgo_unsafe_stack_pointer str
void strpointer(void *str) {
    // must not callback to go
}

Now, the str pointer must be allocated on the heap, for safety.

AFAIK, the unsafe cases:

1. C code calls back into Go code, and the Go code triggers a stack copy, the str pointer might move. from: https://github.com/golang/go/issues/20281#issuecomment-299928114
2. GC may trigger shrinkstack, the str pointer might move, before C returns.

So, when people use //go:cgo_unsafe_stack_pointer, they must make sure the C code will not call back to go, it's an unsafe usage.

And, the go compiler needs these changes:

1. skip generating _Cgo_use for str pointer.
2. skip shrinkstack when the goroutine is invoking such an unsafe C function (we could set a flag under g, before invoking the C function).

[doujiang24]

2. GC may trigger shrinkstack, the str pointer might move, before C returns.

Oh, sorry. I made a mistake here. The stack won't move while in syscall.

Then, seems this can be easier, just need to skip _Cgo_use.

[aclements]

Alternatively, we could have an annotation that says a C call does not call back into Go. That captures a user intent rather than a language implementation detail, and we can easily check that the call obeys this at run-time.

[aclements]

//go:cgo_no_go_callback? We use the term "callback" in the runtime for this, but it could be interpreted as "this doesn't take a callback function pointer."

//go:cgo_no_go_calls?

[rsc]

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

[bcmills]

we can easily check that the call obeys this at run-time.

I don't think it's sufficient just to check that the same C thread doesn't call back into Go. What if the C thread passes the pointer to a different C thread, when then passes that pointer back to a Go function?

[aclements]

That's an interesting point. That's probably something we'd want to account for in the documentation of any such directive (and maybe in its name), but I'm not sure that having a check that doesn't trip in a tiny fraction of cases is a deal-breaker.

[hherman1]

How much of a performance improvement might we expect from using this directive? And in what situations? I’m trying to understand where this proposal is coming from.

[doujiang24]

Thanks all, happy to see this proposal is active.

That's probably something we'd want to account for in the documentation of any such directive (and maybe in its name)

yep, I think it's an unsafe annotation - for better performance, since we can not protect it at run-time.

For its name, I think the unsafe keywords may be deserved. But, I have no good idea for the full name if we want a high-level name, not implementation details.

[doujiang24]

How much of a performance improvement might we expect from using this directive? And in what situations? I’m trying to understand where this proposal is coming from.

Hi @hherman1

That depends, it could be very significant - when it's heavily using cgo and escaping Go objects frequency - with large GC overhead.

We are implementing the Envoy Golang filter extension, https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/golang_filter That's the heavy cgo use case, it may lead to multiple Go objects escaping in each request, and that could a significant overhead.

[ianlancetaylor]

Note that I believe that it is possible today for a goroutine that is suspended in a call to C to have its stack shrunk. That is OK because the current escaping implementation mean that no stack allocated address can be passed to C. If we implement this new pragma, then I think that in order to make it useful it will have to somehow disable stack shrinking for any call to a C function using this pragma.

[ianlancetaylor]

Upon further thought I don't fully understand where this pragma should be placed. Currently the cgo tool makes no attempt to parse any portion of the cgo comment. It just passes everything to the C compiler. So I don't see how the pragma can appear in the cgo comment. But I also don't understand where else it could appear.

[doujiang24]

Thanks @ianlancetaylor In the current implementation, maybe isShrinkStackSafe is protecting a goroutine from shrinking while invoking into C? https://github.com/golang/go/blob/master/src/runtime/stack.go#L1142 I'm not sure about it.

Currently the cgo tool makes no attempt to parse any portion of the cgo comment.

Oh, that was my idea, a line before the C function. So, that sounds like a big change.

[aclements]

That depends, it could be very significant - when it's heavily using cgo and escaping Go objects frequency - with large GC overhead.

It would be valuable to quantify this more, if possible. This is tricky to do at scale, though I think not impossible. For your particular application, are you positive that cgo is the only reason these objects are escaping? We've always assumed that, in practice, objects being passed to cgo are typically going to be on the heap for other reasons anyway. Strong evidence that this isn't true, at least in some range of applications, would help motivate this.

Note also that we might improve escape analysis in the future, so having more information about C calls may have more impact in the future.

In the current implementation, maybe isShrinkStackSafe is protecting a goroutine from shrinking while invoking into C?

I believe you're correct that this prevents us from shrinking stacks while in C. If it doesn't, that's certainly something we could do.

[rsc]

The annotations in the C comment today begin with #cgo, and we can't easily match it to the "upcoming" C function since we don't parse the C code. Perhaps:

#cgo noescape <function name>

for the annotation? The important part is that the arguments do not escape from the C function back into Go. Unfortunately to put something on the stack in the main Go toolchain we also need to know that there is no call back into Go, because that might grow the stack. So we probably also need

#cgo nocallback <function name>

Implementations that use a segmented stack would be able to make the optimization of keeping values on the stack with only "noescape". The current toolchain would need both for the same optimization.

So maybe we should have both, to allow a C implementation to declare what it does and a Go implementation to make use of what it needs.

Thoughts?

[doujiang24]

It would be valuable to quantify this more, if possible. This is tricky to do at scale, though I think not impossible.

Yep, it's possible. Now, the total GC CPU overhead is about 10% of the total CPU usage, in our application(gateway). We need to quantify the GC overhead of this escaping thing. I think there are two ways:

1. the number of GC objects that could be avoided escaping / total temporarily GC objects, could be near 5%, which means we could save 0.5% of the total CPU usage. It's should be significant for us, if our application(gateway) is a large-scale infrastructure, even 0.1% is a big value for us.
2. testing the CPU usage with a POC optimization implementation, that I haven't implemented yet.

For your particular application, are you positive that cgo is the only reason these objects are escaping

Yes, I can confirm it, here is an example in the Envoy Golang extension:

	var value string
	res := C.envoyGoFilterHttpGetStringValue(r, ValueRouteName, unsafe.Pointer(&value))
        return strings.Clone(value)

Note also that we might improve escape analysis in the future, so having more information about C calls may have more impact in the future.

Cool, sounds great.

[doujiang24]

#cgo noescape <function name>

Great, I think it's very good.

Implementations that use a segmented stack would be able to make the optimization of keeping values on the stack with only "noescape".

Sorry, I'm confused about this, the current Go implement is not using a segmented stack, right?

So maybe we should have both, to allow a C implementation to declare what it does and a Go implementation to make use of what it needs.

Sorry, I think I haven't got the meaning to have them both. In my opinion, if the pointer won't callback to Go, then it's safe to avoid escape. So, I think one annotation may be enough.

Thanks.

[ianlancetaylor]

Note that I don't think your example code is good programming style. You are passing a pointer to a Go string to C. The documented API permits C code to accept that string as _GoString_*. But there is no documented way for C code to change the values in the string. The only documented API is to fetch the length and and byte pointer from the string (using _GoStringLen and _GoStringPtr). Your code must be using an undocumented and unsupported API.

It would be cleaner to stick to the documented and supported APIs by having the C code return a length and a C pointer, one way or another, and have the Go code call C.GoStringN.

[ianlancetaylor]

The current Go implementation does not use a segmented stack, but it's something we've used in the past and that we've considered using again in the future.

Technically, a C function that is marked nocallback does not guarantee that any pointer passed to that function does not escape. For example, the C function could pass the pointer to a different thread that could call back into Go. That is why we need both nocallback and noescape. Of course, we could say that nocallback implies noescape, but that is a subtle point and it seems at least possible that we would regret that in the future.

[doujiang24]

The current Go implementation does not use a segmented stack, but it's something we've used in the past and that we've considered using again in the future.

Okay, understand. Thanks for your clarification. Then, having these two annotations does make sense.

Note that I don't think your example code is good programming style.

Yep, I knew it is an unsafe usage, using this way, just to make the code simpler. Also, in the Envoy Go extension, we do disable cgocheck to support this usage. In some other more complicated cases, we use it for better performance.

i.e. we preallocate memory Go side and pass pointer to C, then fill memory(write) on C side. https://github.com/envoyproxy/envoy/blob/main/contrib/golang/filters/http/source/go/pkg/http/capi_impl.go#L86-L98 so that we could save memory copy on the C side, since the original memory lifecycle is controlled by Envoy filtermanager(related to downstream request), maybe freed in an Envoy worker thread, we can not assume that memory is still there when we copy them in Go(in another Go thread).

[rsc]

Updated title. Sounds like #cgo noescape and #cgo nocallback are okay.

Have all concerns about this proposal been addressed?

[doujiang24]

Thanks, yep, it's good from my side.

[dr2chase]

Skimming this, I didn't see mention/proposal of the possibility of enforcing this dynamically, and I think we can do that for #cgo nocallback. If that sets a bit in the gororutine (we need to note that the stack cannot be shrunk, right?) we could also check that bit on any callback into Go, and panic if it is set.

[rsc]

Yes, we should check nocallback dynamically - very easy.

[rsc]

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

[rsc]

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

[gopherbot]

Change https://go.dev/cl/497837 mentions this issue: cmd/cgo: add #cgo noescape/nocallback annotations

[gopherbot]

Change https://go.dev/cl/522937 mentions this issue: doc/go1.22: mention new #cgo directives

[gopherbot]

Change https://go.dev/cl/522939 mentions this issue: cmd/cgo/internal/test: benchmark for #cgo noescape directive

[rsc]

It turns out that for compatibility reasons we cannot release this feature until Go 1.23. We will land the appropriate preparatory work in Go 1.22 and then enable it in Go 1.23. Reopening.

[rsc]

Also any rollback of my rollback for Go 1.23 should chase down why using #cgo noescape causes crashes, as in #63739.