grpc-go icon indicating copy to clipboard operation
grpc-go copied to clipboard

preloader=on flag in benchmarks impacts only the client and not the server

Open s-matyukevich opened this issue 2 years ago • 4 comments

NOTE: if you are reporting is a potential security vulnerability or a crash, please follow our CVE process at https://github.com/grpc/proposal/blob/master/P4-grpc-cve-process.md instead of filing an issue here.

Please see the FAQ in our main README.md, then answer the questions below before submitting your issue.

What version of gRPC are you using?

latest code on master branch

What version of Go are you using (go version)?

go1.20.4

What operating system (Linux, Windows, …) and version?

macOS

What did you do?

If possible, provide a recipe for reproducing the error.

go run benchmark/benchmain/main.go -workloads unconstrained -kbps 0 -latency 0s -preloader=on -maxConcurrentCalls 1000 -reqSizeBytes 1048576 -respSizeBytes 1048576 -benchtime 1m -memProfile ~/tmp/profile
go tool pprof -http :8080 ~/tmp/profile

What did you expect to see?

A small amount of memory should be spend on marshaling protos because with preloader-on we should prepare messages in advance and don't spend CPU/memory on marshaling them at test runtime.

What did you see instead?

86.81% of total memory is spent on proto.MarshalOptions.marshal

Type: inuse_space
Time: May 26, 2023 at 2:36pm (MDT)
Showing nodes accounting for 2.32GB, 99.79% of 2.33GB total
Dropped 26 nodes (cum <= 0.01GB)
      flat  flat%   sum%        cum   cum%
    2.02GB 86.81% 86.81%     2.02GB 86.81%  google.golang.org/protobuf/proto.MarshalOptions.marshal
    0.30GB 12.98% 99.79%     0.30GB 12.98%  bytes.growSlice
         0     0% 99.79%     0.30GB 12.98%  bytes.(*Buffer).Write
         0     0% 99.79%     0.30GB 12.98%  bytes.(*Buffer).grow
         0     0% 99.79%     2.02GB 86.81%  github.com/golang/protobuf/proto.Marshal (inline)
         0     0% 99.79%     2.02GB 86.81%  github.com/golang/protobuf/proto.marshalAppend
         0     0% 99.79%     0.89GB 38.31%  google.golang.org/grpc.(*PreparedMsg).Encode
         0     0% 99.79%     0.02GB  0.75%  google.golang.org/grpc.(*Server).handleRawConn.func1
         0     0% 99.79%     0.02GB  0.75%  google.golang.org/grpc.(*Server).serveStreams
         0     0% 99.79%     1.13GB 48.52%  google.golang.org/grpc.(*serverStream).SendMsg
         0     0% 99.79%     2.02GB 86.81%  google.golang.org/grpc.encode
         0     0% 99.79%     1.13GB 48.50%  google.golang.org/grpc.prepareMsg
         0     0% 99.79%     1.13GB 48.52%  google.golang.org/grpc/benchmark.(*testServer).UnconstrainedStreamingCall.func2
         0     0% 99.79%     2.02GB 86.81%  google.golang.org/grpc/encoding/proto.codec.Marshal
         0     0% 99.79%     0.28GB 12.23%  google.golang.org/grpc/internal/transport.(*http2Client).handleData
         0     0% 99.79%     0.29GB 12.28%  google.golang.org/grpc/internal/transport.(*http2Client).reader
         0     0% 99.79%     0.02GB  0.75%  google.golang.org/grpc/internal/transport.(*http2Server).HandleStreams
         0     0% 99.79%     0.02GB  0.75%  google.golang.org/grpc/internal/transport.(*http2Server).handleData
         0     0% 99.79%     1.13GB 48.52%  google.golang.org/grpc/interop/grpc_testing.(*benchmarkServiceStreamingCallServer).Send
         0     0% 99.79%     2.02GB 86.81%  google.golang.org/protobuf/proto.MarshalOptions.MarshalAppend
         0     0% 99.79%     0.89GB 38.31%  main.main
         0     0% 99.79%     0.89GB 38.31%  main.makeFuncUnconstrainedStreamPreloaded
         0     0% 99.79%     0.89GB 38.31%  main.unconstrainedStreamBenchmark
         0     0% 99.79%     0.89GB 38.31%  runtime.main

s-matyukevich avatar May 26 '23 20:05 s-matyukevich

In fact preloader works only on the client and not on the server. If you want a fix for that I can submit a PR. We can pass a parameter to the test server constructor with the list of requested response sizes and use ResponseSize property of the incoming request to take the prepared grpc response from a static map.

s-matyukevich avatar May 26 '23 20:05 s-matyukevich

Hello, thank you for raising this. We'd gladly accept and review a PR :)!

zasweq avatar Jun 06 '23 17:06 zasweq

Ok, I'll wait for https://github.com/grpc/grpc-go/pull/6299 to be merged first, as it added a mechanizm to pass additional data from clients to servers in benchmarks. If you are ok with this approach I'll use the same to pass preloader=on flag to the server.

s-matyukevich avatar Jun 06 '23 18:06 s-matyukevich

Sounds good, took a pass at that and wrote some minor nits but that PR LGTM otherwise.

zasweq avatar Jun 06 '23 19:06 zasweq