Commit graph

61 commits

Author SHA1 Message Date
Bryan Boreham a3e02f35d6 labels: extract common code between slice and stringlabels
This reduces bulk and should avoid issues if a fix is made in one file
and not the other.

A few methods now call `Range()` instead of `range`, but nothing
performance-sensitive.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-11-17 18:20:03 +00:00
Bryan Boreham 1bfb3ed062
Labels: reduce allocations when creating from TSDB WAL (#13044)
* Labels: reduce allocations when creating from TSDB

When reading the WAL, by passing references into the buffer we can avoid
copying strings under `-tags stringlabels`.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-11-14 11:36:35 +00:00
Oleksandr Redko fa90ca46e5 ci(lint): enable godot; append dot at the end of comments
Signed-off-by: Oleksandr Redko <Oleksandr_Redko@epam.com>
2023-10-31 19:53:38 +02:00
Goutham Veeramachaneni 86729d4d7b
Update exp package (#12650) 2023-09-21 22:53:51 +02:00
Bryan Boreham d6e1b1acdb
Merge pull request #12681 from prometheus/labels-unused-code
labels: remove some unused code
2023-08-14 15:48:17 +01:00
Bryan Boreham ce260b1fe1 labels: remove some unused code
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-08-13 15:14:53 +01:00
Bryan Boreham b5c6807fea Labels.Has quick check on first character
Exit early if we've gone past - labels are sorted in order.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-08-13 15:11:22 +01:00
Bryan Boreham 33aab1b2cc labels: extend benchmark for Has()
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-08-13 14:55:50 +01:00
Bryan Boreham e1115ae58d
labels: improve Get method for stringlabels build (#12485)
Inline one call to `decodeString`, and skip decoding the value string
until we find a match for the name.
Do a quick check on the first character in each string,
and exit early if we've gone past - labels are sorted in order.

Also improve tests and benchmark:
* labels: test Get with varying lengths - it's not typical for Prometheus labels to all be the same length.
* extend benchmark with label not found

---------

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-06-26 18:35:22 +01:00
Bryan Boreham 87d08abe11
labels: faster Compare function when using -tags stringlabels (#12451)
Instead of unpacking every individual string, we skip to the point
where there is a difference, going 8 bytes at a time where possible.

Add benchmark for Compare; extend tests too.

---------

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
Co-authored-by: Oleg Zaytsev <mail@olegzaytsev.com>
2023-06-20 20:58:47 +01:00
Oleg Zaytsev 6a18962cfa
mv labels_string.go labels_stringlabels.go (#12328)
This is a minor cosmetical change, but my IDE (and I guess many of them)
nests `labels_string.go` under `labels.go` because it assumes it's the
file generated by the `stringer` tool, which follows that naming
pattern.

Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
2023-06-13 09:38:00 +01:00
Michael Hoffmann 344c8ff97c
feat: dont compile regex matcher if we know its a literal (#12434)
labels: dont compile regex matcher if we know its a literal

Signed-off-by: Michael Hoffmann <mhoffm@posteo.de>

Co-authored-by: Sharad <sharadgaur@gmail.com>
2023-06-07 21:54:30 +01:00
Bryan Boreham a073e04a9b
Merge pull request #12366 from prometheus/release-2.44
Merge release 2.44 back to main
2023-05-16 18:06:29 +01:00
Bryan Boreham 7a48a266b6
labels: respect Set after Del in Builder (#12322)
* labels: respect Set after Del in Builder

The implementations are not symmetric between `Set()` and `Del()`, so
we must be careful. Add tests for this, both in labels and in relabel
where the issue was reported.

Also make the slice implementation consistent re `slices.Contains`.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-05-03 11:59:27 +01:00
cui fliter 276ca6a883 fix some comments
Signed-off-by: cui fliter <imcusg@gmail.com>
2023-04-25 14:19:16 +08:00
beorn7 5b53aa1108 style: Replace else if cascades with switch
Wiser coders than myself have come to the conclusion that a `switch`
statement is almost always superior to a statement that includes any
`else if`.

The exceptions that I have found in our codebase are just these two:

* The `if else` is followed by an additional statement before the next
  condition (separated by a `;`).
* The whole thing is within a `for` loop and `break` statements are
  used. In this case, using `switch` would require tagging the `for`
  loop, which probably tips the balance.

Why are `switch` statements more readable?

For one, fewer curly braces. But more importantly, the conditions all
have the same alignment, so the whole thing follows the natural flow
of going down a list of conditions. With `else if`, in contrast, all
conditions but the first are "hidden" behind `} else if `, harder to
spot and (for no good reason) presented differently from the first
condition.

I'm sure the aforemention wise coders can list even more reasons.

In any case, I like it so much that I have found myself recommending
it in code reviews. I would like to make it a habit in our code base,
without making it a hard requirement that we would test on the CI. But
for that, there has to be a role model, so this commit eliminates all
`if else` occurrences, unless it is autogenerated code or fits one of
the exceptions above.

Signed-off-by: beorn7 <beorn@grafana.com>
2023-04-19 17:22:31 +02:00
Bryan Boreham 1801cd4196 labels: small optimization to stringlabels
Add a fast path for the common case that a string is less than 127 bytes
long, to skip a shift and the loop.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-04-14 15:06:57 +00:00
Bryan Boreham 10cc60af01 labels: add ScratchBuilder.Overwrite for slice implementation
This is a method used by some downstream projects; it was created to
optimize the implementation in `labels_string.go` but we should have one
for both implementations so the same code works with either.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-04-13 11:07:54 +00:00
Ganesh Vernekar 5588cab8b2
Merge pull request #12173 from bboreham/builder-no-empty-labels
labels: simplify call to get Labels from Builder
2023-04-04 12:02:55 +05:30
Bryan Boreham e917202766 labels: make sure estimated size is not negative
Deleted labels are remembered, even if they were not in `base` or were
removed from `add`, so `base+add-del` could go negative.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-04-02 11:17:09 +01:00
Bryan Boreham ee1157c14a labels: shrink stack arrays in Builder.Range
Go spends some time initializing all the elements of these arrays to
zero, so reduce the size from 1024 to 128. This is still much bigger
than we ever expect for a set of labels.

(If someone does have more than 128 labels it will still work, but via
heap allocation.)

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-03-22 17:14:43 +00:00
Bryan Boreham b987afa7ef labels: simplify call to get Labels from Builder
It took a `Labels` where the memory could be re-used, but in practice
this hardly ever benefitted. Especially after converting `relabel.Process`
to `relabel.ProcessBuilder`.

Comparing the parameter to `nil` was a bug; `EmptyLabels` is not `nil`
so the slice was reallocated multiple times by `append`.

Lastly `Builder.Labels()` now estimates that the final size will depend
on labels added and deleted.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-03-22 17:05:20 +00:00
Bryan Boreham 3743d87c56 labels: cope with mutating Builder during Range call
Although we had a different slice, the underlying memory was the same so
any changes meant we could skip some values.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-03-16 13:28:15 +00:00
Bryan Boreham 3c4ab7a069 labels: add test for Builder.Range
Including mutating the Builder being Ranged over.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-03-16 13:25:55 +00:00
Julien Pivotto 5583c77b3a
Merge pull request #12095 from damnever/unnecessary-sort
Remove unnecessary sort
2023-03-09 13:12:02 +01:00
Xiaochao Dong (@damnever) 36fc1158b5 Remove unnecessary sort
Signed-off-by: Xiaochao Dong (@damnever) <the.xcdong@gmail.com>
2023-03-08 15:36:02 +08:00
Bryan Boreham d740abf0c6 model/labels: add Get and Range to Builder
This lets relabelling work on a `Builder` rather than converting to and
from `Labels` on every rule.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-03-07 17:20:54 +00:00
Bryan Boreham ff993b279a
Merge pull request #12073 from bboreham/slices-sort2
labels: use slices.Sort for better performance
2023-03-07 09:31:50 +00:00
Bryan Boreham 38c6d3da9f labels: use slices.Sort for better performance
The difference is modest, but we've used `slices.Sort` in lots of other
places so why not here.

name     old time/op    new time/op    delta
Builder    1.04µs ± 3%    0.95µs ± 3%   -8.27%  (p=0.008 n=5+5)

name     old alloc/op   new alloc/op   delta
Builder      312B ± 0%      288B ± 0%   -7.69%  (p=0.008 n=5+5)

name     old allocs/op  new allocs/op  delta
Builder      2.00 ± 0%      1.00 ± 0%  -50.00%  (p=0.008 n=5+5)

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-03-06 18:22:49 +00:00
Bryan Boreham a07a0be024 Add benchmark for labels.Builder
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-03-06 18:21:58 +00:00
Bryan Boreham 30297f0d9b stringlabels: size buffer for added labels
This makes the buffer the correct size for the common case that labels
have only been added. It will be too large for the case that labels are
changed, but the current buffer resize logic in `appendLabelTo` doubles
the buffer, so a small over-estimate is better.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-03-06 16:29:21 +00:00
Bryan Boreham 35026fb26d
Merge pull request #11746 from prometheus/remove-microbenchmarks
These benchmarks were testing things related to what Prometheus does, but not testing actual Prometheus code. 
Moved the label-copying benchmark into the labels package.
2023-02-23 12:33:24 +01:00
Bryan Boreham f03b8d0968 Add benchmark copying labels
Taken from previous tsdb/test/BenchmarkLabelsClone.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-02-22 16:36:45 +00:00
Bryan Boreham 6136ae67e0 labels: shrink by making internals a single string
This commit adds an alternate implementation for `labels.Labels`, behind
a build tag `stringlabels`.

Instead of storing label names and values as individual strings, they
are all concatenated into one string in this format:

    [len][name0][len][value0][len][name1][len][value1]...

The lengths are varint encoded so usually a single byte.

The previous `[]string` had 24 bytes of overhead for the slice and 16
for each label name and value; this one has 16 bytes overhead plus 1
for each name and value.

In `ScratchBuilder.Overwrite` and `Labels.Hash` we use an unsafe
conversion from string to byte slice. `Overwrite` is explicitly unsafe,
but for `Hash` this is a pure performance hack.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2023-02-22 15:34:23 +00:00
Bryan Boreham 10b27dfb84 Simplify IndexReader.Series interface
Instead of passing in a `ScratchBuilder` and `Labels`, just pass the
builder and the caller can extract labels from it. In many cases the
caller didn't use the Labels value anyway.

Now in `Labels.ScratchBuilder` we need a slightly different API: one
to assign what will be the result, instead of overwriting some other
`Labels`. This is safer and easier to reason about.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-12-19 15:22:09 +00:00
Bryan Boreham b10fd9aea3 model/labels: add a basic test for ScratchBuilder
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-12-19 15:22:09 +00:00
Bryan Boreham cbf432d2ac Update package labels tests for new labels.Labels type
Re-did the FromStrings test to avoid assumptions about how it works.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-12-19 15:22:09 +00:00
Bryan Boreham 617bee60f1 labels: use ScratchBuilder in ReadLabels
Instead of relying on being able to append to it like a slice.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-12-19 15:22:09 +00:00
Bryan Boreham 2b8b8d9ac7 labels: new methods to work without access to internals
Without changing the definition of `labels.Labels`, add methods which
enable code using it to work without knowledge of the internals.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-12-19 15:22:09 +00:00
Bryan Boreham ea7345a09c labels: improve comment on Builder.Set
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-12-19 15:22:09 +00:00
Bryan Boreham a19b369f9e labels: avoid lint warning on New()
This code is a bit cleaner.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-12-19 15:22:09 +00:00
Julien Pivotto bb323db613
Merge pull request #11074 from damnever/fix/datamodelvalidation
Validate the metric name and label names
2022-12-08 14:31:12 +01:00
Xiaochao Dong (@damnever) 9979024a30 Report error if the series contains invalid metric names or labels during scrape
Signed-off-by: Xiaochao Dong (@damnever) <the.xcdong@gmail.com>
2022-12-08 20:01:20 +08:00
Bryan Boreham 8d4140a06e labels: note that Hash may change
For performance reasons we may use a different implementation of Hash()
in future, so note this so callers can be warned.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-11-28 16:17:32 +00:00
Bryan Boreham 5421c778ba labels: in tests use labels.FromStrings
Replacing code which assumes the internal structure of `Labels`.

Add a convenience function `EmptyLabels()` which is more efficient than
calling `New()`.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-09-09 13:34:49 +02:00
Cosrider bef6556ca5
delete redundant alias (#11180)
Signed-off-by: Cosrider <cosrider7@gmail.com>

Signed-off-by: Cosrider <cosrider7@gmail.com>
2022-08-31 15:50:38 +02:00
Bryan Boreham 8b863c42dd
Optimise relabeling by re-using memory (#11147)
* 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>
2022-08-19 15:27:52 +05:30
Bryan Boreham a7f19b5775 labels: add a test for JSON and YAML marshalling
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-08-03 11:18:02 +02:00
Bryan Boreham 10699c37a3 labels: test BytesWithoutLabels does not remove __name__ by default
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-08-03 11:18:02 +02:00
Bryan Boreham d46ef0aa8e labels: tweak BenchmarkLabels_Get()
So the benchmark works without requiring `Labels` internals to be a slice.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-08-03 11:18:02 +02:00