grpc / grpc-go

The Go language implementation of gRPC. HTTP/2 based RPC
https://grpc.io
Apache License 2.0
21.13k stars 4.39k forks source link

optimize to reduce memory alloc and copying? #124

Closed awalterschulze closed 9 years ago

awalterschulze commented 9 years ago

May I ask what the plan is over here?

https://github.com/grpc/grpc-go/blob/9c4157fb5a0472c1dbfa016439a0146cd8f0221e/rpc_util.go#L142 // TODO(zhaoq): optimize to reduce memory alloc and copying.

iamqizhao commented 9 years ago

Thanks for asking. I plan to start it sometime next week. But I could be preempted by other things. :)

On Wed, Mar 18, 2015 at 8:06 AM, Walter Schulze notifications@github.com wrote:

May I ask what the plan is over here?

https://github.com/grpc/grpc-go/blob/9c4157fb5a0472c1dbfa016439a0146cd8f0221e/rpc_util.go#L142 // TODO(zhaoq): optimize to reduce memory alloc and copying.

— Reply to this email directly or view it on GitHub https://github.com/grpc/grpc-go/issues/124.

awalterschulze commented 9 years ago

What do you plan to do?

With github.com/gogo/protobuf I generate a Size, Marshal and Marshalto method. The Marshal method calls Size and then allocates a byte slice of that size. Then it pass that to MarshalTo which fills in the byte slice. This way there is only one alloc per message.

Given a big enough pre allocated buffer it would be easy to reuse the buffer with Marshalto. And Marshalto could even be changed to a MarshalZeroTo which does the same job as Marshalto, but with less copying.

My experience is that golang/protobuf does not really want to generate code, but wants to do almost everything with reflect and unsafe pointer. This is definitely slower, but I understand that it is less code to maintain.

So what I want to know is what you plan to do here and if you golang/protobuf is still not generating code (things might have changed), how can I still get the maximum speed out by leveraging gogoprotobuf's generated code?

If you just call Marshal then atleast I just do one alloc. But there is so much speed to gain here, since writing the buffer does a copy anyway thus it is really easy to reuse a big buffer. For example https://github.com/gogo/protobuf/blob/master/io/varint.go I could make this even faster by not having any copying instructions in MarshalTo

grpc looks great and I would really love to use it, but speed is very important. I would like to be prepared for any changes I can make to gogoprotobuf to still gain the maximum amount of speed. I really don't want to make another fork. So maybe there is someway we could work together?

I have grpc code generation merged into gogoprotobuf on the proto3 branch if you are at all interested.

iamqizhao commented 9 years ago

The good news is that I am making a change so that it will be very easy to support various codecs for grpc-go because essentially grpc is independent of IDL and codec. Once it is done, it will be very easy to use your own codec (gogoprotobuf) to try out grpc.

The plan for protobuf is to minimize memory alloc/dealloc and copying in grpc. For example, proto.Buffer ( https://github.com/golang/protobuf/blob/1e73516e50e04b41a6760c856eee86b00a384526/proto/lib.go#L250) would be among the first things I will try on.

On Thu, Mar 19, 2015 at 12:53 AM, Walter Schulze notifications@github.com wrote:

What do you plan to do?

With github.com/gogo/protobuf I generate a Size, Marshal and Marshalto method. The Marshal method calls Size and then allocates a byte slice of that size. Then it pass that to MarshalTo which fills in the byte slice. This way there is only one alloc per message.

Given a big enough pre allocated buffer it would be easy to reuse the buffer with Marshalto. And Marshalto could even be changed to a MarshalZeroTo which does the same job as Marshalto, but with less copying.

My experience is that golang/protobuf does not really want to generate code, but wants to do almost everything with reflect and unsafe pointer. This is definitely slower, but I understand that it is less code to maintain.

So what I want to know is what you plan to do here and if you golang/protobuf is still not generating code (things might have changed), how can I still get the maximum speed out by leveraging gogoprotobuf's generated code?

If you just call Marshal then atleast I just do one alloc. But there is so much speed to gain here, since writing the buffer does a copy anyway thus it is really easy to reuse a big buffer. For example https://github.com/gogo/protobuf/blob/master/io/varint.go I could make this even faster by not having any copying instructions in MarshalTo

grpc looks great and I would really love to use it, but speed is very important. I would like to be prepared for any changes I can make to gogoprotobuf to still gain the maximum amount of speed. I really don't want to make another fork. So maybe there is someway we could work together?

I have grpc code generation merged into gogoprotobuf on the proto3 branch if you are at all interested.

— Reply to this email directly or view it on GitHub https://github.com/grpc/grpc-go/issues/124#issuecomment-83400326.

awalterschulze commented 9 years ago

That is great news about being IDL and codec independent. I could also use that in totally different project I am working on :) Could you tell me when you are ready so I can test your change? Thank you very much.

iamqizhao commented 9 years ago

On Thu, Mar 19, 2015 at 1:31 AM, Walter Schulze notifications@github.com wrote:

That is great news about being IDL and codec independent. I could also use that in totally different project I am working on :) Could you tell me when you are ready so I can test your change?

It should be within next week.

Thank you very much.

— Reply to this email directly or view it on GitHub https://github.com/grpc/grpc-go/issues/124#issuecomment-83416600.

awalterschulze commented 9 years ago

awesome :)

On 19 March 2015 at 19:54, Qi Zhao notifications@github.com wrote:

On Thu, Mar 19, 2015 at 1:31 AM, Walter Schulze notifications@github.com wrote:

That is great news about being IDL and codec independent. I could also use that in totally different project I am working on :) Could you tell me when you are ready so I can test your change?

It should be within next week.

Thank you very much.

— Reply to this email directly or view it on GitHub https://github.com/grpc/grpc-go/issues/124#issuecomment-83416600.

— Reply to this email directly or view it on GitHub https://github.com/grpc/grpc-go/issues/124#issuecomment-83693718.

iamqizhao commented 9 years ago

Hi Walter,

I just checked in #146 to allow custom codec working with grpc-go. For now, we allow a custom codec by doing the following: i) implement your custom codec satisfying https://github.com/grpc/grpc-go/blob/master/rpc_util.go#L53; ii) on client side, conn, err := grpc.Dial(serverAddr, grpc.WithCodec(yourCodec)) iii) on server side, server := grpc.NewServer(grpc.CustomCodec(yourCodec))

Feel free to ping us if there is any problem.

awalterschulze commented 9 years ago

This looks great :) I can't wait to start playing this. Thank you very much.

iamqizhao commented 9 years ago

I am not clear how you will proceed. Just be aware of that the grpc server does unmarshaling inside the generated code (e.g., https://github.com/grpc/grpc-go/blob/master/test/grpc_testing/test.pb.go#L544) for unary rpc. You probably need to treat it properly depending on your approach.

On Thu, Apr 2, 2015 at 1:21 AM, Walter Schulze notifications@github.com wrote:

This looks great :) I can't wait to start playing this. Thank you very much.

— Reply to this email directly or view it on GitHub https://github.com/grpc/grpc-go/issues/124#issuecomment-88820770.

awalterschulze commented 9 years ago

That is fine my fork generates code with the correct import :) But thank you for the heads up.

On 2 April 2015 at 21:36, Qi Zhao notifications@github.com wrote:

I am not clear how you will proceed. Just be aware of that the grpc server does unmarshaling inside the generated code (e.g.,

https://github.com/grpc/grpc-go/blob/master/test/grpc_testing/test.pb.go#L544 ) for unary rpc. You probably need to treat it properly depending on your approach.

On Thu, Apr 2, 2015 at 1:21 AM, Walter Schulze notifications@github.com wrote:

This looks great :) I can't wait to start playing this. Thank you very much.

— Reply to this email directly or view it on GitHub https://github.com/grpc/grpc-go/issues/124#issuecomment-88820770.

— Reply to this email directly or view it on GitHub https://github.com/grpc/grpc-go/issues/124#issuecomment-89020023.

awalterschulze commented 9 years ago

For when you get back from holiday

I have finally created a repo for the grpc benchmarks and some simple tests. https://github.com/gogo/grpctests

I don't think I am doing it exactly right I get quite a few log messages, for example: "failed to write status: connection error: desc = "transport: use of closed network connection"" "http2Client.notifyError got notified that the client transport was broken EOF"

These are the important files, could you please take a look: https://github.com/gogo/grpctests/blob/master/bench/bench_test.go https://github.com/gogo/grpctests/blob/master/simple/grpc_test.go

Thank you again and enjoy your holiday

On 7 April 2015 at 11:57, Walter Schulze awalterschulze@gmail.com wrote:

That is fine my fork generates code with the correct import :) But thank you for the heads up.

On 2 April 2015 at 21:36, Qi Zhao notifications@github.com wrote:

I am not clear how you will proceed. Just be aware of that the grpc server does unmarshaling inside the generated code (e.g.,

https://github.com/grpc/grpc-go/blob/master/test/grpc_testing/test.pb.go#L544 ) for unary rpc. You probably need to treat it properly depending on your approach.

On Thu, Apr 2, 2015 at 1:21 AM, Walter Schulze notifications@github.com wrote:

This looks great :) I can't wait to start playing this. Thank you very much.

— Reply to this email directly or view it on GitHub https://github.com/grpc/grpc-go/issues/124#issuecomment-88820770.

— Reply to this email directly or view it on GitHub https://github.com/grpc/grpc-go/issues/124#issuecomment-89020023.

iamqizhao commented 9 years ago

Do you still have problems running the test and benchmark here? Do you want me to try out?

awalterschulze commented 9 years ago

I ran go get -u google.golang.org/grpc again and then ran my tests and benchmarks again. I still see the same problems. I would really appreciate it if you could take a look.

awalterschulze commented 9 years ago

I was wondering if you have had any time to check those benchmarks out?

iamqizhao commented 9 years ago

On Sat, May 9, 2015 at 12:25 AM, Walter Schulze notifications@github.com wrote:

I was wondering if you have had any time to check those benchmarks out?

nah, planned to do this week but got sick ...

— Reply to this email directly or view it on GitHub https://github.com/grpc/grpc-go/issues/124#issuecomment-100438344.

awalterschulze commented 9 years ago

join the club :)

awalterschulze commented 9 years ago

get well soon