Memory Performance Improvements #78
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mattkahrs
reviewed
Jun 10, 2025
Co-authored-by: Mary Paskhaver <mary.paskhaver@gmail.com> Co-authored-by: mattkahrs <mattkahrs@gmail.com>
…metheus-haskell into mattp/memory-performance
|
|
||
| We're running into problems with unexpectedly high memory use in `prometheus-client`, so I've been investigating improving the behavior. | ||
|
|
||
| Specifically, we're notificing a significant increase in the amount of time and memory allocated while calling `exportMetricsToText`. |
There was a problem hiding this comment.
Suggested change
| Specifically, we're notificing a significant increase in the amount of time and memory allocated while calling `exportMetricsToText`. | |
| Specifically, we're noticing a significant increase in the amount of time and memory allocated while calling `exportMetricsToText`. |
| ## `MetricImpl` | ||
|
|
||
| A `Metric` had a field `construct :: IO (s, IO [SampleGroup])`. | ||
| To avoid tuple, we introduce `MetricImpl s` which uses bang patterns on the fields. |
There was a problem hiding this comment.
Suggested change
| To avoid tuple, we introduce `MetricImpl s` which uses bang patterns on the fields. | |
| To avoid the tuple, we introduce `MetricImpl s` which uses bang patterns on the fields. |
| This means that, should anything write to the `TVar`, all of the computation will be retried. | ||
| This is *probably* bad - we want to capture the state of the metrics *now*, and then return the `SampleGroup`, rather than allowing the computation to be aborted and retried multiple times. | ||
|
|
||
| The first two problems are based on the sizeof the histogram, so the number of buckets. |
There was a problem hiding this comment.
Suggested change
| The first two problems are based on the sizeof the histogram, so the number of buckets. | |
| The first two problems are based on the size of the histogram, so the number of buckets. |
|
|
||
| The first two problems are based on the sizeof the histogram, so the number of buckets. | ||
| Every additional bucket causes another float to be rendered into a string, and another list cons cell to be held on to. | ||
| Since number of buckets is likely small, this is probably not a big deal. |
There was a problem hiding this comment.
Suggested change
| Since number of buckets is likely small, this is probably not a big deal. | |
| Since the number of buckets is likely small, this is probably not a big deal. |
fimad
reviewed
Aug 2, 2025
| @@ -0,0 +1,91 @@ | |||
| # Investigating Memory Use in `prometheus-client` | |||
Owner
There was a problem hiding this comment.
This feels like an odd file to check into main. I think this would be more appropriate as the PR description.
Owner
|
Do these changes resolve the memory issues are seeing? There are several changes described in |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.