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

perf: improve the memory usage of histogram #606

Merged
merged 2 commits into from
Jan 16, 2024

Conversation

xsbchen
Copy link
Contributor

@xsbchen xsbchen commented Jan 11, 2024

We have a use case where it generates a huge amount of histograms, each has more than 10 labels, we found the memory usage is quite high, under this scenario, we did an experiment and found that removing bucketExemplars field from histograms could save about 27% memory usage for each histogram instance, hope the maintainers could review and consider it
SeaTalk_IMG_20240111_195229

@SimenB
Copy link
Collaborator

SimenB commented Jan 11, 2024

Should we do this for the other metric types as well?

@xsbchen
Copy link
Contributor Author

xsbchen commented Jan 12, 2024

Should we do this for the other metric types as well?

I checked only Counter and Histogram support the enableExemplars option, and Counter already handled this case.

@SimenB
Copy link
Collaborator

SimenB commented Jan 12, 2024

@voltbit thoughts on this? Seems reasonable to me, but I have to admit I'm a bit unsure how this code works 😅

Copy link
Collaborator

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my read of it, this would work the same of exemplars are enabled, so should be good to go 👍

I'd like an extra pair of eyes on it, though

lib/histogram.js Show resolved Hide resolved
@voltbit
Copy link
Contributor

voltbit commented Jan 12, 2024

Looks good to me. The data struct being there when exemplars are disabled was an overlook on my side I think.

@xsbchen
Copy link
Contributor Author

xsbchen commented Jan 16, 2024

@zbjornson @siimon master, plz help to review it if you are free, thanks.

@SimenB SimenB merged commit a88a5c3 into siimon:master Jan 16, 2024
13 checks passed
@xsbchen xsbchen deleted the histogram-memory-improve branch January 16, 2024 11:30
@SimenB
Copy link
Collaborator

SimenB commented Mar 26, 2024

https://github.com/siimon/prom-client/releases/tag/v15.1.1

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.

3 participants