jupp0r / prometheus-cpp

Prometheus Client Library for Modern C++
Other
934 stars 330 forks source link

Summary benchmarks bias #384

Open dmitryikh opened 4 years ago

dmitryikh commented 4 years ago

Hello! Firs rof all Thanks for such great library! We use it in production and love it.

I have some concerns about correctness of measurements of summary::observe method.

Here is how you measure it: summary_bench.cc:

while (state.KeepRunning()) {
    auto observation = d(gen);
    auto start = std::chrono::high_resolution_clock::now();
    summary.Observe(observation);
    auto end = std::chrono::high_resolution_clock::now();

    auto elapsed_seconds =
        std::chrono::duration_cast<std::chrono::duration<double>>(end - start);
    state.SetIterationTime(elapsed_seconds.count());
  }

The concern is that you make as many observations per second as possibly. But internal implementation of Summary implies some buffer (it was array<500> when I looked into the code) and it runs some heave work (that can imply memory allocations) when the buffer is full. That means that the performance of summary::observe method depends on the rate of calls.

I'm personally interested in the summary performant on the order of magnitude like 1000 RPS. And it seems like your benchmark is too conservative to show real numbers for my rates.

What do you think? Is it feasible to add such summary's benchmarks for different RPS loads?

bigc2000 commented 3 years ago

@dmitryikh ,Yes, I found the same problem. The function costs not a stable . The duration of 99-percentile is too much higher, while the average seems good,even 95-percentile. much depends on the rate of calls, indead the buckets count internal. The copy ops is key to the performance.