protoc-gen-go: Implement UnimplementedServer with value instead of pointer

[darkfeline]

Is your feature request related to a problem? Please describe.

The UnimplementedServer generated by protoc-gen-go in 1.3.2 implements the server methods with a pointer to an empty struct. This is pointless since the pointer isn't used for anything and forces an embedding struct to embed a pointer to nothing.

Describe the solution you'd like
Make UnimplementedServer implement the server methods with an empty struct value rather than pointer. That way, users do not have to embed a useless pointer.

This is backward compatible because the method set of foo embedding *UnimplementedServer includes the method set of foo embedding UnimplementedServer.

[puellanivis]

All pointers to an empty struct are allowed to be identical, so the pointer is already no worse than a placeholder scalar (heck, the pointer can even be nil, as long as the function does not weirdly deref the pointer to an empty struct.)

So, in the end, the only issue at hand for switching from pointer to value, is… the size of one pointer in your service. https://play.golang.org/p/vpqE3mdXiIL

I am not saying that such a change would not save space, but in context of one process memory space, that space savings ends up being pretty negligible. 🤷‍♀

PS: due to the way pointer receiver methods work, you should be able to use a non-pointer embedded value even if the code is generating pointer receiver methods. As per example playground above. So, does anything even need to change?

PPS: Note that the opposite does not apply. That is, if you have a non-pointer receiver, you could get a nil pointer deref when someone uses the receiver method on an embedded pointer type.

[dsnet]

\cc @dfawley who reviewed #785

[dfawley]

PS: due to the way pointer receiver methods work, you should be able to use a non-pointer embedded value even if the code is generating pointer receiver methods. As per example playground above. So, does anything even need to change?

This is correct. We've implemented the use of UnimplementedXxxServer in grpc-go, and we've done so by value and not pointer.

PPS: Note that the opposite does not apply. That is, if you have a non-pointer receiver, you could get a nil pointer deref when someone uses the receiver method on an embedded pointer type.

Also correct. So if a user has embedded *pb.UnimplementedXxxServer, such a change would break them. While I agree it probably would have been better to pass by value, I'm not sure we can safely change it now, and the value of such a change would be questionable.

[darkfeline]

PPS: Note that the opposite does not apply. That is, if you have a non-pointer receiver, you could get a nil pointer deref when someone uses the receiver method on an embedded pointer type.

I overlooked this.

PS: due to the way pointer receiver methods work, you should be able to use a non-pointer embedded value even if the code is generating pointer receiver methods. As per example playground above. So, does anything even need to change?

I also overlooked this, because you need to explicitly pass the address to satisfy interfaces: https://play.golang.org/p/GobufNmGHU0

All of this still prevents you from passing an empty struct implementation of a gRPC server everywhere, since you'll end up being forced to pass a pointer around at some point.

However, since this isn't a backward compatible change as I initially thought, it's probably not worth it.

[dfawley]

All of this still prevents you from passing an empty struct implementation of a gRPC server everywhere, since you'll end up being forced to pass a pointer around at some point.

I don't understand this comment. The only intended use of this type is to embed it in your service implementation to prevent newly-added methods from breaking your build:

type myFooServer {
  pb.UnimplementedFooServer
  ...
}

Otherwise you should ignore Unimplemented services completely.

[darkfeline]

You can't embed the UnimplementedServer as a value in an empty struct type Foo, and then pass an empty struct Foo by value to satisfy the interface of the Server. You have to pass a pointer to an empty struct Foo to satisfy the Server interface.

I believe that with the way interface values are currently implemented the extra pointer value doesn't really matter, but conceptually it sucks that you can't embed UnimplementedServer and still implement the server interface by value of the embedee struct.

[puellanivis]

Everyone is so busy trying to microoptimize away pointers to a struct.

[darkfeline]

GC is expensive. The "small" design choice of using pointers for all scalar values in protobuf 2 structs in Go results in a couple magnitudes worse performance (although I haven't saved the actual numbers to report).

[puellanivis]

That’s not a microoptimization, and that was not the only reason why proto3 eschewed pointers to scalars. It also made assigning constants into those values rather painful. Unfortunately, some other APIs (cough S3 cough) copied the idea at the time, and still remain annoying to use.

What I mean about microoptimizations is trying to get rid of the pointer to an empty struct Server. The use of pointers there is minimal, and puts zero load on the GC.