This looks promising for us to potentially move off of gogo proto without a performance hit (maybe even an improvement): https://vitess.io/blog/2021-06-03-a-new-protobuf-generator-for-go/
On Fri 30. Apr 2021 at 00:01, Austin Cawley-Edwards <[email protected]> wrote: > Totally on the benchmarking, that would be great! > > I've got a work in progress branch here > <https://github.com/prometheus/prometheus/compare/main...austince:feat/drop-gogo#diff-43227f90be395387b346f5e08e104df4b98cbddf0ad29d11a847172845d1473aR301>, > but it looks like it will be rather intensive, unfortunately. The main > issues seem to be: > * The use of pointers instead of structs, as @Tom Wilkie > <[email protected]> mentioned before. It looks like the lib likely > won't change this <https://github.com/golang/protobuf/issues/1225> > * Finding a replacement for the `gogoproto.nullable` annotation (perhaps > envoy's > validation plugin <https://github.com/envoyproxy/protoc-gen-validate> > might be just that?) > * Size() methods are no longer generated > * Equals() no longer works out of the box, as protos are now not > comparable <https://github.com/golang/protobuf/issues/531> (and I'm struggling > to use the recommended tools > <https://github.com/prometheus/prometheus/commit/7ab41d638535cb5e5d4c2e84f20409629d001422#> > ) > > If anyone has pointers/ ideas here, much appreciated! > > Bringing this up at the next Cortex community is probably the best way: >> https://github.com/cortexproject/cortex#community-meetings >> > > Cool, see you there! > > On Thu, Apr 29, 2021 at 5:00 AM Tom Wilkie <[email protected]> wrote: > >> Sounds good! As this is pretty performance sensitive, I think we'd want >> to benchmark any changes to this code before we merge it. Let me know if I >> can help there.. >> >> > You mentioned there are others for Cortex @[email protected]? >> >> Bringing this up at the next Cortex community is probably the best way: >> https://github.com/cortexproject/cortex#community-meetings >> >> Cheers! >> >> Tom >> >> On Wed, Apr 28, 2021 at 5:39 PM Austin Cawley-Edwards < >> [email protected]> wrote: >> >>> This surprised me a little, and sounds awesome - but I can't find any >>>> more information about it. Does v1.4.0 generate code for the serialisation >>>> / deserialisation function or still rely on Golang reflection? Does it >>>> allow for the neat tricks to get rid of pointers and all the allocations? >>>> Anything I can read about this would be super useful. >>> >>> >>> Hmm, I'm not sure on all the specifics, but here's what I've found so >>> far: >>> * Golang reflection has largely been replaced by the protoreflect >>> <https://pkg.go.dev/google.golang.org/protobuf/reflect/protoreflect> >>> package (which *does not *use Golang reflection under the hood) >>> * SerDe uses the protoreflect package via dedicated modules for >>> different formats (json >>> <https://pkg.go.dev/google.golang.org/[email protected]/encoding/protojson>, >>> text >>> <https://pkg.go.dev/google.golang.org/[email protected]/encoding/prototext>, >>> wire >>> <https://pkg.go.dev/google.golang.org/[email protected]/encoding/protowire> >>> ) >>> * Not sure about the specific tricks/ if the overuse of pointers + >>> allocations are still present, but there is now a first-class lib for >>> creating compiler plugins >>> <https://pkg.go.dev/google.golang.org/protobuf/compiler/protogen> that >>> might be what you're looking for? Looks like there are many plugins that >>> use it already, judging by the imports >>> <https://pkg.go.dev/google.golang.org/protobuf/compiler/protogen?tab=importedby> >>> >>> Off the top of my head, I would think checking in with the Cortex and >>>> Thanos projects is probably a good idea ... >>>> >>> >>> +1 sounds good! Are there specific people I should ping on this list? Or >>> open up issues in their repos? You mentioned there are others for Cortex @ >>> [email protected]? >>> >>> On Wed, Apr 28, 2021 at 8:05 AM Tom Wilkie <[email protected]> wrote: >>> >>>> > Along with that, many of the performance boosts that are provided by >>>> gogo have been addressed in the official Go lib, golang/protobuf >>>> <https://github.com/golang/protobuf>, as of v1.4.0 >>>> <https://blog.golang.org/protobuf-apiv2>. >>>> >>>> This surprised me a little, and sounds awesome - but I can't find any >>>> more information about it. Does v1.4.0 generate code for the serialisation >>>> / deserialisation function or still rely on Golang reflection? Does it >>>> allow for the neat tricks to get rid of pointers and all the allocations? >>>> Anything I can read about this would be super useful. >>>> >>>> > Off the top of my head, I would think checking in with the Cortex and >>>> Thanos projects is probably a good idea, I know both have a good amount of >>>> optimizations optimizing allocations, so it would be good to check that >>>> these would still be possible. >>>> >>>> From a Cortex PoV, we have copies of these protos so I don't think this >>>> would be too much of a problem, but I'd defer to people who know better >>>> than me. >>>> >>>> Cheers >>>> >>>> Tom >>>> >>>> On Wed, Apr 28, 2021 at 10:07 AM Frederic Branczyk <[email protected]> >>>> wrote: >>>> >>>>> I'd be very happy with this. One consideration that we should take >>>>> (certainly not blocking this but should keep in mind) is how this would >>>>> affect immediate downstream users. Off the top of my head, I would think >>>>> checking in with the Cortex and Thanos projects is probably a good idea, I >>>>> know both have a good amount of optimizations optimizing allocations, so >>>>> it >>>>> would be good to check that these would still be possible. >>>>> >>>>> On Tue, 27 Apr 2021 at 22:51, Austin Cawley-Edwards < >>>>> [email protected]> wrote: >>>>> >>>>>> >>>>>> Hi all, >>>>>> >>>>>> The protobuf library used in Prometheus, gogo/protobuf >>>>>> <https://github.com/gogo/protobuf>, is no longer actively maintained >>>>>> (activity largely stopped pre-2020, looking for new ownership >>>>>> <https://github.com/gogo/protobuf/issues/691>). Along with that, >>>>>> many of the performance boosts that are provided by gogo have been >>>>>> addressed in the official Go lib, golang/protobuf >>>>>> <https://github.com/golang/protobuf>, as of v1.4.0 >>>>>> <https://blog.golang.org/protobuf-apiv2>. >>>>>> >>>>>> Many projects that used gogo/protobuf have since switched to the >>>>>> official lib (ex: Istio <https://github.com/istio/istio/pull/17132>, >>>>>> Envoyproxy >>>>>> <https://github.com/envoyproxy/go-control-plane/issues/213>), >>>>>> largely for eco-system compatibility reasons now that performance is not >>>>>> a >>>>>> factor. The gogo-compiled protobufs are not compatible with any >>>>>> golang-compiled protobufs, and vice-versa. This makes consuming external >>>>>> protobuf APIs impossible unless they also maintain a gogo version, which >>>>>> is >>>>>> not common. >>>>>> >>>>>> I'm wondering if anyone has done work in this area recently, and if >>>>>> the community agrees it's a net benefit switching to the official >>>>>> golang/protobuf implementation. >>>>>> >>>>>> *What this would mean for Prometheus* >>>>>> >>>>>> Looking at the history of protobuf in Prometheus, it seems like both >>>>>> golang/protobuf and gogo/protobuf were until the end of 2017, when it >>>>>> then >>>>>> made sense to consolidate onto gogo (#3346 >>>>>> <https://github.com/prometheus/prometheus/issues/3346>) (#3394 >>>>>> <https://github.com/prometheus/prometheus/pull/3394>). >>>>>> >>>>>> As noted in the above issues, the official golang/protobuf package is >>>>>> still present in vendored files, so it is just the Prometheus protos that >>>>>> would need to be updated. The build procedures (mainly >>>>>> scripts/genproto.sh >>>>>> <https://github.com/prometheus/prometheus/blob/75e505babb9bbcefd8849e945814d35c7ce97e9f/scripts/genproto.sh>) >>>>>> have not changed much since the 2017 shift, so the work would be mainly >>>>>> adjusting this back to use golang/protobuf and recompiling the `prompb` >>>>>> package. >>>>>> >>>>>> Does anyone know of other necessary changes/ complications that I'm >>>>>> missing? >>>>>> >>>>>> Thanks all for your time, >>>>>> Austin >>>>>> >>>>>> -- >>>>>> You received this message because you are subscribed to the Google >>>>>> Groups "Prometheus Developers" group. >>>>>> To unsubscribe from this group and stop receiving emails from it, >>>>>> send an email to [email protected]. >>>>>> To view this discussion on the web visit >>>>>> https://groups.google.com/d/msgid/prometheus-developers/38caa51d-e88a-489b-a045-54144cd1a03fn%40googlegroups.com >>>>>> <https://groups.google.com/d/msgid/prometheus-developers/38caa51d-e88a-489b-a045-54144cd1a03fn%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>>> . >>>>>> >>>>> -- >>>>> You received this message because you are subscribed to the Google >>>>> Groups "Prometheus Developers" group. >>>>> To unsubscribe from this group and stop receiving emails from it, send >>>>> an email to [email protected]. >>>>> To view this discussion on the web visit >>>>> https://groups.google.com/d/msgid/prometheus-developers/CAOs1Umy%2BhjN10r9q_MOwZ2wHL-0MEDZWw_2SCUPW%2B%3D8xpKvdQw%40mail.gmail.com >>>>> <https://groups.google.com/d/msgid/prometheus-developers/CAOs1Umy%2BhjN10r9q_MOwZ2wHL-0MEDZWw_2SCUPW%2B%3D8xpKvdQw%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>> . >>>>> >>>> -- You received this message because you are subscribed to the Google Groups "Prometheus Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAOs1Umwjo3KdK1%3DCKvQev%2BO-wyV%3Def9K6wWK6smEtewGS1nRNw%40mail.gmail.com.
