Skip to content
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.

Sign up for GitHub

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

Jump to bottom

encoding/proto: simplify & optimize proto codec #3958

Merged
merged 1 commit into from Oct 14, 2020
Merged

encoding/proto: simplify & optimize proto codec #3958

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
66 changes: 2 additions & 64 deletions encoding/proto/proto.go
View file
Open in desktop
Expand Up @@ -21,9 +21,6 @@
package proto

import (
"math"
"sync"

"github.com/golang/protobuf/proto"
"google.golang.org/grpc/encoding"
)
Expand All @@ -38,73 +35,14 @@ func init() {
// codec is a Codec implementation with protobuf. It is the default codec for gRPC.
type codec struct{}

type cachedProtoBuffer struct {
lastMarshaledSize uint32
proto.Buffer
}

func capToMaxInt32(val int) uint32 {
if val > math.MaxInt32 {
return uint32(math.MaxInt32)
}
return uint32(val)
}

func marshal(v interface{}, cb *cachedProtoBuffer) ([]byte, error) {
protoMsg := v.(proto.Message)
newSlice := make([]byte, 0, cb.lastMarshaledSize)

cb.SetBuf(newSlice)
cb.Reset()
if err := cb.Marshal(protoMsg); err != nil {
return nil, err
}
out := cb.Bytes()
cb.lastMarshaledSize = capToMaxInt32(len(out))
return out, nil
}

func (codec) Marshal(v interface{}) ([]byte, error) {
if pm, ok := v.(proto.Marshaler); ok {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is breaking existing code generate with gogoproto.customtype = "MyType", where a user can write custom marshaling/unmarshaling code to have more control. IMHO should not be removed.

https://github.com/gogo/protobuf/blob/master/custom_types.md#custom-type-method-signatures

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gogoproto is not officially supported by gRPC-Go.

If you want a codec that suits your needs better, it's easy to install your own. Just copy/paste this code into another package, update the parts that are important, and import it in your main package after any grpc imports. (If you don't control main, you can give it a new name and use the Codec Call-/Dial-/Server- Options.)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok thanks, I will give it a try. I have also created an issue about this. Please feel free to close it #4192 I will also comment the solution there after testing it.

// object can marshal itself, no need for buffer
return pm.Marshal()
}

cb := protoBufferPool.Get().(*cachedProtoBuffer)
out, err := marshal(v, cb)

// put back buffer and lose the ref to the slice
cb.SetBuf(nil)
protoBufferPool.Put(cb)
return out, err
return proto.Marshal(v.(proto.Message))
}

func (codec) Unmarshal(data []byte, v interface{}) error {
protoMsg := v.(proto.Message)
protoMsg.Reset()

if pu, ok := protoMsg.(proto.Unmarshaler); ok {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same problem, removing this will break use case of gogoproto.customtype = "MyType"

// object can unmarshal itself, no need for buffer
return pu.Unmarshal(data)
}

cb := protoBufferPool.Get().(*cachedProtoBuffer)
cb.SetBuf(data)
err := cb.Unmarshal(protoMsg)
cb.SetBuf(nil)
protoBufferPool.Put(cb)
return err
return proto.Unmarshal(data, v.(proto.Message))
}

func (codec) Name() string {
return Name
}

var protoBufferPool = &sync.Pool{
New: func() interface{} {
return &cachedProtoBuffer{
Buffer: proto.Buffer{},
lastMarshaledSize: 16,
}
},
}