Labels: reduce allocations when creating from TSDB WAL #13044
Conversation
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>
model/labels/labels.go
Outdated
| @@ -617,6 +617,12 @@ func (b *ScratchBuilder) Add(name, value string) { | |||
| b.add = append(b.add, Label{Name: name, Value: value}) | |||
| } | |||
|
|
|||
| // Add a name/value pair, using []byte instead of string. | |||
| // Intended for '-tags stringlabels'; this version purely so the old way still compiles. | |||
There was a problem hiding this comment.
| // Intended for '-tags stringlabels'; this version purely so the old way still compiles. | |
| // Intended for '-tags stringlabels'; this version has the same same (safe) semantics as Add. |
Hm - this is bit weird - unsafe is sometimes safe sometimes unsafe. Why not making this use yolo string as well?
There was a problem hiding this comment.
Like it feels we need to ensure liveness of those bytes on caller side of this no matter what tag (labels implementation) is used.
There was a problem hiding this comment.
We have to copy some time. stringlabels makes one block of memory with all the values in, so that's a copy. Previously all versions copied in UvarintStr().
I don't think it's a great idea to make the documentation say it's safe.
There was a problem hiding this comment.
I pushed a commit which I hope makes the description clearer. Let me know what you think.
I see a reduction from 218812154 B/op to 206498400 B/op. |
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
This is used when reading the WAL; by passing references into the buffer we can avoid copying strings under
-tags stringlabels. It's even better with #12304.I had to bump the number of iterations to get stable results on this benchmark:
Before:
After: