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
codegen: Use Go generics for stream types #7030
Comments
Something like this could be really interesting. Unfortunately this is a very niche kind of thing and we likely won't have the bandwidth to work on it in the foreseeable future. If you are interested in fleshing this out and sending PRs, we can review them. As for an implementation plan, I'd propose something like:
Not being backward compatible would be the biggest problem with changing how the codegen works. We'd either need to generate both together with different names (the non-generic implementation could even call the generic one), or we'd need a flag to decide which one to generate. If we generated both, we'd need to choose good names to be ergonomic and to avoid conflicts. I'm not concerned about supporting versions of Go older than 1.18, as this kind of thing is necessary from time to time anyway (e.g. we will soon be making changes to the codegen that will require the most recent release of grpc-go, which doesn't support 1.18 already). |
Thanks for taking the time to read! I'd be happy to contribute implementation, but of course also have a lot of other things on my plate so no guarantees. Would you also be willing to accept an implementation plan like this:
To my mind, the big change here is in the publicly-exposed interface, and that's the thing worth protecting behind a flag. But I definitely don't have insight into all of the things the gRPC team might be concerned about (do Go generics have meaningfully different runtime performance? I don't think so, but maybe) so I'm not sure if swapping out the underlying implementation is too risky to not put in Thanks for the note about go 1.18; I'll keep an eye on other compatibility stuff and just make sure any reliance on generics lands after the other changes you mentioned. |
I definitely would like to put the interfaces in Does the type aliases prevent any backward compatibility breakage problems? If so, that might be the ideal final state? I really don't want to be in a situation where users have to choose between two different forms of codegen that are incompatible. Otherwise no old generated code library can ever migrate to the new style (without a v2 since it's a breaking change), and users of that generated code would be stuck implementing it that way forever. |
I've added the generic implementations to the experimental package, and added the new codegen behind a flag, in this PR: #7057 Let me know what you think! |
Let's leave this issue open to track:
|
Use case(s)
Suppose you have the following service definitions:
The idea here being that someone with access to a Greeter client could call StoreHello multiple times to store many names that should be greeted, and then either a Greeter client or a GreeterReadOnly client can call SayHello to get a stream of all the stored replies. Obviously this is a toy example, but you can see a real example of this same pattern here. The salient feature here is this: we have two different services which define the exact same streaming RPC.
Now because both Greeter and GreeterReadOnly share functionality, we want to use a single implementation type to implement both of them. So we write something like this:
But of course when we go to Register this implementation, it doesn't actually work. It satisfies the
Greeter
interface, but not theGreeterReadOnly
interface.This is because grpc-go has generated the following interfaces for us:
The
SayHello
methods in these two interfaces do not have the same signature, despite having the exact same definition in the original .proto file. This means that they can never both be implemented by the same Go struct.Note: this is different from the case for Unary RPCs: if two different gRPC services define the same unary rpc, then the generated Go methods have the exact same signature and can both be implemented by the same type.
Proposed Solution
The obvious solution here is that the stream object could be named by the message type(s) it is capable of streaming, rather than by the service and rpc which define it. For example, instead of
Greeter_SayHelloServer
, the type could be namedHelloReplyServerStreamServer
, indicating that it a) is the Server half of the stream object, b) that it is for a Server-Streaming (i.e. one request, many replies) method, and c) that it streamsHelloReply
messages. Then this same exact type could be used in both versions of theSayHello
method, giving them the same signature, and allowing them to be implemented by a single type.But I propose going even further: rather than generating a unique type for every stream, I think that protoc-gen-go-grpc should instead take advantage of Go's generics support and define only six streaming types, parameterized by the message type they stream:
Then the generated code for the service interfaces and their
SayHello
method would simply be reduced to:Note first that this eliminates the need for there ever to be service- or method-specific types in the protoc-gen-go-grpc generated code. And in fact these
[Server|Client|Bidi]Stream[Server|Client]
generic types could be defined once in the gRPC package itself and then simply referenced in the generated code.But note more importantly that this means that both
GreeterServer.SayHello
andGreeterReadOnlyServer.SayHello
have the exact same function signature, meaning that they can both be implemented by a single type:Additional Context
This proposal is obviously not backwards-compatible, and if it were adopted it would mean that the new version of protoc-gen-go-grpc cannot be used by versions of Go prior to when it got generics (1.18). But I think it is sufficiently elegant that it is worth considering anyway.
The text was updated successfully, but these errors were encountered: