Discussion :

[Frifron]

Bonjour, j'aimerai optimiser le swap d'objets de classe contenant un tableau dynamique.

Comme la taille du tableau n'est connu qu'a la création de l'objet et que le tableau n'appartient qu'a l'objet et ne peux pas être partagé, j'ai redéfini le contructeur par copie et l'opérateur d'assignation. Le destructeur libère le tableau.

swapper de tels objets avec un swap classique n'est pas du tout optimisé :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
 
swap(A& a,A& b)
{
    A tmp(a);
    a = b;
    b = tmp;
}

On va avoir 3 créations de tableaux et 3 destructions, plus 3 copies des éléments du tableau. J'ai fait une petite opti pour l'assignation et la copie qui ne realloue pas le tableau si le nouveau tableau est de même taille que l'ancien, cela se contente de copier les éléments un a un. Donc pour un swap entre 2 objets contenant un tableau de même taille, on va avoir quand même 1 création, 1 destruction et 3 copies des éléments.

Or toutes ces opérations ne sont pas necessaires dans le cas du swap, car il suffit de permutter les 2 pointeurs vers le tableau.

J'ai donc testé et benché 3 méthodes qui ne me plaisent en fait que moyennement :

Méthode 1 : A la barbare on swap les blocs mémoire des objets

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
 
swap(A& a,A& b)
{
    char* tmp[sizeof(A)];
    memcpy(tmp,&a,sizeof(A));
    memmove(&a,&b,sizeof(A));
    memcpy(&b,tmp,sizeof(A));
}

le memmove est la pour gérer le swap d'un objet sur lui même.
Je ne pense pas que les memcpy soient particulièrement adapté a des petits blocs de mémoire. Je ne sais pas pourquoi mais j'aime pas trop cette méthode

Méthode 2 : On swap 1 à 1 chacun des paramètres de la classe

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
 
swap(A& a,A& b)
{
    type1 TMPparam1 = a.param1;
    type2 TMPparam2 = a.param2;
    ...
 
    type1 a.param1 = b.param1;
    type2 a.param2 = b.param2;
    ...
 
    type1 b.param1 = TMPparam1;
    type2 b.param2 = TMPparam2;
    ...
}

La ca me plait pas parceque c'est très lourd a écrire et pas très élégant(surtout s'il y a beacoup de paramètre) et puis si on en rajoute, il faut modifier la fonction.

Méthode 3 : On créé des méthodes privées qui nous permettent de swapper 2 objets

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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
 
class A
{
private :
 
    A(bool nothing);
    void copy(const A& a);
}
 
A::A(bool nothing){}
 
void A::copy(const A& a);
{
    param1 = a.param1;
    param2 = a.param2;
    ...
}
 
swap(A& a,A& b)
{
    A tmp(false);
    tmp.copy(b);
    a.copy(b);
    b.copy(tmp);
    tmp.t = NULL;
}

La dans l'idée ca me plait assez mais en fait je suis obligé de faire du bricolage pour y parvenir :
Créer un constructeur privée qui fait rien avec un paramètre bidon qui sert a rien pour le différencier des autres. (dans le but d'éviter la création originelle du tableau)
Mettre le tableau de tmp a NULL a la fin pour éviter qu'il soit détruit (car il pointe sur le tableau qui est maintenant celui de b) par le destructeur de tmp lorsque l'on sort du swap.

Bref je n'arrive pas a trouver une méthode élégante pour faire ce que je veux faire. Je précise que cette classe n'est pas destiner a être dérivée.

J'ai fait quelques tests de performances sur un très grand nombre de swap entre 2 objets. En gros ca donne ca (j'ai plus les chiffre sous les yeux) :

classique : 5000 ms
methode 1 : 170 ms
méthode 2 : 80 ms
méthode 3 : 130 ms

Par contre parfois en fonction des générations (il faut regénéraer le projet pour que ca arrive), j'avais ce genre de résultats :

classique : 5000 ms
methode 1 : 170 ms
méthode 2 : 1300 ms
méthode 3 : 1500 ms

Je ne sais pas du tout a quoi c'est du, quelqu un a une idée ?
Pour l'instant j'ai choisi la méthode 1 (swap de blocs mémoire) parce c'est la moins "bricolage" et qu'il n'y a pas le problème avec les résultats variables que je ne comprend pas du tout même si ce n'est pas la plus rapide. Mais je pense qu'il y a mieux a faire. Vous auriez des conseils / idées a ce propos. Merci

[Luc Hermitte]

L'opérateur d'affectation devrait être défini à l'aide du swap, et non le contraire.
Quand à std::swap, définis-le à partir d'une fonction membre de swap qui sait très précisément déplacer ce qu'il faut. (C'est l'approche retenue dans la SL)

[Frifron]

L'opérateur d'affectation devrait être défini à l'aide du swap, et non le contraire.)

Je ne comprend pas exactement

Quand à std::swap, définis-le à partir d'une fonction membre de swap qui sait très précisément déplacer ce qu'il faut. (C'est l'approche retenue dans la SL)

C'est ce que j'ai fait en fait, la spécialisation de std::swap appelle une méthode public de A qui fait ce qui faut faire. J'ai simplifié dans le message parceque ce n'est pas utile a mon problème en soi.

[Médinoc]

Normalement, la spécialisation de swap<> sert à l'idiome Non-throwing Swap : Les objets eux-mêmes ne contenant que des pointeurs vers les données, on swappe simplement les pointeurs.

[Luc Hermitte]

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
struct T {
    T(T const&);
    ~T() ;
 
    T& operator=(T const& rhs) {
        T tmp(rhs);
        tmp.swap(*this);
        return *this;
    }
 
    void swap(T & other) {
        std::swap(other.m_ptr, this->m_ptr);
    }
};

Pour faire plus efficace, il faut limiter le nombre d'éléments à échanger lors du swap (avec un pimpl p.ex.), prendre rhs par copie dans l'opérateur d'affectation, ou attendre le C++0x qui disposera de std::move et autres facilités (rvalue references) améliorant les perfs quand on n'a pas besoin d'une copie complète.

[Jean-Marc.Bourguet]

Je ne comprend pas exactement

Tu definis ton swap et apres tu utilises

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
C& C::operator=(C other)
{
   this->swap(other);
   return *this;
}

ou

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
C& C::operator=(C const& other)
{
   C(other).swap(*this);
   return *this;
}

comme operateur d'assignation.

Questions subsidiaires pour ceux qui suivent:
- quelle est la difference entre les deux formes ?
- pourquoi pas this->swap(C(other)) ?

[loufoque]

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
    T& operator=(T rhs) {
        rhs.swap(*this);
        return *this;
    }

est mieux que

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
    T& operator=(T const& rhs) {
        T tmp(rhs);
        tmp.swap(*this);
        return *this;
    }

[Médinoc]

Enfin de toute façon, si on définit l'opérateur en inline, il y a des chances que les deux donnent le même code assembleur...

Edit: Ah non, pas forcément pour un objet temporaire:

Code C++ :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
MyClass a;
a = MyClass(param1, param2);

Là, il peut y avoir une copie de plus pour le passage par référence...

[Frifron]

En fait le truc c'est que je souhaite que l'assignation créé une copie de mon tableau dynamique. Je ne veux pas que l'utilisateur puisse avoir a un moment donné 2 objets de la classe A avec leur pointeur pointant sur le même tableau. C'est pour cela que j'ai redéfinie la copie et l'assigation dans la classe.

Par contre pour le swap, je souhaiterai me passer de cette allocation dynamique inutile. Un objet interne a la fonction pourra avoir exeptionnellement un pointeur pointant sur le même tableau dynamique qu'un autre objet parcequ'il sera détruit en sortie de fonction. En gros pour le swap j'aimerai utiliser quelquechose d'équivalent a l'assignation par défaut et la construction par copie par défaut sauf que je les ai redéfini.

Je ne peux donc pas utiliser ma méthode swap dans la redéfinition de l'opérateur =.

[Jean-Marc.Bourguet]

Par contre pour le swap, je souhaiterai me passer de cette allocation dynamique inutile. Un objet interne a la fonction pourra avoir exeptionnellement un pointeur pointant sur le même tableau dynamique qu'un autre objet parcequ'il sera détruit en sortie de fonction. En gros pour le swap j'aimerai utiliser quelquechose d'équivalent a l'assignation par défaut et la construction par copie par défaut sauf que je les ai redéfini.

C'est bien ce que nous avons compris.

Normalement swap est une fonction membre, definie en utilisant swap sur chacun des membres et n'echoue jamais, meme pas avec une exception. On defini aussi classiquement une fonction libre swap appelant le membre dans le namespace de la classe.

Je ne peux donc pas utiliser ma méthode swap dans la redéfinition de l'opérateur =.

Je n'arrive pas a suivre ton raisonnement. L'avantage de l'assignation telle que definie ci-dessus est que si elle echoue avec une exception, il n'y a rien de modifie dans la classe. On peut parfois etre amene a faire autrement -- pour des raisons de performances ou d'occupation en memoire -- mais atteindre alors cet objectif est plus complique si la classe n'est pas simplissime.

[loufoque]

Il y a aussi la solution de mettre l'objet dans un état "vide" en cas d'échec, ce qui s'applique de manière efficace dans tous les cas.
L'affectation n'a plus de sémantique rollback, mais ça reste de l'exception-safety basique.

[stillman]

Dans les deux versions de l'opérateur d'affectation proposées par Jean-Marc, tu peux remarquer qu'une copie du paramètre est effectuée :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
C& C::operator=(C other) // Passage par valeur --> construction d'une copie lors de l'appel
{
   this->swap(other);
   return *this;
}

et

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
T& operator=(T const& rhs) {
        T tmp(rhs); // Construction explicite d'une copie
        tmp.swap(*this);
        return *this;
    }

Comme tu as définis ton constructeur par copie pour qu'il recopie la mémoire allouée dynamiquement, les objets temporaires construits ne pointent pas sur la même zone mémoire que le paramètre, et donc le swap peut faire un simple échange de pointeurs.

[Frifron]

Je ne comprend pas pourquoi utiliser le swap dans l'opérateur d'assignation : l'assignation est unilatérale alors que le swap va échanger les paramètre des 2objets... En d'autre termes, on fait des opérations inutiles...

Sinon désolé mais je ne comprend pas non plus comment utiliser mon swap dans l'opérateur d'affection alors que je veux justement que le comportement soit différent entre les 2 (copie d'un tableau dans un cas et simple échange de pointeur dan l'autre). Ok dans le cas général mais dans mon cas ca ne fonctionne pas. Il y a quelquechose que je ne saisi pas bien quoi.

[Frifron]

Comme tu as définis ton constructeur par copie pour qu'il recopie la mémoire allouée dynamiquement, les objets temporaires construits ne pointent pas sur la même zone mémoire que le paramètre, et donc le swap peut faire un simple échange de pointeurs.

Ok j'ai compris. En effet ca marche. J'avais un peu de mal... je suis fatigué désolé...

Et donc ensuite quelles est la meilleur méthode pour construire la méthode swap. J'en ai proposé quelque unes dan sle premier post. Mais je pense qu'il y a mieux.

[Jean-Marc.Bourguet]

Et donc ensuite quelles est la meilleur méthode pour construire la méthode swap. J'en ai proposé quelque unes dan sle premier post. Mais je pense qu'il y a mieux.

Normalement swap est une fonction membre, definie en utilisant swap sur chacun des membres et n'echoue jamais, meme pas avec une exception. On defini aussi classiquement une fonction libre swap appelant le membre dans le namespace de la classe.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
void C::swap(C& other)
{
   // pour utiliser le swap fourni par les types ou std::swap s'il n'y en a pas
   using std::swap; 
   swap(m1, other.m1);
   swap(m2, other.m2);
   ...
   swap(mn, other.mn);
}
 
void swap(C& c1, C& c2)
{
   c1.swap(c2);
}

[Frifron]

ok, je vais essayer ca.

Merci a tous

[yan]

en complement, si cela t'interesse
http://www.developpez.net/forums/d44...rer-algorithm/

[yan]

SI je comprend bien,
un operateur = devrait plustôt être comme cela ? et c'est le constructeur de recopie qui fait la copie éléments par éléments

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
T& operator=(T rhs) {
        rhs.swap(*this);
        return *this;
    }

en faite j'ai toujours appris l'inverse, qu'il fallait utiliser le couple :
* constructeur de recopie qui utilise l'operateur =
* operateur = qui fait la copie éléments par éléments

[Médinoc]

La "bonne pratique" est pourtant bien de réutiliser le constructeur de copie dans l'opérateur =.

Explication simple: Si ton constructeur de copie utilise l'opérateur = , cela veut dire que toutes les variables membres seront construites puis assignées (au passage, ça veut aussi dire qu'elles doivent donc toutes être assignables).

Quand l'opérateur = utilise le constructeur de copie et le swap, les variables membres sont juste construites, puis swappées (le swap étant toujours "léger"). De plus, les variables ont juste besoin d'être copiables et swappables (même si la plupart du temps, un type qui est à la fois copiable et swappable est généralement assignable).

[Frifron]

C'est ca que je ne compren pas vraiment : pourquoi l'utilisation d'un constructeur par recopie et d'un swap dans l'opérateur d'assignation est il mieux qu'une utilisation de l'assignation optimisée (plus ou moins calquée sur celle du constructeur par recopie avec en plus un test pour le self assignment.). Dans le cas de l'assignation avec swap on va en effet construire et assigner un objet temporaire de portée locale pour rien. Il paraitrait plus naturel et optimisé de faire une assignation unilatérale des membres qu'il faut.