prometheus
Update label validation to run once per metric
This is a minor performance improvement during the label parsing stages of metric handling.
When running a profiler on an application that utilizes this library, we noticed that this validation was 1/3 of the time spent in startLabelName and was an easy minor performance win.
When parsing metric files with more significant numbers of tags, this validation scales poorly and leads to a significant number of maps being created/iterated through. This may cause a slight decrease in performance when parsing invalidly tagged metrics in some cases, but will increase performance on the happy path. This performance result is not noticeably faster in the performance test below due to to a low number of tags used in the unit tests.
Bench results: This branch:
pkg: github.com/prometheus/common/expfmt
cpu: Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
BenchmarkTextParse
BenchmarkTextParse-16 6176 190301 ns/op
BenchmarkParseError
BenchmarkParseError-16 23068 53793 ns/op
PASS
Main:
pkg: github.com/prometheus/common/expfmt
cpu: Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
BenchmarkTextParse
BenchmarkTextParse-16 5448 193167 ns/op
BenchmarkParseError
BenchmarkParseError-16 22438 54203 ns/op
PASS
When tested in isolation a more clear performance improvement is shown. The happy path test is tested with with 25 metrics, each containing 8 tags. The error case is tested with one metric with 9 tags, the last being a duplicate of the first. This branch:
pkg: github.com/prometheus/common/expfmt
cpu: Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
BenchmarkTextParse
BenchmarkTextParse-16 9082 113003 ns/op
BenchmarkParseError
BenchmarkParseError-16 294361 4102 ns/op
PASS
Main:
pkg: github.com/prometheus/common/expfmt
cpu: Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
BenchmarkTextParse
BenchmarkTextParse-16 7752 140930 ns/op
BenchmarkParseError
BenchmarkParseError-16 253125 5157 ns/op
PASS
Tagging for review: @roidelapluie
Signed-off-by: jdhudson3 [email protected]
Thanks! I think the current path is more 'logical' and easy to use than the proposal. Does this really cause performance issues?
I can see what you mean that the initial approach fits in the code a little more obviously, I do think though that this fits the overall flow of the state machine as well.
I can't speak for anyone else's use of the package, but in our case we are parsing a significant number of metrics that each contain a minimum of 8 tags. This at minimum creates/destroys 7 extra hashmaps with 28 extra assignments per metric and scales as O(n^2).
In our case this is significant enough to care about, as it represents an 8% overall increase on our applications performance.
Attached below is a screenshot of our profiler, currently map specific operations take up 38% of the time to parse a metric label. This proposal should cut it to ~8%.
