-
Notifications
You must be signed in to change notification settings - Fork 101
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
metrics #149
metrics #149
Conversation
turuslan
commented
Jun 9, 2021
- instance count and list annotations for metrics.
- count libp2p instances: tcp/noise/yamuxed connection, upgrader session, yamux stream, gossip stream and peer context.
- metrics test.
- fix unused warning (yamuxed_connection.cpp).
static auto &count() { \ | ||
static auto &count = ([]() -> auto & { \ | ||
auto &state{::libp2p::metrics::instance::State::get()}; \ | ||
std::lock_guard lock{state.mutex}; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Object creation or destruction locks and unlocks a mutex. That looks to be a too heavy operation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
count()
's count
is static local variable, so this lambda with mutex locking is called only once, to insert key-counter pair into map.
Generally LIBP2P_METRICS_INSTANCES_COUNT constructor/destructor is just checking "static guard variable" (was static variable initialized) followed by atomic increment/decrement.
LIBP2P_METRICS_INSTANCES_LIST is heavier and always locks mutex, because it inserts/erases list. Developer chooses whether to keep list of all type
instances or not (or just count with LIBP2P_METRICS_INSTANCES_COUNT).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
disabled by default via cmake option
Better to have this feature disabled by default in build time, and to be enabled explicitly by CMake variable and preprocessor, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commented earlier about making this feature a non-default option
Signed-off-by: turuslan <turuslan.devbox@gmail.com>