You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I'm not talking about effectiveness of Add from the point of view of creating a unique_ptr with metric before checking that it is already exists - this is covered in one of opened PRs.
I want to talk about prometheus::Labels passed as the first argument. prometheus::Labels is an alias to std::map which is constructed by the caller and later this map serves as a key in the Family::metrics_ map. If Add(...) finds out that this is a new set of labels than it will insert a new element into Family::metrics_ creating a deep copy of supplied Labels with all subsequent allocations of map nodes and strings (if they exceed SSO limit).
I think it would be better to pass Labels by value, so that Family::Add method would like: template <typename... Args> T& Add(Labels labels, Args&&... args) { return Add(std::move(labels), detail::make_unique<T>(args...)); }
Private overload of Family::Add could use emplace and also move these Labels into the key of a new element in Family::metrics_.
I could make a PR, the fix is obvious but I noted reading the discussions that the maintainer wants to preserve the ABI of library (which is good).
So I need to understand if such a PR would be welcomed - what strategy to implement it is OK? Maybe it is better to have a second overload of public Family::Add with the folowing declaration: template <typename... Args> T& Add(Labels&& labels, Args&&... args)
The text was updated successfully, but these errors were encountered:
Hi!
I've looked how prometheus_cpp is used by me and by other projects and noticed a common pattern when using Family::Add(...):
prometheus::Family<prometheus::Counter>& counter = .... /* Init with builder */
.....
counter.Add({{ "event", "some_event_name }, { "host", "some_host_name"}}).Increment();
I'm not talking about effectiveness of Add from the point of view of creating a unique_ptr with metric before checking that it is already exists - this is covered in one of opened PRs.
I want to talk about prometheus::Labels passed as the first argument. prometheus::Labels is an alias to std::map which is constructed by the caller and later this map serves as a key in the Family::metrics_ map. If Add(...) finds out that this is a new set of labels than it will insert a new element into Family::metrics_ creating a deep copy of supplied Labels with all subsequent allocations of map nodes and strings (if they exceed SSO limit).
I think it would be better to pass Labels by value, so that Family::Add method would like:
template <typename... Args>
T& Add(Labels labels, Args&&... args) {
return Add(std::move(labels), detail::make_unique<T>(args...));
}
Private overload of Family::Add could use emplace and also move these Labels into the key of a new element in Family::metrics_.
I could make a PR, the fix is obvious but I noted reading the discussions that the maintainer wants to preserve the ABI of library (which is good).
So I need to understand if such a PR would be welcomed - what strategy to implement it is OK? Maybe it is better to have a second overload of public Family::Add with the folowing declaration:
template <typename... Args>
T& Add(Labels&& labels, Args&&... args)
The text was updated successfully, but these errors were encountered: