Hi all,
I was adding some statistics to net/nimble/host, and I encountered a few
"snags." I wanted to describe the problems I ran into and propose a few
solutions. As always, please feel free to let the vehemence fly in your
replies :). Also, this is probably going to be a long and wordy email,
so I apologize upfront.
First, let me summarize my understanding of the sys/stats interface. If
anything here is incorrect, that may render the remainder of my email
irrelevant.
1. A group of stats is defined using the following set of macros:
STATS_SECT_START(struct_name)
STATS_SECT_ENTRY(stat_name)
STATS_SECT_END(struct_name)
This creates a new struct type with the name stats_struct_name, and an
instance of this type called g_stats_struct_name. The struct type
contains a single 32-bit member called sstat_name. In all, the above set
of macros expands to the following:
struct stats_struct_name {
struct stats_hdr s_hdr;
uint32_t sstat_name;
} g_stats_struct_name;
2. Optionally, name strings can be assigned to each statistic as follows:
STATS_NAME_START(struct_name)
STATS_NAME(struct_name, stat_name)
STATS_NAME_END(struct_name)
This expands to the following:
struct stats_name_map g_stats_map_struct_name = {
{ g_stats_struct_name.sstat_name, "stat_name" },
};
3. Finally, to increment a stat, you use either of the following two
macros:
STATS_INC(struct_name, stat_name);
STATS_INCN(struct_name, stat_name, 5);
These invocations expand to:
g_stats_struct_name.sstat_name++;
g_stats_struct_name.sstat_name += 5;
(There is more functionality in the stats module involving the shell and
newtmgr, but I don't think that is relevant here).
Here are the issues I encountered:
* The STATS_SECT_START / STATS_SECT_END macros define a new struct
and declare an instance at the same time. This is fine if the
stats section is local to a single source file, but it is
incompatible with the requirement to define the struct in a header
file so that the stats can be read and modified in several source
files.
* You can only have a single instance of each stat structure, also
due to the nature of the STATS_SECT_START and STATS_SECT_END
macros. There may be times when an application needs several
instances of a stats structure (e.g., per-connection statistics).
* Just a nit pick, but I dislike having to use macros just to access
a statistic. It would be nice if we could do away with the
STAT_INC() macros.
Finally, here is my proposal to address these issues:
1. The STATS_SECT_START and STATS_SECT_END macros just define a struct;
they don't create an instance. Generally, these macros would be used
in a header file so that source files can have access to the struct
definition.
2. The addition of a STATS_SECT_DECL macro. This macro would be used in
two places:
* In source files to instantiate a stats struct.
* In header files to expose an extern declaration of a stats
instance.
3. As a consequence of the above two points: the names of struct
instances are no longer auto-generated. The user needs to specify the
exact name. All macros which derive the instance name from the struct
name are changed: now they just accept the instance name directly.
4. Remove the "s" which prefaces the name of each stat field in a
statistics struct. By doing this, the STATS_SECT_VAR, STATS_INC, and
STATS_INCN macros can be removed.
Below is some example code showing how the above proposals would look in
practice:
// -------------
// (ble_hs_priv.h)
STATS_SECT_START(ble_hs_stats)
STATS_SECT_ENTRY(num_connects)
STATS_SECT_END
extern STATS_SECT_DECL(ble_hs_stats) ble_hs_my_stats;
// (ble_hs.c)
STATS_SECT_DECL(ble_hs_stats) ble_hs_my_stats;
STATS_NAME_START(ble_hs_stats)
STATS_NAME(ble_hs_stats, num_connects)
STATS_NAME_END(ble_hs_stats)
rc = stats_init(STATS_HDR(ble_hs_my_stats),
STATS_SIZE_INIT_PARMS(ble_hs_my_stats, STATS_SIZE_32),
STATS_NAME_INIT_PARMS(ble_hs_stats));
// (some_other_file.c)
ble_hs_my_stats.num_connects++;
// -------------
Thanks,
Chris
