From 0ecf3a1701f9f9337acb98d6d5ec0ac04a0182a1 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Wed, 30 Aug 2023 11:38:48 +0100 Subject: [PATCH 1/3] This delays obtaining a sample from steady_clock unless it is configured to be needed. Excessive sampling of the steady_clock has an impact on node performance so should only be done when needed. --- nano/lib/stats.cpp | 71 ++++++++++++++++++++++++++-------------------- 1 file changed, 41 insertions(+), 30 deletions(-) diff --git a/nano/lib/stats.cpp b/nano/lib/stats.cpp index 25e5e3b081..41b95acb85 100644 --- a/nano/lib/stats.cpp +++ b/nano/lib/stats.cpp @@ -391,8 +391,6 @@ void nano::stats::update (uint32_t key_a, uint64_t value) static file_writer log_count (config.log_counters_filename); static file_writer log_sample (config.log_samples_filename); - auto now (std::chrono::steady_clock::now ()); - nano::unique_lock lock{ stat_mutex }; if (!stopped) { @@ -403,40 +401,53 @@ void nano::stats::update (uint32_t key_a, uint64_t value) entry->counter.add (value); entry->count_observers.notify (old, entry->counter.get_value ()); - std::chrono::duration duration = now - log_last_count_writeout; - if (config.log_interval_counters > 0 && duration.count () > config.log_interval_counters) - { - log_counters_impl (log_count); - log_last_count_writeout = now; - } - - // Samples - if (config.sampling_enabled && entry->sample_interval > 0) + auto has_interval_counter = [&] () { + return config.log_interval_counters > 0; + }; + auto has_sampling = [&] () { + return config.sampling_enabled && entry->sample_interval > 0; + }; + if (has_interval_counter () || has_sampling ()) { - entry->sample_current.add (value, false); - - std::chrono::duration duration = now - entry->sample_start_time; - if (duration.count () > entry->sample_interval) + auto now = std::chrono::steady_clock::now (); // Only sample clock if necessary as this impacts node performance due to frequent usage + if (has_interval_counter ()) { - entry->sample_start_time = now; - - // Make a snapshot of samples for thread safety and to get a stable container - entry->sample_current.set_timestamp (std::chrono::system_clock::now ()); - entry->samples.push_back (entry->sample_current); - entry->sample_current.set_value (0); - - if (!entry->sample_observers.empty ()) + std::chrono::duration duration = now - log_last_count_writeout; + if (duration.count () > config.log_interval_counters) { - auto snapshot (entry->samples); - entry->sample_observers.notify (snapshot); + log_counters_impl (log_count); + log_last_count_writeout = now; } + } + + // Samples + if (has_sampling ()) + { + entry->sample_current.add (value, false); - // Log sink - duration = now - log_last_sample_writeout; - if (config.log_interval_samples > 0 && duration.count () > config.log_interval_samples) + std::chrono::duration duration = now - entry->sample_start_time; + if (duration.count () > entry->sample_interval) { - log_samples_impl (log_sample); - log_last_sample_writeout = now; + entry->sample_start_time = now; + + // Make a snapshot of samples for thread safety and to get a stable container + entry->sample_current.set_timestamp (std::chrono::system_clock::now ()); + entry->samples.push_back (entry->sample_current); + entry->sample_current.set_value (0); + + if (!entry->sample_observers.empty ()) + { + auto snapshot (entry->samples); + entry->sample_observers.notify (snapshot); + } + + // Log sink + duration = now - log_last_sample_writeout; + if (config.log_interval_samples > 0 && duration.count () > config.log_interval_samples) + { + log_samples_impl (log_sample); + log_last_sample_writeout = now; + } } } } From 38ea6b629a71819155e86a6719cab81888787185 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Wed, 30 Aug 2023 17:03:35 +0100 Subject: [PATCH 2/3] Only update timestamp when sampling is enabled as this has a performance impact --- nano/lib/stats.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/nano/lib/stats.cpp b/nano/lib/stats.cpp index 41b95acb85..6085581b43 100644 --- a/nano/lib/stats.cpp +++ b/nano/lib/stats.cpp @@ -395,18 +395,17 @@ void nano::stats::update (uint32_t key_a, uint64_t value) if (!stopped) { auto entry (get_entry_impl (key_a, config.interval, config.capacity)); - - // Counters - auto old (entry->counter.get_value ()); - entry->counter.add (value); - entry->count_observers.notify (old, entry->counter.get_value ()); - auto has_interval_counter = [&] () { return config.log_interval_counters > 0; }; auto has_sampling = [&] () { return config.sampling_enabled && entry->sample_interval > 0; }; + + // Counters + auto old (entry->counter.get_value ()); + entry->counter.add (value, has_sampling ()); // Only update timestamp when sampling is enabled as this has a performance impact + entry->count_observers.notify (old, entry->counter.get_value ()); if (has_interval_counter () || has_sampling ()) { auto now = std::chrono::steady_clock::now (); // Only sample clock if necessary as this impacts node performance due to frequent usage From 37359c5f2aa871e2ae7be75e7820bc204777a41d Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Wed, 30 Aug 2023 17:45:33 +0100 Subject: [PATCH 3/3] Changing use of map to unordered_map since we don't need ordering and it was impacting performance. --- nano/lib/stats.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nano/lib/stats.hpp b/nano/lib/stats.hpp index 324bdfe84c..e644658051 100644 --- a/nano/lib/stats.hpp +++ b/nano/lib/stats.hpp @@ -9,10 +9,10 @@ #include #include -#include #include #include #include +#include namespace nano { @@ -440,7 +440,7 @@ class stats final nano::stats_config config; /** Stat entries are sorted by key to simplify processing of log output */ - std::map> entries; + std::unordered_map> entries; std::chrono::steady_clock::time_point log_last_count_writeout{ std::chrono::steady_clock::now () }; std::chrono::steady_clock::time_point log_last_sample_writeout{ std::chrono::steady_clock::now () };