From c37c57c68a94d4ed2a77d4f5a794c75c0ee49564 Mon Sep 17 00:00:00 2001
From: Piotr Balcer <piotr.balcer@intel.com>
Date: Fri, 13 Dec 2019 11:50:04 +0100
Subject: [PATCH] obj: introduce transient/persistent statistics enabling
With this patch, all transients statistics will be enabled by default,
and users will have ability to selectively disable or enable both types
of statistics. This changes the stats.enabled entry point, but in
a backward compatible way.
---
doc/libpmemobj/pmemobj_ctl_get.3.md | 23 ++++----
src/include/libpmemobj/ctl.h | 9 +++-
src/libpmemobj/stats.c | 53 ++++++++++++++++---
src/libpmemobj/stats.h | 51 +++++++++++++++---
src/test/obj_ctl_stats/obj_ctl_stats.c | 26 +++++++--
.../obj_fragmentation2/obj_fragmentation2.c | 3 --
6 files changed, 134 insertions(+), 31 deletions(-)
diff --git a/doc/libpmemobj/pmemobj_ctl_get.3.md b/doc/libpmemobj/pmemobj_ctl_get.3.md
index fa374def8..336d4e12a 100644
--- a/doc/libpmemobj/pmemobj_ctl_get.3.md
+++ b/doc/libpmemobj/pmemobj_ctl_get.3.md
@@ -328,14 +328,17 @@ naming in the application (e.g. when writing a library that uses libpmemobj).
The required class identifier will be stored in the `class_id` field of the
`struct pobj_alloc_class_desc`.
-stats.enabled | rw | - | int | int | - | boolean
+stats.enabled | rw | - | enum pobj_stats_enabled | enum pobj_stats_enabled | - |
+string
-Enables or disables runtime collection of statistics. Statistics are not
-recalculated after enabling; any operations that occur between disabling and
-re-enabling will not be reflected in subsequent values.
+Enables or disables runtime collection of statistics. There are two types of
+statistics: persistent and transient ones. Persistent statistics survive pool
+restarts, whereas transient ones don't. Statistics are not recalculated after
+enabling; any operations that occur between disabling and re-enabling will not
+be reflected in subsequent values.
-Statistics are disabled by default. Enabling them may have non-trivial
-performance impact.
+Only transient statistics are enabled by default. Enabling persistent statistics
+may have non-trivial performance impact.
stats.heap.curr_allocated | r- | - | uint64_t | - | - | -
@@ -343,6 +346,8 @@ Reads the number of bytes currently allocated in the heap. If statistics were
disabled at any time in the lifetime of the heap, this value may be
inaccurate.
+This is a persistent statistic.
+
stats.heap.run_allocated | r- | - | uint64_t | - | - | -
Reads the number of bytes currently allocated using run-based allocation
@@ -354,9 +359,9 @@ This is a transient statistic and is rebuilt every time the pool is opened.
stats.heap.run_active | r- | - | uint64_t | - | - | -
-Reads the number of bytes currently occupied by all memory blocks occupied by
-runs, including both allocated and free space, i.e., all space that's not
-occupied by huge blocks.
+Reads the number of bytes currently occupied by all run memory blocks, including
+both allocated and free space, i.e., this is all the all space that's not
+occupied by huge allocations.
This value is a sum of all allocated and free run memory. In systems where
memory is efficiently used, `run_active` should closely track
diff --git a/src/include/libpmemobj/ctl.h b/src/include/libpmemobj/ctl.h
index 7d01f74f9..e055cc484 100644
--- a/src/include/libpmemobj/ctl.h
+++ b/src/include/libpmemobj/ctl.h
@@ -1,5 +1,5 @@
/*
- * Copyright 2017-2018, Intel Corporation
+ * Copyright 2017-2019, Intel Corporation
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -166,6 +166,13 @@ struct pobj_alloc_class_desc {
unsigned class_id;
};
+enum pobj_stats_enabled {
+ POBJ_STATS_ENABLED_TRANSIENT,
+ POBJ_STATS_ENABLED_BOTH,
+ POBJ_STATS_ENABLED_PERSISTENT,
+ POBJ_STATS_DISABLED,
+};
+
#ifndef _WIN32
/* EXPERIMENTAL */
int pmemobj_ctl_get(PMEMobjpool *pop, const char *name, void *arg);
diff --git a/src/libpmemobj/stats.c b/src/libpmemobj/stats.c
index ede15d06c..cfb34e57c 100644
--- a/src/libpmemobj/stats.c
+++ b/src/libpmemobj/stats.c
@@ -60,9 +60,43 @@ CTL_READ_HANDLER(enabled)(void *ctx,
{
PMEMobjpool *pop = ctx;
- int *arg_out = arg;
+ enum pobj_stats_enabled *arg_out = arg;
- *arg_out = pop->stats->enabled > 0;
+ *arg_out = pop->stats->enabled;
+
+ return 0;
+}
+
+/*
+ * stats_enabled_parser -- parses the stats enabled type
+ */
+static int
+stats_enabled_parser(const void *arg, void *dest, size_t dest_size)
+{
+ const char *vstr = arg;
+ enum pobj_stats_enabled *enabled = dest;
+ ASSERTeq(dest_size, sizeof(enum pobj_stats_enabled));
+
+ int bool_out;
+ if (ctl_arg_boolean(arg, &bool_out, sizeof(bool_out)) == 0) {
+ *enabled = bool_out ?
+ POBJ_STATS_ENABLED_BOTH : POBJ_STATS_DISABLED;
+ return 0;
+ }
+
+ if (strcmp(vstr, "disabled") == 0) {
+ *enabled = POBJ_STATS_DISABLED;
+ } else if (strcmp(vstr, "both") == 0) {
+ *enabled = POBJ_STATS_ENABLED_BOTH;
+ } else if (strcmp(vstr, "persistent") == 0) {
+ *enabled = POBJ_STATS_ENABLED_PERSISTENT;
+ } else if (strcmp(vstr, "transient") == 0) {
+ *enabled = POBJ_STATS_ENABLED_TRANSIENT;
+ } else {
+ ERR("invalid enable type");
+ errno = EINVAL;
+ return -1;
+ }
return 0;
}
@@ -77,14 +111,19 @@ CTL_WRITE_HANDLER(enabled)(void *ctx,
{
PMEMobjpool *pop = ctx;
- int arg_in = *(int *)arg;
-
- pop->stats->enabled = arg_in > 0;
+ pop->stats->enabled = *(enum pobj_stats_enabled *)arg;
return 0;
}
-static const struct ctl_argument CTL_ARG(enabled) = CTL_ARG_BOOLEAN;
+static const struct ctl_argument CTL_ARG(enabled) = {
+ .dest_size = sizeof(enum pobj_stats_enabled),
+ .parsers = {
+ CTL_ARG_PARSER(sizeof(enum pobj_stats_enabled),
+ stats_enabled_parser),
+ CTL_ARG_PARSER_END
+ }
+};
static const struct ctl_node CTL_NODE(stats)[] = {
CTL_CHILD(heap),
@@ -105,7 +144,7 @@ stats_new(PMEMobjpool *pop)
return NULL;
}
- s->enabled = 0;
+ s->enabled = POBJ_STATS_ENABLED_TRANSIENT;
s->persistent = &pop->stats_persistent;
VALGRIND_ADD_TO_GLOBAL_TX_IGNORE(s->persistent, sizeof(*s->persistent));
s->transient = Zalloc(sizeof(struct stats_transient));
diff --git a/src/libpmemobj/stats.h b/src/libpmemobj/stats.h
index 54378bdad..e3390be05 100644
--- a/src/libpmemobj/stats.h
+++ b/src/libpmemobj/stats.h
@@ -38,6 +38,7 @@
#define LIBPMEMOBJ_STATS_H 1
#include "ctl.h"
+#include "libpmemobj/ctl.h"
#ifdef __cplusplus
extern "C" {
@@ -53,25 +54,59 @@ struct stats_persistent {
};
struct stats {
- int enabled;
+ enum pobj_stats_enabled enabled;
struct stats_transient *transient;
struct stats_persistent *persistent;
};
#define STATS_INC(stats, type, name, value) do {\
- if ((stats)->enabled)\
- util_fetch_and_add64((&(stats)->type->name), (value));\
+ STATS_INC_##type(stats, name, value);\
+} while (0)
+
+#define STATS_INC_transient(stats, name, value) do {\
+ if ((stats)->enabled == POBJ_STATS_ENABLED_TRANSIENT ||\
+ (stats)->enabled == POBJ_STATS_ENABLED_BOTH)\
+ util_fetch_and_add64((&(stats)->transient->name), (value));\
+} while (0)
+
+#define STATS_INC_persistent(stats, name, value) do {\
+ if ((stats)->enabled == POBJ_STATS_ENABLED_PERSISTENT ||\
+ (stats)->enabled == POBJ_STATS_ENABLED_BOTH)\
+ util_fetch_and_add64((&(stats)->persistent->name), (value));\
} while (0)
#define STATS_SUB(stats, type, name, value) do {\
- if ((stats)->enabled)\
- util_fetch_and_sub64((&(stats)->type->name), (value));\
+ STATS_SUB_##type(stats, name, value);\
+} while (0)
+
+#define STATS_SUB_transient(stats, name, value) do {\
+ if ((stats)->enabled == POBJ_STATS_ENABLED_TRANSIENT ||\
+ (stats)->enabled == POBJ_STATS_ENABLED_BOTH)\
+ util_fetch_and_sub64((&(stats)->transient->name), (value));\
+} while (0)
+
+#define STATS_SUB_persistent(stats, name, value) do {\
+ if ((stats)->enabled == POBJ_STATS_ENABLED_PERSISTENT ||\
+ (stats)->enabled == POBJ_STATS_ENABLED_BOTH)\
+ util_fetch_and_sub64((&(stats)->persistent->name), (value));\
} while (0)
#define STATS_SET(stats, type, name, value) do {\
- if ((stats)->enabled)\
- util_atomic_store_explicit64((&(stats)->type->name), (value),\
- memory_order_release);\
+ STATS_SET_##type(stats, name, value);\
+} while (0)
+
+#define STATS_SET_transient(stats, name, value) do {\
+ if ((stats)->enabled == POBJ_STATS_ENABLED_TRANSIENT ||\
+ (stats)->enabled == POBJ_STATS_ENABLED_BOTH)\
+ util_atomic_store_explicit64((&(stats)->transient->name),\
+ (value), memory_order_release);\
+} while (0)
+
+#define STATS_SET_persistent(stats, name, value) do {\
+ if ((stats)->enabled == POBJ_STATS_ENABLED_PERSISTENT ||\
+ (stats)->enabled == POBJ_STATS_ENABLED_BOTH)\
+ util_atomic_store_explicit64((&(stats)->persistent->name),\
+ (value), memory_order_release);\
} while (0)
#define STATS_CTL_LEAF(type, name)\
diff --git a/src/test/obj_ctl_stats/obj_ctl_stats.c b/src/test/obj_ctl_stats/obj_ctl_stats.c
index f7bbf2327..7e443a190 100644
--- a/src/test/obj_ctl_stats/obj_ctl_stats.c
+++ b/src/test/obj_ctl_stats/obj_ctl_stats.c
@@ -79,7 +79,7 @@ main(int argc, char *argv[])
size_t run_allocated = 0;
ret = pmemobj_ctl_get(pop, "stats.heap.run_allocated", &run_allocated);
UT_ASSERTeq(ret, 0);
- UT_ASSERTeq(allocated, run_allocated);
+ UT_ASSERT(run_allocated /* 2 allocs */ > allocated /* 1 alloc */);
pmemobj_free(&oid);
@@ -87,10 +87,9 @@ main(int argc, char *argv[])
UT_ASSERTeq(ret, 0);
UT_ASSERTeq(allocated, 0);
- allocated = 0;
ret = pmemobj_ctl_get(pop, "stats.heap.run_allocated", &run_allocated);
UT_ASSERTeq(ret, 0);
- UT_ASSERTeq(allocated, run_allocated);
+ UT_ASSERT(run_allocated /* 2 allocs */ > allocated /* 1 alloc */);
TX_BEGIN(pop) {
oid = pmemobj_tx_alloc(1, 0);
@@ -102,6 +101,27 @@ main(int argc, char *argv[])
UT_ASSERTeq(ret, 0);
UT_ASSERTeq(allocated, oid_size);
+ enum pobj_stats_enabled enum_enabled;
+ ret = pmemobj_ctl_get(pop, "stats.enabled", &enum_enabled);
+ UT_ASSERTeq(enabled, POBJ_STATS_ENABLED_BOTH);
+ UT_ASSERTeq(ret, 0);
+
+ run_allocated = 0;
+ ret = pmemobj_ctl_get(pop, "stats.heap.run_allocated", &run_allocated);
+ UT_ASSERTeq(ret, 0);
+
+ enum_enabled = POBJ_STATS_ENABLED_PERSISTENT; /* transient disabled */
+ ret = pmemobj_ctl_set(pop, "stats.enabled", &enum_enabled);
+ UT_ASSERTeq(ret, 0);
+
+ ret = pmemobj_alloc(pop, &oid, 1, 0, NULL, NULL);
+ UT_ASSERTeq(ret, 0);
+
+ size_t tmp = 0;
+ ret = pmemobj_ctl_get(pop, "stats.heap.run_allocated", &tmp);
+ UT_ASSERTeq(ret, 0);
+ UT_ASSERTeq(tmp, run_allocated); /* shouldn't change */
+
pmemobj_close(pop);
DONE(NULL);
diff --git a/src/test/obj_fragmentation2/obj_fragmentation2.c b/src/test/obj_fragmentation2/obj_fragmentation2.c
index 89252345b..809e08bc1 100644
--- a/src/test/obj_fragmentation2/obj_fragmentation2.c
+++ b/src/test/obj_fragmentation2/obj_fragmentation2.c
@@ -236,9 +236,6 @@ main(int argc, char *argv[])
objects = ZALLOC(sizeof(PMEMoid) * MAX_OBJECTS);
UT_ASSERTne(objects, NULL);
- int enabled = 1;
- pmemobj_ctl_set(pop, "stats.enabled", &enabled);
-
workloads[w](pop);
/* this is to trigger global recycling */
--
GitLab