Skip to content

Commit f75b1aa

Browse files
yossigoyossigo
authored
Fix memory leaks involving hdr_histogram. (#140)
Handle all allocation, deallocation and copying of hdr_histogram automatically using a safe_hdr_histogram wrapper class.
1 parent d0c48ca commit f75b1aa

File tree

4 files changed

+61
-77
lines changed

4 files changed

+61
-77
lines changed

‎run_stats.cpp

Lines changed: 5 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -117,21 +117,6 @@ run_stats::run_stats(benchmark_config *config) :
117117
{
118118
memset(&m_start_time, 0, sizeof(m_start_time));
119119
memset(&m_end_time, 0, sizeof(m_end_time));
120-
hdr_init(
121-
LATENCY_HDR_MIN_VALUE, // Minimum value
122-
LATENCY_HDR_MAX_VALUE, // Maximum value
123-
LATENCY_HDR_SIGDIGTS, // Number of significant figures
124-
&m_get_latency_histogram); // Pointer to initialise
125-
hdr_init(
126-
LATENCY_HDR_MIN_VALUE, // Minimum value
127-
LATENCY_HDR_MAX_VALUE, // Maximum value
128-
LATENCY_HDR_SIGDIGTS, // Number of significant figures
129-
&m_set_latency_histogram); // Pointer to initialise
130-
hdr_init(
131-
LATENCY_HDR_MIN_VALUE, // Minimum value
132-
LATENCY_HDR_MAX_VALUE, // Maximum value
133-
LATENCY_HDR_SIGDIGTS, // Number of significant figures
134-
&m_wait_latency_histogram); // Pointer to initialise
135120

136121
if (config->arbitrary_commands->is_defined()) {
137122
setup_arbitrary_commands(config->arbitrary_commands->size());
@@ -142,13 +127,6 @@ void run_stats::setup_arbitrary_commands(size_t n_arbitrary_commands) {
142127
m_totals.setup_arbitrary_commands(n_arbitrary_commands);
143128
m_cur_stats.setup_arbitrary_commands(n_arbitrary_commands);
144129
m_ar_commands_latency_histograms.resize(n_arbitrary_commands);
145-
for (unsigned int i=0; i<n_arbitrary_commands; i++) {
146-
hdr_init(
147-
LATENCY_HDR_MIN_VALUE, // Minimum value
148-
LATENCY_HDR_MAX_VALUE, // Maximum value
149-
LATENCY_HDR_SIGDIGTS, // Number of significant figures
150-
&m_ar_commands_latency_histograms[i]); // Pointer to initialise
151-
}
152130
}
153131

154132
void run_stats::set_start_time(struct timeval* start_time)
@@ -1023,15 +1001,10 @@ void run_stats::print_avg_latency_column(output_table &table) {
10231001
table_el el;
10241002
table_column column(15);
10251003

1026-
struct hdr_histogram* m_totals_latency_histogram;
1027-
hdr_init(
1028-
LATENCY_HDR_MIN_VALUE, // Minimum value
1029-
LATENCY_HDR_MAX_VALUE, // Maximum value
1030-
LATENCY_HDR_SIGDIGTS, // Number of significant figures
1031-
&m_totals_latency_histogram); // Pointer to initialise
1032-
hdr_add(m_totals_latency_histogram,m_set_latency_histogram);
1033-
hdr_add(m_totals_latency_histogram,m_get_latency_histogram);
1034-
hdr_add(m_totals_latency_histogram,m_wait_latency_histogram);
1004+
safe_hdr_histogram m_totals_latency_histogram;
1005+
hdr_add(m_totals_latency_histogram,m_set_latency_histogram);
1006+
hdr_add(m_totals_latency_histogram,m_get_latency_histogram);
1007+
hdr_add(m_totals_latency_histogram,m_wait_latency_histogram);
10351008

10361009
column.elements.push_back(*el.init_str("%15s ", "Avg. Latency"));
10371010
column.elements.push_back(*el.init_str("%s", "----------------"));
@@ -1070,12 +1043,7 @@ void run_stats::print_quantile_latency_column(output_table &table, double quanti
10701043
table_el el;
10711044
table_column column(15);
10721045

1073-
struct hdr_histogram* m_totals_latency_histogram;
1074-
hdr_init(
1075-
LATENCY_HDR_MIN_VALUE, // Minimum value
1076-
LATENCY_HDR_MAX_VALUE, // Maximum value
1077-
LATENCY_HDR_SIGDIGTS, // Number of significant figures
1078-
&m_totals_latency_histogram); // Pointer to initialise
1046+
safe_hdr_histogram m_totals_latency_histogram;
10791047
hdr_add(m_totals_latency_histogram,m_set_latency_histogram);
10801048
hdr_add(m_totals_latency_histogram,m_get_latency_histogram);
10811049
hdr_add(m_totals_latency_histogram,m_wait_latency_histogram);

‎run_stats.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,10 @@ class run_stats {
101101
// current second stats ( appended to m_stats and reset every second )
102102
one_second_stats m_cur_stats;
103103

104-
struct hdr_histogram* m_get_latency_histogram;
105-
struct hdr_histogram* m_set_latency_histogram;
106-
struct hdr_histogram* m_wait_latency_histogram;
107-
std::vector<struct hdr_histogram*> m_ar_commands_latency_histograms;
104+
safe_hdr_histogram m_get_latency_histogram;
105+
safe_hdr_histogram m_set_latency_histogram;
106+
safe_hdr_histogram m_wait_latency_histogram;
107+
std::vector<safe_hdr_histogram> m_ar_commands_latency_histograms;
108108

109109
void roll_cur_stats(struct timeval* ts);
110110

‎run_stats_types.cpp

Lines changed: 8 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -26,36 +26,16 @@
2626

2727

2828

29-
one_sec_cmd_stats::one_sec_cmd_stats() {
30-
m_bytes = 0;
31-
m_ops = 0;
32-
m_hits = 0;
33-
m_misses = 0;
34-
m_moved = 0;
35-
m_ask = 0;
36-
m_total_latency = 0;
37-
hdr_init(
38-
LATENCY_HDR_MIN_VALUE, // Minimum value
39-
LATENCY_HDR_MAX_VALUE, // Maximum value
40-
LATENCY_HDR_SIGDIGTS, // Number of significant figures
41-
&latency_histogram); // Pointer to initialise
29+
one_sec_cmd_stats::one_sec_cmd_stats() :
30+
m_bytes(0),
31+
m_ops(0),
32+
m_hits(0),
33+
m_misses(0),
34+
m_moved(0),
35+
m_ask(0),
36+
m_total_latency(0) {
4237
}
4338

44-
one_sec_cmd_stats::one_sec_cmd_stats(const one_sec_cmd_stats& b1){
45-
m_bytes = b1.m_bytes;
46-
m_ops = b1.m_ops;
47-
m_hits = b1.m_hits;
48-
m_misses = b1.m_misses;
49-
m_moved = b1.m_moved;
50-
m_ask = b1.m_ask;
51-
m_total_latency = b1.m_total_latency;
52-
hdr_init(
53-
LATENCY_HDR_MIN_VALUE, // Minimum value
54-
LATENCY_HDR_SEC_MAX_VALUE, // Maximum value
55-
LATENCY_HDR_SEC_SIGDIGTS, // Number of significant figures
56-
&latency_histogram); // Pointer to initialise
57-
hdr_add(latency_histogram, b1.latency_histogram);
58-
}
5939

6040
void one_sec_cmd_stats::reset() {
6141
m_bytes = 0;
@@ -269,11 +249,6 @@ totals::totals() :
269249
m_latency(0),
270250
m_bytes(0),
271251
m_ops(0) {
272-
hdr_init(
273-
LATENCY_HDR_MIN_VALUE, // Minimum value
274-
LATENCY_HDR_MAX_VALUE, // Maximum value
275-
LATENCY_HDR_SIGDIGTS, // Number of significant figures
276-
&latency_histogram); // Pointer to initialise
277252
}
278253

279254
void totals::setup_arbitrary_commands(size_t n_arbitrary_commands) {

‎run_stats_types.h

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,48 @@
3131
#include "deps/hdr_histogram/hdr_histogram.h"
3232
#include "memtier_benchmark.h"
3333

34+
// A wrapper around hdr_histogram that handles allocation, deallocation
35+
// and deep copy.
36+
//
37+
// TODO: In the future we may want to consider extending this wrapper
38+
// further to expose all hdr functions as native methods and avoid the
39+
// casting operator completely.
40+
class safe_hdr_histogram {
41+
hdr_histogram *m_hdr;
42+
public:
43+
safe_hdr_histogram() {
44+
hdr_init(
45+
LATENCY_HDR_MIN_VALUE, // Minimum value
46+
LATENCY_HDR_SEC_MAX_VALUE, // Maximum value
47+
LATENCY_HDR_SEC_SIGDIGTS, // Number of significant figures
48+
&m_hdr);
49+
}
50+
51+
virtual ~safe_hdr_histogram() {
52+
hdr_close(m_hdr);
53+
}
54+
55+
safe_hdr_histogram(const safe_hdr_histogram& other) {
56+
hdr_init(
57+
LATENCY_HDR_MIN_VALUE, // Minimum value
58+
LATENCY_HDR_SEC_MAX_VALUE, // Maximum value
59+
LATENCY_HDR_SEC_SIGDIGTS, // Number of significant figures
60+
&m_hdr);
61+
hdr_add(m_hdr, other.m_hdr);
62+
}
63+
64+
safe_hdr_histogram& operator=(const safe_hdr_histogram& other) {
65+
if (this != &other) {
66+
hdr_reset(m_hdr);
67+
hdr_add(m_hdr, other.m_hdr);
68+
}
69+
return *this;
70+
}
71+
72+
// Cast safe_hdr_historgram to hdr_histogram to make it transparent
73+
operator hdr_histogram* () const { return m_hdr; }
74+
};
75+
3476
class one_sec_cmd_stats {
3577
public:
3678
unsigned long int m_bytes;
@@ -40,9 +82,8 @@ class one_sec_cmd_stats {
4082
unsigned int m_moved;
4183
unsigned int m_ask;
4284
unsigned long long int m_total_latency;
43-
struct hdr_histogram* latency_histogram;
85+
safe_hdr_histogram latency_histogram;
4486
one_sec_cmd_stats();
45-
one_sec_cmd_stats(const one_sec_cmd_stats& b1);
4687
void reset();
4788
void merge(const one_sec_cmd_stats& other);
4889
void update_op(unsigned int bytes, unsigned int latency);
@@ -125,7 +166,7 @@ class totals {
125166
totals_cmd m_get_cmd;
126167
totals_cmd m_wait_cmd;
127168
ar_totals_cmd m_ar_commands;
128-
struct hdr_histogram* latency_histogram;
169+
safe_hdr_histogram latency_histogram;
129170
double m_ops_sec;
130171
double m_bytes_sec;
131172
double m_hits_sec;

0 commit comments

Comments
 (0)