Optimise handling of integers and floats for counters, gauges, summaries#176
Optimise handling of integers and floats for counters, gauges, summaries#176NelsonVides merged 1 commit intomasterfrom
Conversation
Codecov ReportAttention: Patch coverage is
🚀 New features to boost your workflow:
|
2fbd3d7 to
df43cb9
Compare
The point is that these tables keep separate positions in the tuple for integers and for floats, as on the integer position we can use the optimised `ets:update_counter/3,4`, and probabilistically integers are the most common input. The issue is that under some circumstances, integer values would be appended to the float position, which, while correct, misses the opportunity for the optimisation. Note too that gauges could take the value `undefined` to effectively block changes until explicitly set. This was implemented, but not tested, and in some edge-cases (the integer/float optimisation) it would be buggy.
df43cb9 to
c739c85
Compare
|
|
||
| insert_metric(Registry, Name, LabelValues, Value, ConflictCB) -> | ||
| prometheus_metric:check_mf_exists(?TABLE, Registry, Name, LabelValues), | ||
| insert_metric_int(Registry, Name, LabelValues, Value, ConflictCB) -> |
There was a problem hiding this comment.
Not that great in compiling beam code in my brain but wouldn't it help to hint the compiler with a is_integer(Value) guard here or is it sufficient enough to have a separate function for it? 🤔
(Same all the other specific functions)
There was a problem hiding this comment.
I'd say this function is only called from another function that already did the is_integer(Value) guard, so I'm pretty sure the compiler already generated optimised code for it. Haven't verify it though 🤔
|
@prometheus-erl/prometheus-erl any reviewer here? Just planning a new release after this minor optimisation is merged :) |
Sorry my bad! Never pressed approve after your last reply! |
The point is that these tables keep separate positions in the tuple for
integers and for floats, as on the integer position we can use the
optimised
ets:update_counter/3,4, and probabilistically integers arethe most common input.
The issue is that under some circumstances, integer values would be
appended to the float position, which, while correct, misses the
opportunity for the optimisation.
Note too that gauges could take the value
undefinedto effectivelyblock changes until explicitly set. This was implemented, but not
tested, and in some edge-cases (the integer/float optimisation) it would
be buggy.
Split from #173