Map et fuite mémoire [Résolu]

[transgohan]

Bonjour à toutes et à tous,

reprenant un code j'essaie d'en supprimer les fuites mémoires. Cependant je suis actuellement sur une partie dont je n'arrive pas à me dépatouiller.

Une Map est utilisée pour stocker des pointeurs de n'importe quoi (on y stocke aussi bien des pointeurs vers des entiers que des pointeurs vers des Vectors, ect).

Code c++ :

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
#include <iostream>
#include <map>
 
using namespace std;
 
class BenchEnvironment {
private:
  static BenchEnvironment* pInstance;
 
 public:
  virtual ~BenchEnvironment();
 
  static BenchEnvironment* getInstance();
 
  void addData(std::string key, void* p_dataToAdd);
  void* getData(std::string key);
 
 private:  
  BenchEnvironment();
 
 private:
  std::map<std::string, void*> datas;   
};

Code c++ :

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
#include "BenchEnvironment.h"
 
BenchEnvironment *BenchEnvironment::pInstance = NULL;
 
BenchEnvironment::BenchEnvironment()
{
}
 
BenchEnvironment::~BenchEnvironment()
{
	pInstance = NULL;
}
 
BenchEnvironment *BenchEnvironment::getInstance()
{
	if (pInstance == NULL) {
		pInstance = new BenchEnvironment();
	}
	return pInstance;
}
 
void BenchEnvironment::addData(string key, void *p_dataToAdd)
{
	datas[key] = p_dataToAdd;
}
 
void *BenchEnvironment::getData(std::string key)
{
	return datas.find(key)->second;
}

Utilisant la class de cette manière :

Code c++ :

1
2
int test = 1;
BenchEnvironment::getInstance()->addData("test", &test);

Cela m'ajoute 3 allocations mais aucune désallocation selon Valgrind.
Est-ce parce que c'est du void* qu'il ne sait pas comment aller libérer la mémoire des pointeurs ?

[transgohan]

Avec un delete c'est toujours mieux en fait. T_T
Perdu dans la taille du code je n'avais pas fait attention à cela.
Il y a des jours où un café me ferrai pas de mal malgré le fait que je le digère pas...

[gbdivers]

Oui, mais ...

Un delete sur void* compile ???

Envoyé par N3376 5.3.5.3

if the static type of the object to be deleted is different from its dynamic type, the static type shall be a base class of the dynamic type of the object to be deleted and the static type shall have a virtual destructor or the behavior is undefined.
Note 78 : This implies that an object cannot be deleted using a pointer of type void* because void is not an object type.

Il faut donc (sauf erreur de ma part) faire un cast vers le type, soit avoir une hiérarchie d'objet avec destructeur virtuel

Code :

1
2
3
4
5
6
7
8
9
10
11
12
class A { virtual ~A() {} };
class B : public A {};
 
void delete(void* p) {
   if (p.type() == "int") {
       int* pInt = static_cast<int*>(p)
       delete pInt;
   } else if (p.type() == "A" || p.type() == "") {
       A* pA = static_cast<A*>(p)
       delete pA;
   }
}

Plus généralement, utiliser un void* est très très très très moche
Au pire, utilise boost.any ou boost.variant, mais vérifie quand même si tu n'as pas un problème de conception

Et utiliser les pointeurs intelligents permet d'éviter d'oublier les delete (et est plus exception safe). Par contre, pas sur que ça passe sur un void*

[transgohan]

boost n'est pas une bibliothèque disponible de base.
De plus mes contraintes techniques m'empêcheront de toute manière de l'utiliser. Je joue la taille de mon application à l'octet près en raison d'une cible embarquée très très peu véloce et qui contient peu de mémoire.

Sinon cela ne lui pose aucun problème au compilateur (testé avec GCC et DIAB).
J'effectue actuellement le delete sur l'objet BenchEnvironment (qui est la seule allocation dynamique).
Mes variables sont statiques et donc la mémoire est libérée sans souci.

Sinon je reste dibutatif.
J'ai travaillé hier avec les static_cast pour autre chose et j'ai pu observer qu'il effectuait une copie lors de l'utilisation de static_cast.
Donc du coup il fait une copie castée, et delete le void *, puis delete le int *. Donc on revient à une surcouche pour faire la même chose puisqu'on a toujours le delete du void *.

A moins que ce sur quoi je travaillais soit un cas spécial :
char *pChar = static_cast< char * >(maString.c_str());

[gbdivers]

Pour le cast, oui, tu créés forcement une copie. Quand tu écris

Code :

1
2
float f = 12.34;
int i = static_cast<int>(f);

Tu crées une nouvelle variable (i) et il y a bien une copie du temporaire retourné par le cast.
Pour les pointeurs, idem, tu le copies, mais pas l'objet pointé. Il ne devrait pas y avoir d'appelle à delete sur le void* (implicite) puis sur le int* (explicite)

Et pour moi, un delete sur void* devrait échouer, le compilateur ne sait pas le type pointé et devrait donc ne pas savoir la taille de l'objet en mémoire à supprimer

Pour

Code :

char *pChar = static_cast< char * >(maString.c_str());

c'est quoi cette idée de faire un cast de const char* vers char* pour faire un delete ? (si j'ai bien compris ce que tu fais)
c_str() créé a priori une copie de ta chaîne en ajoutant un \0 puis tu fais un delete dessus, c'est quoi l'intérêt.
Et string est déjà une mise en oeuvre du RAII, pas besoin de se préoccuper de supprimer les données

Bref, j'ai un peu de mal à comprendre...

[transgohan]

Pas d’inquiétudes la variable char* est belle est bien utilisée avant le delete !

Le programme utilise des string, jusque là pas de souci.
Mais dans une message queue on fait passer que du char* donc vient de là ce cast.

Et string est déjà une mise en oeuvre du RAII, pas besoin de se préoccuper de supprimer les données

On est tout à fait d'accord là dessus, seule une string * mérite ce traitement.

[gbdivers]

Hum... jamais vu de delete sur const char* retourné par c_str(). A vérifier sur ce coup, pas sur que l'on a besoin (dont ça serait pas une copie du contenu de la string comme j'ai dit)
Dans http://www.cplusplus.com/reference/string/string/c_str/ le pointeur retourné par c_str() n'est pas gardé donc pas de delete dessus

Par la même occasion, cette indique la bonne méthode pour convertir le const char* en char* : il faut copier la chaîne avec strcpy

Bref, tu fais des trucs bizarres et tu obtiens des résultats bizarres...

[transgohan]

Envoyé par gbdivers

Hum... jamais vu de delete sur const char* retourné par c_str(). A vérifier sur ce coup, pas sur que l'on a besoin (dont ça serait pas une copie du contenu de la string comme j'ai dit)

valgrind devrait me crier dessus si ce delete était inutile. Ce serait comme en faire deux. Mais je ne vois pas pourquoi tu me dis qu'il ne faut pas de delete (char * au fait et non const char *)... Il y a bien allocation dynamique, par contre il serait à confirmer si ce n'est pas un pointeur intelligent qui est utilisé. Je testerai demain d'enlever le delete pour voir.

Envoyé par gbdivers

Bref, tu fais des trucs bizarres et tu obtiens des résultats bizarres...

Quel résultat bizarre ?
Je transmet correctement la chaîne via ma message queue, je n'ai pas de fuite mémoire, pas de warning, pas d'erreur.

[transgohan]

Ne pas mettre de delete indique bien une fuite mémoire.
J'avais un équilibre de new / delete indiqué par Valgrind, si j'enlève mon delete du char * j'ai un delete de moins.

[gbdivers]

équilibre de new / delete ? Quand fais tu un new ?

Tu fais des trucs bizarre parce que tu cast un const char* en char* au lieu de faire une copie. Tu fais des trucs bizarre parce que tu fais un delete sur un const char* retourné par c_str. Tu fais des trucs bizarre parce que tu fais un delete sur un void* (je laisse de côté pour l'instant l'histoire de l'utilisation de void* ou d'un singleton)

Tu as des résultats bizarre par ce que tu n'as pas d'erreurs de compilation en faisant un delete sur void* ou sur le retour de c_str
Chez moi, j'ai des erreurs de compilation (ubuntu gcc) :

Code :

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
#include <iostream>
#include <string>
using namespace std;
 
int main(int, char**)
{
    string s = "une chaine de test";
    const char* cStr = s.c_str();
    char* ncStr = static_cast<char*>(cStr);
    delete ncStr;
 
    int* p = new int(123);
    void* pVoid = static_cast<void*>(p);
    delete pVoid;
 
    return 0;
}

J'ai des erreurs de compilation sur le premier cast et sur le second delete :

/home/guillaume/Bureau/test/main.cpp:9: erreur : invalid static_cast from type 'const char*' to type 'char*'
/home/guillaume/Bureau/test/main.cpp:14: erreur : deleting 'void*' is undefined [-Werror]

En fait, le second est un warning promu en erreur avec -Werror. Mais cela produira quand même une fuite mémoire du fait que delete ne sait pas quelle mémoire libérer

Ou alors j'ai pas compris ce que tu fais...

[transgohan]

Je ne fais pas de delete de const char *...
J'avais remis une couche dans mon avant dernier message mais visiblement c'était pas encore suffisant.

Tu fais des trucs bizarre parce que tu cast un const char* en char* au lieu de faire une copie.

Pourrais-tu m'expliquer l'utilité et les cas d'utilisation de static_const s'il te plait ? Je veux bien admettre faire des erreurs, mais j'aimerai savoir pourquoi.

Tu fais des trucs bizarre parce que tu fais un delete sur un void*

Pas de delete de ma part sur un void *, comme précédemment dit ce n'est pas une instanciation dynamique. On est donc dans ce cas là :

Code C :

1
2
3
int a = 1;
std::map<std::string, void*> datas;
datas["a"] = (void *)(&a);

Donc aucun delete. La mémoire ce n'est pas moi qui la gère. (et aucune fuite mémoire)

Le souci provenait du singleton qui lui n'était pas libéré et donc il ne me libérait pas la map et les pointeurs stockés.

Ou alors j'ai pas compris ce que tu fais...

On est au moins d'accord sur ce point là. J'avais posté le code tel quel et tu as imaginé du code autour en ne comprenant pas mes phrases d'explications.

erreur : invalid static_cast from type 'const char*' to type 'char*'

Avec quel flag compiles-tu pour obtenir cela ? Je compile toujours en -Wall mais visiblement cela ne rentre pas dedans.

Edit : je viens d'effectuer le changement tel que décrit dans la doc du c_str() pour passer de const char* à char*.
Cependant même si c'est plus propre je ne pense pas que je pourrai le valider dans le cadre de mon utilisation. Cela rajoute une allocation dynamique, et c'est une chose qu'on tente par tout les moyens d'éviter. Mon test ne portait que sur l'ajout de 10 octets mais on pourrait dans le cadre de nos application avoir bien plus et donc avoir un comportement incertain. Le processus étant critique il faut qu'on puisse affirmer avec certitudes qu'on ne consomme pas plus que X distribués de façon prédictif en gros. Avec de l'allocation dynamique c'est totalement impossible.

[koala01]

Salut,

Moi, ce qui me chagrine beaucoup, c'est que personne n'a encore posé le vrai problème dans cette discussion:

Vouloir gérer une map dont les valeurs peuvent être tout et n'importe quoi me semble réellement bancal, conceptuellement parlant, surtout si cela se fait avec un void *

Au mieux, cela relève "simplement" d'un manque d'abstraction, au pire, cela relève d'une responsabilité clairement mal définie

Dans le premier cas, il serait bon de passer par une structure comme "Variant" (celle de boost, idéalement, mais cela pourrait être une hiérarchie de classes non template "classique" )pour apporter le polymorphisme nécessaire à cette gestion et, dans le deuxième cas, il y aurait sans doute beaucoup d'intérêt à déléguer la gestion des différents types dans autant de map que nécessaires.

Je ne vais pas me lancer maintenant dans le roman qui serait nécessaire pour expliquer les deux possibilités, mais je reste à disposition pour en dire plus sur chacune d'elle en cas de besoin

[transgohan]

Comme précédemment dit boost n'est pas une solution envisageable dans mon projet.

Sinon il m'est aussi impossible de refaire une série de class pour refaire la joyeuse boost::variant en raison de mes contraintes mémoires. Je cherche à gagner de précieux octets, pas à en rajouter.

Donc je comprends votre point de vue du "c'est moche", mais cela n'a surement pas été choisi ainsi par mes prédécesseurs en raison de leur méconnaissance. Je ne peux me permettre d'instancier une map par type sous peine de faire exploser la mémoire de ma cible embarquée.

[koala01]

Heuu... En toute honnêteté, combien de types différents places tu dans ta map

La taille renvoyée par sizeof(std::map<std::string, quoi que ce soit>) est de... 48 bytes sur mon système 64 bits!!!

Il est vrai que de rajouter X*48 bytes pour maintenir autant de types différents, + le fait de maintenir le type en question en mémoire, ca va faire exploser ta mémoire

[koala01]

Je me rends compte que, même si j'ai raison dans le fond, mon sarcasme ne fait pas avancer le problème, je vais donc être un peu plus didactique dans ma manière d'expliquer les choses

Que se passe-t-il, selon toi, lorsque tu manipule une std::map<std::string, void*> et que tu y mets du "grand tout et n'importe quoi"

Tout est stocké dans une std::pair<std::string, void *>...
Cout :

• 16 bytes (la taille des type std::string et void*), auquel il faut rajouter:
• l'espace mémoire suffisant pour représenter la chaine entière
• l'espace mémoire suffisant pour représenter ce que tu as (honteusement!!) casté en void *

Cela induit un problème majeur : tu ne sais jamais quel est le type réel de ta valeur, à moins qu'il ne soit écrit dans ta chaine de caractères, d'une manière ou d'une autre.

S'il est effectivement introduit dans la chaine de caractères, tu te trouves, en plus, confronté à un autre problème: pour pouvoir récupérer le type d'origine, tu es obligé de comparer (tout ou partie de) ta chaine et recaster le void * dans le type réellement représenté.

La comparaison d'une (sous) chaines de caractères est ce qui prend le plus de temps, simplement parce qu'elle se base essentiellement sur la comparaison de tous les caractères deux à deux!!!

Si tu décides de déléguer un tout petit peu en créant, mettons, 50 maps différentes, correspondant aux différents types que tu utilises (à titre d'infos : il existe 13 types primitifs, si tu rajoutes std::string et un vecteur pour chaque type primitif + un pour std::string, tu arrives à ... 28 map ), tu vas, effectivement, rajouter 50 * 48 bytes ( == sizeof(std::map ) ) = 2400 bytes (enfin, en 64 bits ) mais...

Pour chaque élément, tu gagnera au moins 8 bytes, vu que tu pourras le stocker par valeur et non plus passer par un pointeur pour y accéder (sauf pour les objets polymorphes).

Si tu as ne serait-ce que 2400 / 8 = 300 éléments qui ne doivent plus être représentés par des pionteurs, tu gagnes en utilisation mémoire

En plus, cela te permet de gagner en performances, car, si tu sais d'avance à quel type tu as affaire, tu peux t'éviter une série de tests sur tes chaines de caractères afin de récupérer le "type adéquat" !!!

Moralité, tu as tout à y gagner, même si l'on peut envisager le fait qu'il serait peut etre intéressant de garder la liste de toutes les chaines de caractères "quelque part"

[transgohan]

Envoyé par koala01

Heuu... En toute honnêteté, combien de types différents places tu dans ta map

La taille renvoyée par sizeof(std::map<std::string, quoi que ce soit>) est de... 48 bytes sur mon système 64 bits!!!

Il est vrai que de rajouter X*48 bytes pour maintenir autant de types différents, + le fait de maintenir le type en question en mémoire, ca va faire exploser ta mémoire

Ma mémoire libre se compte en kilo-octets.
Comme je l'ai dit je ne développe pas pour des PCs, ni pour des cartes embarquées comprenant des Gio de RAM.

Cependant je vais m'intéresser à ta solution, même si je ne peux l'appliquer (cela restera à définir) je suis curieux de nature.

2400 bytes à vide, je me trompe pas sur ce point ? Donc si j'insère un pointeur d'entier et que le reste est vide j'aurai 2404 bytes occupés en mémoire ?

Aussi j'aimerai comprendre le fait de faire des tests sur le type avant de le récupérer. Actuellement on est rigoureux sur les index, donc on ne fait jamais de test avant de faire le cast pour la récupération. Donc le seul test effectué se fait lorsqu'on demande l'index X de la map. Ai-je mal compris tes explications ?

Pour chaque élément, tu gagnera au moins 8 bytes, vu que tu pourras le stocker par valeur et non plus passer par un pointeur pour y accéder (sauf pour les objets polymorphes).

Je n'ai absolument pas compris cette partie je crois.

Code c++ :

1
2
map<string, int> datas;
datas["nbLoop"] = 5;

Est-ce cela ? Et donc travailler par la suite avec datas["nbLoop"] au lieu de :

Code c++ :

1
2
3
map<string, int *> datas;
int nbLoop = 5;
datas["nbLoop"] = &nbLoop;

On gagne dans le premier cas 4 octets grâce à l'élimination du pointeur.

Merci de t'intéresser autant à mes interrogations.

[koala01]

Envoyé par transgohan

2400 bytes à vide, je me trompe pas sur ce point ?

non, j'ai dit : si tu crées 50 maps différentes pour pouvoir contenir 50 types différents (un par map), le cout en mémoire sera de 2400 bytes à vide, vu que cela fait 50 (le nombre de map) *48 bytes (la taille d'une map)...

Donc si j'insère un pointeur d'entier et que le reste est vide j'aurai 2404 bytes occupés en mémoire ?

Non, tu auras plus :

1. 8 bytes pour la std::string
2. le nombre de bytes nécessaire à la représentation interne de la chaine de caractères
3. 4 ou 8 bytes pour la représentation du pointeur (selon l'architecture sur laquelle tu travailles)
4. 4 bytes pour la représentation de l'entier

Si l'on se base sur des chaines de caractères de 10 caractères utiles (car il y a un '\0' qu'il faut compter avec) ("ABCDEFGHIJ"), chaque fois que tu inséreras un élément dans ta map, ton utilisation de la mémoire sera de
11 bytes pour la représentation interne de la chaine de caractères + 8 bytes pour la classe string + 4 (ou 8) bytes pour le pointeur + 4 bytes pour ton int = 27 à 31 bytes (en fonction de l'architecture)

Si, au lieu d'utiliser un pointeur sur ton entier dans ta map, tu introduit directement l'entier, tu économise les 4 (ou 8) bytes de ton pointeur

Ce qu'il faut comprendre, c'est qu'un pointeur (quel que soit le type pointé) n'est jamais qu'une variable numérique non signée particulière dans le sens où elle contient "l'adresse mémoire à laquelle se trouve un objet du type indiqué"

En tant que variable de type défini (c'est une valeur numérique non signée), elle utilise un espace mémoire de taille clairement fixé et qui correspond à "la taille suffisante pour représenter toutes les adresses mémoires accessibles sur le système".

En prenant quelques libertés, on peut estimer sur pc que la taille d'un pointeur est de 4bytes (32 bits) sur les architectures 32bits et de 8 bytes (64 bits) sur les architectures 64 bits.

Mais comme l'utilisation d'un pointeur revient, en définitive, à utiliser une variable supplémentaire, si tu évites d'utiliser cette variable supplémentaire, tu évite le cout en utilisation de la mémoire qu'elle implique, CQFD )

Aussi j'aimerai comprendre le fait de faire des tests sur le type avant de le récupérer. Actuellement on est rigoureux sur les index, donc on ne fait jamais de test avant de faire le cast pour la récupération. Donc le seul test effectué se fait lorsqu'on demande l'index X de la map. Ai-je mal compris tes explications ?

Bon, je vais reprendre mon raisonnement depuis de début, cela te permettra de mieux comprendre

Tu manipules, pour l'instant, une map<string, void *>.

Cela implique que, au moment d'insérer un (pointeur sur un) objet dans cette map, tu le caste d'un pointeur sur le type réel vers un un pointeur sur void.

D'un autre coté, la chaine de caractères qui sert de clé est spécialement conçue pour te donner une indication sur le type réel d'origine (pour te permettre de caster ton void * en un pointeur sur le type "qui va bien"). On est d'accord là dessus

Si tu places quelque chose dans cette map, c'est, j'ose l'espérer, pour pouvoir récupérer ce "quelque chose" plus tard, non

Pour "récupérer ce quelque chose plus tard", tu as deux solutions :

• Soit tu sais ce que tu veux récupérer, et tu crées ta chaine de caractères en conséquence, avant d'essayer de trouver l'élément qui correspond dans ta map, (en devant le caster vers le type d'origine une fois trouvé)
• Soit tu parcoures l'intégralité de ta map et tu te bases sur la chaine de caractères pour déterminer dans quel type le transtyper afin de l'utiliser "comme il le faut".

D'après ce que tu expliques, tu te trouves de toutes évidences dans la première situation qui est sans doute "moins mauvaise" que la deuxième

Mais je parle de "moins mauvaise solution" pour une raison finalement toute simple: en introduisant une information permettant de déterminer le type dans la chaine de caractères, tu augmentes, plus ou moins significativement, la taille de cette chaine de caractères par rapport à celle qui aurait pu identifier un élément de manière unique si tu n'avais pas introduit cette notion de type.

Or, il faut savoir que la comparaison de chaines se fait caractères par caractères.

En gros, c'est une grosse boucle qui passe chaque caractère en revue deux à deux (le premier issu de la chaine que l'on teste, le deuxième issu de la chaine de référence, ou l'inverse )

C'est donc une comparaison qui prend énormément de temps, et c'est d'autant plus vrai lorsqu'une chaine de caractères est utilisée comme clé dans une map que la clé est testée par équivalence, à savoir sous une forme proche de

Code :

1
2
3
4
5
6
7
8
9
10
11
12
if (key < tofind)
{
   // on peut faire quelque chose ici
}
else if (tofind < key )
{
   // on peut faire autre chose ici
}
else
{
    // si ce n'est ni plus grand, ni plus petit, c'est que c'est égal
}

Et donc, chaque caractère que tu rajoute dans ta chaine risque d'occasionner une itération supplémentaire dans la boucle de comparaison.

Si tu peux virer les XXX lettres qui fournissent la notion de type de ta chaine de caractères, tu économises autant d'itérations pour la comparaison ==> tu gagnes en performances. CQFD

On est encore d'accord là dessus

Je n'ai absolument pas compris cette partie je crois.

Code c++ :

Est-ce cela ? Et donc travailler par la suite avec datas["nbLoop"] au lieu de :

Code c++ :

On gagne dans le premier cas 4 octets grâce à l'élimination du pointeur.

... Et parfois plus...

En fait, on gagne la taille d'un pointeur (qui peut aller, comme je l'ai indiqué plus haut, jusqu'à 8bytes sur les architectures 64 bits )

Si tu joues "à la recherche du byte perdu", tu risques d'en gagner pas mal en peu de temps

[EDIT] Par comparaison, les 48 bytes (sur une architecture 64 bits, car je ne serais pas étonné outre mesure que ce soit moins sur une architecture 32 bits ) nécessaires pour rajouter une map pour un type donné ne représentent plus grand chose : il suffit d'insérer 6 éléments dans ta map (par map que tu auras créée ) pour que tu commences à gagner en termes d'occupation de la mémoire

[transgohan]

Merci pour ce long message, cela me conforte dans l'idée que nous sommes sur la même longueur d'onde.

Je vais laisser de côté pour le moment cette histoire de hash map, modifier son utilisation va opérer pas mal de changements dans le programme.
Or il y a des optimisations mémoire à faire ailleurs, donc avec un peu de chance je vais pouvoir en gagner ailleurs et ne pas avoir à revenir là dessus. (surtout que mon chef est pas trop chaud pour une map par type)