proto: avoid deep defensive copy when marshaling

[irfansharif]

The following pattern is oft-repeated:

if p := st.Proto(); p != nil {
    bytes, err := proto.Marshal(p)
    ...
}

Notably it appears in both transport.WriteStatus implementations. The
earlier revision with proto.Clone() is unnecessary for the immediate
marshaling as the proto message is not mutated. The inner calls to the
reflect package shows up on CPU profiles in addition to a tiny
allocation cost that can be easily avoided.

An alternate Status.ProtoClone() method is provided for when a deep
copy is in fact necessary, namely when the proto message is the be
subsequently mutated. One such example of this is Status.WithDetails
where the embedded proto is annotated with additional detail messages.

NB: This is a breaking API change if imports depend on this specific
behaviour. Alternatively another accessor can be added.

[irfansharif]

(pprof) list WriteStatus
Total: 7.97GB
ROUTINE ======================== google.golang.org/grpc/transport.(*http2Server).WriteStatus in /Users/irfansharif/Software/src/google.golang.org/grpc/transport/http2_server.go
         0    97.50MB (flat, cum)  1.19% of Total
         .          .    793:                   Name:  "grpc-status",
         .          .    794:                   Value: strconv.Itoa(int(st.Code())),
         .          .    795:           })
         .          .    796:   t.hEnc.WriteField(hpack.HeaderField{Name: "grpc-message", Value: encodeGrpcMessage(st.Message())})
         .          .    797:
         .    97.50MB    798:   if p := st.Proto(); p != nil && len(p.Details) > 0 {
         .          .    799:           stBytes, err := proto.Marshal(p)
         .          .    800:           if err != nil {
         .          .    801:                   // TODO: return error instead, when callers are able to handle it.
         .          .    802:                   panic(err)
         .          .    803:           }

Running pprof -alloc_space on a toy pinger program: https://github.com/irfansharif/pinger

[MakMukhi]

@irfansharif Seems like this PR is trying to optimize the error path. Am I understanding this right? If so, we'd rather not get into this. What do you think?

[thelinuxfoundation]

Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement.

After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.

• User @irfansharif isn't covered by a CLA. They will need to complete the form at https://identity.linuxfoundation.org/projects/cncf

Regards,
The Linux Foundation CLA GitHub bot