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

Avoid temporary objects #683

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Avoid temporary objects #683

wants to merge 1 commit into from

Conversation

gjasny
Copy link
Collaborator

@gjasny gjasny commented Dec 28, 2023

When collecting many metrics, the intermediate std::vector<MetricFamily> objects can heavily grow in size and cause memory fragmentation (see #646). This PR implements direct text-serialization into a buffer array whose buffers are 1 MByte at maximum.

The extreme memory fragmentation that I saw with the glibc allocator as well as the huge memory spikes during collection are now gone. There are now multiple other optimization possibilities, but this is the bare minimum needed.

@gjasny
Copy link
Collaborator Author

gjasny commented Dec 28, 2023

@owent, @esigo, @lalitb: FYI: this PR would change the Collectable signature.

Did you ever run into problems with excessive number of metrics? It's nice to see that prometheus-cpp can cope with 2 million counters. But do such setups exist in the wild?

@owent
Copy link

owent commented Dec 29, 2023

@owent, @esigo, @lalitb: FYI: this PR would change the Collectable signature.

Did you ever run into problems with excessive number of metrics? It's nice to see that prometheus-cpp can cope with 2 million counters. But do such setups exist in the wild?

Is there any way to tell otel-cpp whether to use the new class Serializer to get Serializer? I found no macro to check the ABI changes.

@gjasny gjasny marked this pull request as draft March 17, 2024 19:35
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

Successfully merging this pull request may close these issues.

2 participants