Skip to content

scrape: Optimize protobuf scrape parsing #15726

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
Closed
wants to merge 1 commit into from
Closed

scrape: Optimize protobuf scrape parsing #15726

wants to merge 1 commit into from

Conversation

bwplotka
Copy link
Member

@bwplotka bwplotka commented Dec 30, 2024 •
edited
Loading

Fixes #14668

Depends on #15720

  • prompb: Generate client proto using Opaque API, for parsing only.. This also removes all uses of gogo for both client_model and prompb generated Go code for Prometheus proto exp format.

@bwplotka bwplotka force-pushed the protoparseopt branch 2 times, most recently from 02554ce to b055d64 Compare December 30, 2024 10:15
@bwplotka
Copy link
Member Author

bwplotka commented Dec 30, 2024 •
edited
Loading

Just moving out of gogo proto (using new go protobuf API)

benchstat -filter="/fmt:PromProto" scrape-loop-v1.txt scrape-loop-v2.txt
goos: darwin
goarch: arm64
pkg: github.com/prometheus/prometheus/scrape
cpu: Apple M1 Pro
                                 │ scrape-loop-v1.txt │         scrape-loop-v2.txt         │
                                 │       sec/op       │   sec/op     vs base               │
ScrapeLoopAppend/fmt=PromProto-2          54.48µ ± 8%   66.39µ ± 2%  +21.88% (p=0.002 n=6)

                                 │ scrape-loop-v1.txt │         scrape-loop-v2.txt          │
                                 │        B/op        │     B/op      vs base               │
ScrapeLoopAppend/fmt=PromProto-2         75.58Ki ± 1%   57.03Ki ± 3%  -24.55% (p=0.002 n=6)

                                 │ scrape-loop-v1.txt │         scrape-loop-v2.txt         │
                                 │     allocs/op      │  allocs/op   vs base               │
ScrapeLoopAppend/fmt=PromProto-2           810.0 ± 0%   1110.0 ± 0%  +37.04% (p=0.002 n=6)

@bwplotka bwplotka force-pushed the protoparseopt branch 6 times, most recently from c265bf5 to d0b175a Compare December 30, 2024 10:53
@bwplotka
Copy link
Member Author

Just OpaqueAPI (no lazy decoding, no string reuse) results.

benchstat -filter="/fmt:PromProto" scrape-loop-v1.txt scrape-loop-v3.txt
goos: darwin
goarch: arm64
pkg: github.com/prometheus/prometheus/scrape
cpu: Apple M1 Pro
                                 │ scrape-loop-v1.txt │         scrape-loop-v3.txt         │
                                 │       sec/op       │   sec/op     vs base               │
ScrapeLoopAppend/fmt=PromProto-2          54.48µ ± 8%   68.65µ ± 4%  +26.02% (p=0.002 n=6)

                                 │ scrape-loop-v1.txt │         scrape-loop-v3.txt          │
                                 │        B/op        │     B/op      vs base               │
ScrapeLoopAppend/fmt=PromProto-2         75.58Ki ± 1%   58.47Ki ± 3%  -22.64% (p=0.002 n=6)

                                 │ scrape-loop-v1.txt │         scrape-loop-v3.txt         │
                                 │     allocs/op      │  allocs/op   vs base               │
ScrapeLoopAppend/fmt=PromProto-2           810.0 ± 0%   1211.0 ± 0%  +49.51% (p=0.002 n=6)

Getting worse so far (:

@bwplotka
prompb: Generate client proto using Opaque API, for parsing only.
e7bde7a 
Signed-off-by: bwplotka <bwplotka@gmail.com>
@bwplotka
Copy link
Member Author

Ok, I will proceed with the experiments further, but this path of hacking registries is dangerous and difficult. You pretty much have to have only one generated Go code for one proto file, even if some dependency vendors one. e.g. I can't pass custom registry to unmarshallers without changing globals. We got away with this in the past as we used both new proto API and gogo proto, each having separate registries. See https://protobuf.dev/reference/go/faq/#namespace-conflict

Generally all APIs will be opaque by default, so it's going to be fun for client_golang which returns client_model dto as an API cc @ArthurSens @kakkoyun

I think I will try similar approach but with a separate proto file in the next PR.

@bwplotka bwplotka closed this Dec 30, 2024
This was referenced Dec 30, 2024
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@bwplotka