Hi all, sorry to disrupt this discussion. Before stumbling upon this thread, I had worked on a separate fork <https://github.com/prometheus/prometheus/compare/main...mircodz:prometheus:deprecate-gogo> to deprecate gogo in favor of csproto, as compiling it using enableunsafedecode=true seems <https://github.com/mircodz/go-proto-bench> to give performance even better than vtproto. (Note, I have only compared the performance of csproto and vtproto to the official proto generator, and not gogo). As of now the branch compiles and passes all tests, but I haven't gone through the code to check for possible optimizations that could arise from migrating away from gogo. Would you be interested in a pull request? As mentioned above, this would be also a good opportunity to cleanup the proto generation code using buf.
P.S.: This would depend on a change in prometheus/client_model, but would allow removing the duplicate proto definition in the repository. King Regards, Mirco De Zorzi. On Monday, February 5, 2024 at 10:58:17 AM UTC+1 Bartłomiej Płotka wrote: > Issue for reference: https://github.com/prometheus/prometheus/issues/11908 > > Kind Regards, > Bartek Płotka > > On Saturday, February 3, 2024 at 12:56:09 PM UTC Bartłomiej Płotka wrote: > >> We did a bit of testing for remote write 2.0 work (e.g. here >> <https://github.com/bwplotka/go-proto-bench>) for gogo vs different >> plugins, and vtproto is the most promising even with more pointers. >> >> We have to get rid of nullables, yes (more pointers, pore separate >> objects on heap, generally more allocs), but even for our current remote >> write (especially with interning) there is literally not many slices (with >> many elements) that use custom types. And even if there are (e.g. >> []*TimeSeries) those objects might be worth to keep separate on the heap. >> This is also what protobuf direction will be, given the vision of "opaque >> API" (ability to load/allocate/ parts of proto message in a lazy way). >> >> Furthermore we hit a roadblock a bit, as a apparently "optional >> <https://github.com/gogo/protobuf/issues/713>" proto3 option does not >> work with proto. This makes it maybe even more worth doing. (e.g. PRW 2.0 >> optional timestamp int64 would not be able to have valid value of 0 etc). >> >> I think I would consider doing this work this summer, perhaps as a GSoC >> mentorship >> <https://github.com/cncf/mentoring/blob/main/programs/summerofcode/2024.md>. >> Anyone would like to mentor/co-mentor that with me or Callum? (: >> >> Kind Regards, >> Bartek Plotka >> >> On Wednesday, November 29, 2023 at 2:38:14 AM UTC [email protected] >> wrote: >> >>> As part of all the remote write proto changes I've been working on I >>> tried out moving us off of gogoproto, cherry picking Austin's original >>> changes into a new branch off of the current main branch. >>> >>> As Tom mentioned, the main reason for using gogoproto is that `repeated >>> SomeMessageType = n;` fields within messages are generated as slices of >>> concrete types rather than slices of pointers, which makes it much easier >>> to write code that avoids extra memory allocations. From what I've hacked >>> together, we can get similar (or potentially better) performance using >>> vtproto and their pooling feature, but it's going to be a big refactoring >>> effort. >>> >>> It might, however, be worth it. It looks to me like even with slightly >>> more allocations the proto marshalling is faster and the marshalled message >>> is smaller. I'll push what I have later this week when I'm more confident >>> it's working correctly. >>> >>> On Thursday, July 8, 2021 at 2:42:41 AM UTC-7 Frederic Branczyk wrote: >>> >>>> I think I'd be most useful to rebase, and create a PR from this, then >>>> we can see whether tests pass and we can run prombench (although I don't >>>> think there are any perf tests that involve the proto parts). Then we can >>>> discuss on there and figure out where to take this. >>>> >>>> Thank you so much for the work you have already put into this! >>>> >>>> On Mon, 21 Jun 2021 at 19:53, Austin Cawley-Edwards < >>>> [email protected]> wrote: >>>> >>>>> I've updated my branch ( >>>>> https://github.com/austince/prometheus/tree/feat/drop-gogo) to use >>>>> both the vitess plugin and the buf tool, which indeed fit very nicely >>>>> together. >>>>> >>>>> I've only updated the code enough for it to compile, have not >>>>> investigated the semantic differences. This is likely the furthest I'll >>>>> be >>>>> able to take this for a bit, so feedback and playing around are welcome >>>>> and >>>>> appreciated if this is where we'd like protobuf in Prometheus to go :) >>>>> >>>>> Best, >>>>> Austin >>>>> >>>>> On Thu, Jun 17, 2021 at 12:56 PM Frederic Branczyk <[email protected]> >>>>> wrote: >>>>> >>>>>> I have heard great thing, but haven’t used it. Wrongfully thought >>>>>> that they are mutually exclusive but turns out they are actually >>>>>> complementary: >>>>>> https://twitter.com/fredbrancz/status/1405192828049838080?s=21 >>>>>> >>>>>> We should probably do an investigation of the combination. >>>>>> >>>>>> On Thu 17. Jun 2021 at 18:26, Austin Cawley-Edwards < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> Just saw this on the CNCF blog as well, seems like a promising >>>>>>> library. >>>>>>> >>>>>>> Tangentially, have you heard of https://github.com/bufbuild/buf? It >>>>>>> seems much nicer than compiling with shell scripts and protoc. >>>>>>> >>>>>>> >>>>>>> -- 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/9124d162-2d04-434f-ba9e-9fac6b656f5cn%40googlegroups.com.
