protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

`(gogoproto.stdtime) = true` causes low marshal performance

Open TennyZhuang opened this issue 5 years ago • 3 comments

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 .

https://github.com/gogo/protobuf/blob/5628607bb4c51c3157aacc3a50f0ab707582b805/proto/timestamp_gogo.go#L38-L46

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

https://github.com/gogo/protobuf/blob/5628607bb4c51c3157aacc3a50f0ab707582b805/proto/table_marshal.go#L2951-L2954

Then the global marshal info lock will be performance bottleneck.

https://github.com/gogo/protobuf/blob/5628607bb4c51c3157aacc3a50f0ab707582b805/proto/table_marshal.go#L107-L116

image

A simple workaround fix is https://github.com/gogo/protobuf/pull/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 avatar Jan 09 '20 12:01 TennyZhuang

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

TennyZhuang avatar Jan 09 '20 12:01 TennyZhuang

@awalterschulze @jmarais PTAL

TennyZhuang avatar Jan 09 '20 12:01 TennyZhuang

watching

xujianhai666 avatar Jan 15 '20 16:01 xujianhai666