How about letting `ClientBuilder` expose `io.grpc.Channel`?

[Lincong]

Hi there, we build gRPC clients with Armeria 1.16.3. I wonder whether either of the following ideas could be good:

1. Let com.linecorp.armeria.client.ClientBuilder class expose an API to build and return a io.grpc.Channel (implemented by ArmeriaChannel).
2. Let ClientBuilder build a io.grpc.Channel once (at the first time when ClientBuilder#build(...) is called) and then re-use the channel for all future invocations of ClientBuilder#build(...).

Here is why I think these^ ideas would benefit our use case:

1. Currently we provide an abstraction called GrpcChannel to our users. GrpcChannel is not aware of any stub type. Users can create stubs from GrpcChannel. For example, channel.createStub[StubType](...).
2. Note that the above GrpcChannel is a custom abstraction defined by my team and it is NOT io.grpc.Channel.
3. Each GrpcChannel instance contains its own ClientBuilder (decorators) and ClientFactory (connection pool). The goal is to have a clear separation of resources among GrpcChannel instances. Users can create and manage GrpcChannels by themselves. grpc-java provides a similar mental mode (example) to allow users to create stub(s) from a io.grpc.Channel (a grpc-java interface/API).
4. In our current implementation, every channel.createStub[StubType](...) invocation requires calling ClientBuilder.build(stubClass) which creates a io.grpc.Channel. That means when N stubs are created from our GrpcChannel, there are N grpc-java io.grpc.Channels created (one for each stub).
5. This^ creates a lot of overhead to our stub creation process. Ideally we hope to let every GrpcChannel instance have one io.grpc.Channel instance from which all stubs are built.
6. We have done a POC for this^ idea and the performance improvement is significant. However, the POC is a bit hacky due to the fact that ClientBuilder does not provide an API to build io.grpc.Channel. Specifically, in the POC, after the first stub was created, we call stub.getChannel to get a reference to its underlying io.grpc.Channel built by ClientBuilder, then cache and reuse the io.grpc.Channel to build future stubs. If ClientBuilder provides an API to build and return a io.grpc.Channel (proposed idea 1), we can get rid of the hack. Or we can consider making ClientBuilder internally build a io.grpc.Channel once and keep reusing it for all invocations of ClientBuilder.build(stubClass). With this change, our code (in GrpcChannel.createStub) can simply call ClientBuilder.build(stubClass) to build every stub since ClientBuilder.build(stubClass) would be much more lightweight (because the io.grpc.Channel is reused).

Please let me know WYT. Thanks!

[minwoox]

Thanks for the detailed information. 👍

Currently, we can't reuse an ArmeriaChannel for different stubs because the jsonMarshaller must be created individually for each stub: https://github.com/line/armeria/blob/0960d091bfc7f350c17e68f57cc627de584b9705/grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/GrpcClientFactory.java#L170

We would need to disable JSON conversion to reuse the channel or introduce another implementation for the Channel. Given this limitation, I think a dedicated API for creating the channel is necessary, such as GrpcClientBuilder.buildChannel().

Do you have any idea on this? @line/dx

[Lincong]

Currently, we can't reuse an ArmeriaChannel for different stubs because the jsonMarshaller must be created individually for each stub

I am curious why this is the case. Is it because every jsonMarshaller instance is associated with some stub-specific state?

I think a dedicated API for creating the channel is necessary, such as GrpcClientBuilder.buildChannel()

SG, it works for our use case. Thanks!

[minwoox]

Is it because every jsonMarshaller instance is associated with some stub-specific state?

Yes, it is. A JSON marshaller has to know the type to serialize and deserialize. Here are links:

• MessageMarshaller that Armeria uses.
• JsonMarshaller that Java gRPC uses.

[trustin]

Maybe we could refactor ArmeriaChannel so that it's capable of having different marshallers for different stub types? Could use weak-key map to avoid leak (if that's gonna be a problem)

[minwoox]

Maybe we could refactor ArmeriaChannel so that it's capable of having different marshallers for different stub types?

Yeah, that's also a good idea. 👍