Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If register can used many times?【help wanted】 #628

Open
chenqingguo opened this issue Jan 3, 2023 · 2 comments
Open

If register can used many times?【help wanted】 #628

chenqingguo opened this issue Jan 3, 2023 · 2 comments

Comments

@chenqingguo
Copy link

If repeat call register leading to performance issues?
prometheus::BuildGauge().Name(name).Register(*prometheus_registry).Add(labels).Increment();
I have to register once and then add many times?
auto f=prometheus::BuildGauge().Name(name).Register(*prometheus_registry); while(true) { f.Add(labels).Increment(); }

@chenqingguo chenqingguo changed the title If register can used many times?【help】 If register can used many times?【help wanted】 Jan 3, 2023
@gjasny
Copy link
Collaborator

gjasny commented Jan 3, 2023

Have you tried any of the options? The first one should start to fail the second time:

TEST(RegistryTest, throw_for_same_family_name) {
const auto same_name = std::string{"same_name"};
Registry registry{Registry::InsertBehavior::Throw};
EXPECT_NO_THROW(BuildCounter().Name(same_name).Register(registry));
EXPECT_ANY_THROW(BuildCounter().Name(same_name).Register(registry));
}

You pay a penalty for calling .Add:

auto f=prometheus::BuildGauge().Name(name).Register(*prometheus_registry);
while(true) {
  f.Add(labels).Increment();
}

This is the optimal case:

auto f=prometheus::BuildGauge().Name(name).Register(*prometheus_registry);
auto some_specific_counter = f.Add(labels);
while(true) {
  some_specific_counter.Increment();
}

See in the example:

for (;;) {
std::this_thread::sleep_for(std::chrono::seconds(1));
const auto random_value = std::rand();
if (random_value & 1) tcp_rx_counter.Increment();
if (random_value & 2) tcp_tx_counter.Increment();
if (random_value & 4) udp_rx_counter.Increment();
if (random_value & 8) udp_tx_counter.Increment();
const std::array<std::string, 4> methods = {"GET", "PUT", "POST", "HEAD"};
auto method = methods.at(random_value % methods.size());
// dynamically calling Family<T>.Add() works but is slow and should be
// avoided
http_requests_counter.Add({{"method", method}}).Increment();
}

@chenqingguo
Copy link
Author

chenqingguo commented Jan 4, 2023

Register's default param is Merge,so the same name and labels just return the existing one but not fail.

  /// \brief name Create a new registry.
  ///
  /// \param insert_behavior How to handle families with the same name.
  explicit Registry(InsertBehavior insert_behavior = InsertBehavior::Merge);

Here is part of my test

void MonitorImpl::RecordGauge(const string& name, const Lables& labels)
{
    // LOG_INFO("name:%s,labels:%s.", name.c_str(), lable2string(labels).c_str());
    prometheus::BuildGauge().Name(name).Register(*prometheus_registry).Add(labels).Increment();
}
void MonitorImpl::counter(const Lables& labels, double value)
{
    mycounter->Add(labels).Increment(value);
}

TEST(common, Monitor)
{
GetMonitor()->Init();
auto counter1 = []() {
        int i = 1000000;
        while (i--) {
            GetMonitor()->RecordCounter("counter", { { "k1", "v1" }, { "k23", "v23" } }, i);
        }
        LOG_INFO("RecordCounter done.");
    };
    auto counter2 = []() {
        int i = 1000000;
        while (i--) {
            GetMonitor()->RecordCounter("counter", { { "k1", "v1" }, { "k23", "v23" } }, i);
        }
        LOG_INFO("RecordCounter done.");
    };
//First register then use. 
GetMonitor()->reg(string("count11"));
    auto counter3 = []() {
        int i = 1000000;
        while (i--) {
            GetMonitor()->counter( { { "k1", "v1" }, { "k23", "v23" } }, i);
        }
        LOG_INFO("counter done.");
    };
    auto counter4 = []() {
        int i = 1000000;
        while (i--) {
            GetMonitor()->counter( { { "k1", "v1" }, { "k23", "v23" } }, i);
        }
        LOG_INFO("RecordCounter done.");
    };
vector<thread> threads;
    threads.push_back(thread(counter1));
    threads.push_back(thread(counter2));
    std::chrono::system_clock::time_point t1 = std::chrono::system_clock::now();
    for (int i = 0; i < threads.size(); i++) {
        threads[i].join();
    }
    std::chrono::system_clock::time_point t2 = std::chrono::system_clock::now();
    auto d1=std::chrono::duration_cast<std::chrono::microseconds>(t2-t1).count();
    LOG_INFO("d1:%d",d1);
    sleep(1);
    threads.clear();
    threads.push_back(thread(counter3));
    threads.push_back(thread(counter4));
    std::chrono::system_clock::time_point t3 = std::chrono::system_clock::now();
    for (int i = 0; i < threads.size(); i++) {
        threads[i].join();
    }
    std::chrono::system_clock::time_point t4 = std::chrono::system_clock::now();
    auto d2=std::chrono::duration_cast<std::chrono::microseconds>(t4-t3).count();
    LOG_INFO("d2:%d",d2);
}

d1 cost time:1944940 microseconds,
d2 cost time:1767512 microseconds,
I wonder if the gap is from calling register(have mutex)?
Sometimes Add can't be called before using ,because labels are not known until using it.
I know this is the best way to use:

auto f=prometheus::BuildGauge().Name(name).Register(*prometheus_registry);
auto some_specific_counter = f.Add(labels);
while(true) {
  some_specific_counter.Increment();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants