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.

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

(gogoproto.stdtime) = true causes low marshal performance #656

Open
TennyZhuang opened this issue Jan 9, 2020 · 3 comments
Open

(gogoproto.stdtime) = true causes low marshal performance #656

TennyZhuang opened this issue Jan 9, 2020 · 3 comments

Comments

@TennyZhuang
Copy link
Contributor

TennyZhuang commented Jan 9, 2020

gogo/protobuf/types.Timestamp is generated by protoc-gen-gogo and imports gogo/protobuf/proto.

To avoid cycling import, gogo/protobuf implement a fake timestamp .

type timestamp struct {
Seconds int64 `protobuf:"varint,1,opt,name=seconds,proto3" json:"seconds,omitempty"`
Nanos int32 `protobuf:"varint,2,opt,name=nanos,proto3" json:"nanos,omitempty"`
}
func (m *timestamp) Reset() { *m = timestamp{} }
func (*timestamp) ProtoMessage() {}
func (*timestamp) String() string { return "timestamp<string>" }

And it does not implement newMarshaler or Marshaler interface, which will cause the Marshal use the slowest path.

var info InternalMessageInfo
siz := info.Size(pb)
b := make([]byte, 0, siz)
return info.Marshal(b, pb, false)

Then the global marshal info lock will be performance bottleneck.

func getMarshalInfo(t reflect.Type) *marshalInfo {
marshalInfoLock.Lock()
u, ok := marshalInfoMap[t]
if !ok {
u = &marshalInfo{typ: t}
marshalInfoMap[t] = u
}
marshalInfoLock.Unlock()
return u
}

image

A simple workaround fix is #655

A better solution is use a real Timestamp instead of a fake timestamp.

To avoid cycling import, we can import golang/protobuf and use Timestamp generated by golang protobuf.

import ptimestamp "github.com/golang/protobuf/ptypes/timestamp"

type timestamp = ptimestamp.Timestamp

Then the performance will be highly improved.

@TennyZhuang
Copy link
Contributor Author

I'd like to fix it if the solution is acceptable.

@TennyZhuang
Copy link
Contributor Author

@awalterschulze @jmarais PTAL

@TennyZhuang TennyZhuang changed the title (gogoproto.stdtime) = true cause low marshal performance (gogoproto.stdtime) = true causes low marshal performance Jan 9, 2020
@xujianhai666
Copy link

watching

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants