Reputation: 115
I am trying to generate objects to add to a vector for use in populating a database, when doing so I noticed a memory leak, I am sure it's something to do with the way I am adding the objects to the Vector due to the fact if I do not add any objects to the vector the function works as expected with no leaks. Where have I gone wrong?
I have tried moving the unique_ptr, same result.
I have found with this code, not all memory will be de-allocated each run of database::populate, after many executions of the populate function almost 400 mb of memory will not be de-allocated, why?
main.cpp
#include <iostream>
#include "database.h"
int main()
{
std::vector<string> cols{"name", "score" };
while (true) {
std::getchar();
database::populate("test", cols, 1000000);
}
}
database.h
#include <string>
#include <vector>
using std::string;
class database {
public:
static void populate(const string table, std::vector<string> cols, int limit);
};
database.cpp
#include "database.h"
#include "model.h"
#include <memory>
#include <random>
#include <iostream>
typedef std::vector<std::shared_ptr<Model>> modelContainer;
static const char alphanum[] =
"0123456789"
"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
"abcdefghijklmnopqrstuvwxyz";
//temporary
int alphaLen = std::size(alphanum) - 1;
std::random_device rd;
std::mt19937 gen(rd());
int random(int min, int max) {
std::uniform_int_distribution<>rng_dice(min, max); return rng_dice(gen);
}
string randstring(int len) {
string str = "";
while (len--) {
int index = random(0, alphaLen);
str += alphanum[index];
}
return str;
}
void database::populate(const string table, std::vector<string> cols, int limit) {
modelContainer models;
std::shared_ptr<Model> e;
for (int i = 0; i < limit; i++) {
e = std::make_shared<Model>(table);
for (int j = 0; j < cols.size(); j++) {
e->addVal(cols[j], randstring(5));
}
models.push_back(std::move(e)); //Leak??
}
std::cout << "done!" << std::endl;
// add to database
}
model.h
#include <map>
#include<string>
#include <vector>
using std::string;
class Model {
private:
std::map<string, string> data;
string table;
public:
Model(const string table);
void addVal(const string& key, const string& val);
};
model.cpp
#include "model.h"
#include <string>
Model::Model(const string table) {
this->table = table;
}
void Model::addVal(const string& key, const string& val) {
(data)[key] = val;
}
Upvotes: 0
Views: 470
Reputation: 117433
Your OS won't report exactly what you have allocated/freed when looking in the TaskManager
or top
, so I wouldn't start worrying about memory consumption in that program unless it increases over time. GNU has an extension in the form of function called malloc_stats()
that you can call to report malloc/free statistics from within a program.
I scanned your program (using scan-build, address sanitizer, ubsan, ...) but couldn't find any problems and the memory consumption is holding at the same level when running in an endless loop. I used malloc_stats
for checking the latter.
Some notes on the code since I read through it anyway:
#ifndef HEADER_H
... or #pragma once
using ...
into the global namespace in header files. A user of such a header file could get surprises.const&
instead of by value. There's currently some unnecessary string
and vector
copying.Model::Model(const string& Table) : data{}, table(Table) { /* now empty */ }
int
), which triggers warnings, where an unsigned integer (size_t
) would have been more appropriate.reserve()
on containers supporting it when you know how many entries you'll add.unique_ptr
over shared_ptr
if you don't need sharing.unique_ptr
s to the objects.uniform_int_distribution
is very lightweight, but if you only need the same distribution all the time, you might as well reuse it.std::mt19937
isn't thread-safe so if you want to use it from more than one thread, make it thread_local
. This isn't a problem in your current code, but could be good to know.database.cpp example
#include "database.h"
#include "model.h"
#include <iostream>
#include <memory>
#include <random>
#include <string_view>
using namespace std::literals::string_view_literals;
using std::string;
using modelContainer = std::vector<Model>; // no smart pointer needed
// A function to return a thread_local generator that is initialized once.
inline std::mt19937& generator() {
// The std::random_device is created, called and disposed of since
// we only need it to seed the std::mt19937 PRNG.
static thread_local std::mt19937 gen(std::random_device{}());
return gen;
}
// Made random into a function template to make it able to return the
// correct type and taking [min, max] as template parameters to only
// instantiate the distribution once per IntType and range used.
template<typename IntType, IntType min, IntType max>
inline IntType random() {
static std::uniform_int_distribution<IntType> rng_dice(min, max);
return rng_dice(generator());
}
string randstring(size_t len) {
static constexpr std::string_view alphanum =
"0123456789"
"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
"abcdefghijklmnopqrstuvwxyz"sv; // sv = string_view literal
string str;
str.reserve(len);
while(len--) {
auto index = random<size_t, 0, alphanum.size() - 1>();
str += alphanum[index];
}
return str;
}
void database::populate(const string& table, const std::vector<string>& cols,
size_t limit) {
modelContainer models;
models.reserve(limit);
for(size_t i = 0; i < limit; ++i) {
Model e(table);
for(size_t j = 0; j < cols.size(); ++j) {
e.addVal(cols[j], randstring(5));
}
models.emplace_back(std::move(e));
}
std::cout << "done!" << std::endl;
}
Upvotes: 1