Optimise relabeling by re-using memory #11147
Merged
Conversation
This file contains 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
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
Saves memory allocations. Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
This reduces memory allocations where the caller has a suitable slice available. Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
To reduce memory allocations. Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
rfratto
reviewed
Aug 11, 2022
Restore original behaviour in PopulateLabels, where we must not overwrite the input set. Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
Use a stack-based array for up to 16 source labels, which will be the vast majority of cases. Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
|
I pushed another commit which re-does "re-use source values slice" in a simpler way, and reduces allocations by 1 more. |
damnever
reviewed
Aug 15, 2022
| @@ -192,24 +192,29 @@ func (re Regexp) String() string { | |||
| // are applied in order of input. | |||
| // If a label set is dropped, nil is returned. | |||
| // May return the input labelSet modified. | |||
| func Process(labels labels.Labels, cfgs ...*Config) labels.Labels { | |||
| func Process(lbls labels.Labels, cfgs ...*Config) labels.Labels { | |||
| lb := labels.NewBuilder(nil) | |||
There was a problem hiding this comment.
I'd like another interface called ProcessWithBuilder.
There was a problem hiding this comment.
OK; is that relevant to this PR?
If not, it would belong in a separate PR with its own justification.
There was a problem hiding this comment.
I was thinking about using a pool of Builders here.
There was a problem hiding this comment.
That could also be done inside Process without changing the api.
codesome
approved these changes
Aug 18, 2022
Comment on lines
+207
to
+211
| var va [16]string | ||
| values := va[:0] | ||
| if len(cfg.SourceLabels) > cap(values) { | ||
| values = make([]string, 0, len(cfg.SourceLabels)) | ||
| } |
There was a problem hiding this comment.
How is this better than the previous?
There was a problem hiding this comment.
Also, would re-using the values slice between the relabel calls make it better?
There was a problem hiding this comment.
For relabeling rules with up to 16 source labels (surely most real-life examples), it uses an array on the stack, thus performs no heap allocation.
I tried re-using the slice; see 135e517. It is more complicated and does one allocation in every case, compared to zero in most cases.
Mama59
pushed a commit
to Arnoways/prometheus
that referenced
this pull request
Aug 25, 2022
* model/relabel: Add benchmark Signed-off-by: Bryan Boreham <bjboreham@gmail.com> * model/relabel: re-use Builder across relabels Saves memory allocations. Signed-off-by: Bryan Boreham <bjboreham@gmail.com> * labels.Builder: allow re-use of result slice This reduces memory allocations where the caller has a suitable slice available. Signed-off-by: Bryan Boreham <bjboreham@gmail.com> * model/relabel: re-use source values slice To reduce memory allocations. Signed-off-by: Bryan Boreham <bjboreham@gmail.com> * Unwind one change causing test failures Restore original behaviour in PopulateLabels, where we must not overwrite the input set. Signed-off-by: Bryan Boreham <bjboreham@gmail.com> * relabel: simplify values optimisation Use a stack-based array for up to 16 source labels, which will be the vast majority of cases. Signed-off-by: Bryan Boreham <bjboreham@gmail.com> * lint Signed-off-by: Bryan Boreham <bjboreham@gmail.com> Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
valyala
added a commit
to VictoriaMetrics/VictoriaMetrics
that referenced
this pull request
Oct 1, 2022
The benchmark name is BenchmarkApplyRelabelConfigs/kubernetes This benchmark has been copied from https://github.com/Arnoways/prometheus/blob/d521933053bdf68d252e365da9376706d04addcc/model/relabel/relabel_test.go#L505 See also prometheus/prometheus#11147
valyala
added a commit
to VictoriaMetrics/VictoriaMetrics
that referenced
this pull request
Oct 1, 2022
The benchmark name is BenchmarkApplyRelabelConfigs/kubernetes This benchmark has been copied from https://github.com/Arnoways/prometheus/blob/d521933053bdf68d252e365da9376706d04addcc/model/relabel/relabel_test.go#L505 See also prometheus/prometheus#11147
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Relabeling can be a large portion of Prometheus memory allocation, and hence overall performance.
Especially relabeling in service discovery.
Since relabelling works by applying different rules to the same label set, we can re-use slices and greatly reduce memory allocation. Also re-use the Builder itself since
NewBuilder()makes some speculative allocations.The benchmark is somewhat lengthy, due to including realistic data.