Discussion :

[Fooshi]

Bonjour,
je poste ici un bout de code que j'aimerais optimiser, c'est donc une boucle while qui dans un premier temps calcule la taille d'un message alloue l'espace d'un pointeur avec cette taille puis dans un deuxieme temps remplis le buffer (2 passages donc). Cet algorithme je vais en avoir besoin souvent dans mon programme.
Quelqu'un peut t'il m'aider a optimiser ce code, moins de lignes et quelque chose de peu etre moins crade ?

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
27
28
29
30
31
32
do
	{
		msg_len = 0;
		if (msg)		memcpy(msg + msg_len, "", 1);				// An empty string selects the non-prepared statement named.
		msg_len += 1;
		if (msg)		memcpy(msg + msg_len, query, strlen(query));// Query string to analyse.
		msg_len += strlen(query);
		if (count > 0 && paramtypes)
		{
			if (msg)	sql_put_int(sizeof(int16), count, msg + msg_len);		// Number format codes following parameters.
			msg_len += sizeof(int16);
			for (i=0; i<count; i++)
			{
				if (msg)sql_put_int(sizeof(int32), paramtypes[i], msg + msg_len);// Object ID of the data type of the parameter.
				msg_len += sizeof(int32);
			}
		}
		else
		{
			if (msg)	sql_put_int(sizeof(int16), 0, msg + msg_len);			// Many types of data specified parameter.
			msg_len += sizeof(int16);
		}		
		if (msg)
		{
			msg[msg_len] = '\0';
			msg_len++;
			break;
		}
		msg_len++;
		msg = (char *) malloc(msg_len);	
	}
	while (msg);

char * msg = NULL en debut de boucle.

Merci d'avance.

[jowo]

Bonjour,

sans vraiment connaître les aboutissants de cette fonction, voici une approche possible sans boucle:

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
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
size_t msg_len = 0;
char *msg = null;
//do
//{
		//**** Determine the size of buffer and allocate it ****
    //msg_len = 0;
		//msg_len += 1;
		//msg_len += strlen(query);
		msg_len = strlen(query) + 1;		
		if (count > 0 && paramtypes)
		{
			msg_len += sizeof(int16) + count * sizeof(int32);
		}
		else
		{
			msg_len += sizeof(int16);
		}		
		// @FIXME: N'a-t-on pas déjà réservé une place pour le caractère terminal
    msg_len++;
 
		msg = (char *) malloc(msg_len);
		if (msg != null) {
      // Fill the buffer
      msg_len = 0;
      memcpy(msg + msg_len, "", 1);				// An empty string selects the non-prepared statement named.
		  msg_len += 1;
		  memcpy(msg + msg_len, query, strlen(query));// Query string to analyse.
		  if (count > 0 && paramtypes)
      {
			  sql_put_int(sizeof(int16), count, msg + msg_len);		// Number format codes following parameters.
			  msg_len += sizeof(int16);
			  for (i=0; i<count; i++)
        {
				  sql_put_int(sizeof(int32), paramtypes[i], msg + msg_len);// Object ID of the data type of the parameter.
				  msg_len += sizeof(int32);
        }
		  }
		  else
      {
			  sql_put_int(sizeof(int16), 0, msg + msg_len);			// Many types of data specified parameter.
			  msg_len += sizeof(int16);
		  }
      msg[msg_len] = '\0';
			msg_len++;
	  }
//  }
//	while (msg);

[Fooshi]

Cela reviendrais a doubler les lignes de calcul de la taille du message, une fois pour pouvoir allouer la taille et la deuxième fois pour indexer l'écriture des données.

[jowo]

Cela reviendrais a doubler les lignes de calcul de la taille du message, une fois pour pouvoir allouer la taille et la deuxième fois pour indexer l'écriture des données.

Je ne comprends pas la remarque....

L'addition peut être fait en une seule ligne.

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
 
msg_len = strlen(query) + 1;		
		if (count > 0 && paramtypes)
		{
			msg_len += sizeof(int16) + count * sizeof(int32);
		}
		else
		{
			msg_len += sizeof(int16);
		}		
		// @FIXME: N'a-t-on pas déjà réservé une place pour le caractère terminal
    msg_len++;

peut être simplifié en

Code :

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

1
2
3
 
msg_len = strlen(query) + 2 + sizeof(int16)
              ((count > 0 && paramtypes) ? count * sizeof(int32) : 0);

Si tu ne veux pas déterminer la taille du tampon. Travaille en taille fixe de tampon si c'est possible.

[Fooshi]

Ce que je voulais dire c'est que dans ce cas :

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
27
28
29
30
31
32
33
34
35
36
37
38
39
 
size_t msg_len = 0;
char *msg = null;
		msg_len = strlen(query) + 1;		
		if (count > 0 && paramtypes)
		{
			msg_len += sizeof(int16) + count * sizeof(int32);
		}
		else
		{
			msg_len += sizeof(int16);
		}		
    msg_len++;
 
		msg = (char *) malloc(msg_len);
		if (msg != null) {
 
      msg_len = 0;
      memcpy(msg + msg_len, "", 1);				
		  msg_len += 1;
		  memcpy(msg + msg_len, query, strlen(query));
		  if (count > 0 && paramtypes)
      {
			  sql_put_int(sizeof(int16), count, msg + msg_len);		
			  msg_len += sizeof(int16);
			  for (i=0; i<count; i++)
        {
				  sql_put_int(sizeof(int32), paramtypes[i], msg + msg_len);
				  msg_len += sizeof(int32);
        }
		  }
		  else
      {
			  sql_put_int(sizeof(int16), 0, msg + msg_len);			
			  msg_len += sizeof(int16);
		  }
      msg[msg_len] = '\0';
			msg_len++;
	  }

tu double au niveau du code les calculs de taille (msg_len += 1 etc ...), il y en a deux fois plus.

[jowo]

Bonjour,

Alors écris une fonction semblable à memcpy mais qui au lieu de retourner le pointeur dest retourne la prochaine cellule à accéder....

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
 
msg_len = strlen(query) + 2 + sizeof(int16)
              ((count > 0 && paramtypes) ? count * sizeof(int32) : 0);
 
msg = malloc(msg_len);
dst = msg;
dst = my_memcpy(dst, "", 1);
dst = my_memcpy(dst, query, strlen(query));
if (count > 0 && paramtypes) {
  dst = my_sql_put_int(sizeof(int16), count, dst);		
  for (i=0; i<count; i++) {
    dst = my_sql_put_int(sizeof(int32), paramtypes[i], dst);
  }
}
else {
  dst = my_sql_put_int(sizeof(int16), 0, dst);			
}
*dst = '\0';
}
++msg_len;

[Fooshi]

Merci de ton conseil, je vais etudier ca

[diogene]

- Stockez la valeur de strlen(query), il est inutile de l'appeler plusieurs fois. Ce peut être pénalisant si query est un peu long.

- Il n'est pas du tout optimal de recoder un memcpy(). On ne peut faire que nettement moins performant que la fonction standard

-

Code :

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

1
2
msg_len = 0;
memcpy(msg + msg_len, "", 1);

Ce genre de chose est en fait

Code :

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

msg[0] = 0;

[jowo]

A tester en prenant note des remarques de DIOGENE

Code :

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

1
2
3
4
5
6
7
8
 
function my_memcopy(void *d, void *s, size_t c) {
  void *e = s + c;
  while (s != e) {
   *(d++) = *(s++);
  }
  return d; 
}

[Invité]

Bonjour Jowo,
Cela ressemble fort à la fonction strcpy() de base, mais si le tableau référencé par le pointeur sur d, n'a pas été dimensionné, il y a un y a une très grande probabilité d'écrasement de mémoire.
En plus, comme les valeurs à copier sont du void combien d'octets seront copiés?
La réponse est naturellement c. Cette fonction est exactement ce que fait strncpy(), au transtypage près.

Je vois le problème autrement, soit on peut trouver une longueur maximum, par exemple une carte (80 caractères), on passe souvent dans cette fonction, alors on la réserve une fois pour toutes : Tab[80];
Soit cette longueur varie dans des proportions importantes, alors malloc() puis free() me paraissent la meilleure solution. Mais il ne faut pas oublier que ces deux fonctions prennent du temps.

Cordialement.

[Fooshi]

@diogene : bien vu , mais memcpy(msg + msg_len, "", 1); est plus 'comprehensible' , on vois que l'on insere un caractere null

sinon je ne ferais pas ma propre fonction, autant utiliser un memcpy ou strcpy de base.

[diogene]

@diogene : bien vu , mais memcpy(msg + msg_len, "", 1); est plus 'comprehensible' , on vois que l'on insere un caractere null

Mais si tu veux optimiser le code, assigner un caractère à 0 est bien plus efficace que l'appel à une fonction qui de plus, comme elle traite le cas général va au moins faire des tests sur le nombre de caractères qu'elle doit écrire.
Si tu veux mettre en évidence le caractère null, écrit

Code :

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

msg[0] = '\0';

sinon je ne ferais pas ma propre fonction, autant utiliser un memcpy ou strcpy de base.

Tu y perdrais, c'est une fonction de base qui doit être pleinement optimisée en bibliothèque

PS : Est-ce voulu que dans le msg final, il reste un '\0' en tête ?

[gl]

A tester en prenant note des remarques de DIOGENE

Code :

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

1
2
3
4
5
6
7
8
 
function my_memcopy(void *d, void *s, size_t c) {
  void *e = s + c;
  while (s != e) {
   *(d++) = *(s++);
  }
  return d; 
}

Attention, en C standard, il n'y a pas d'arithmétique sur des pointeurs void*.

[orfix]

N'y a-t-il pas de fuite mémoire ? tu écrases msg à chaque tour de boucle :

Code :

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

1
2
3
4
5
6
do
{
	<...>
	msg = (char *) malloc(msg_len);	
}
while (msg);

[gl]

N'y a-t-il pas de fuite mémoire ? tu écrases msg à chaque tour de boucle :

Code :

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

1
2
3
4
5
6
do
{
	<...>
	msg = (char *) malloc(msg_len);	
}
while (msg);

Non, si msg est déjà alloué, il y a une sortie de la boucle juste avant :

Code :

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

1
2
3
4
5
6
if (msg)
{
	msg[msg_len] = '\0';
	msg_len++;
	break;
}

Mais ce n'est pas très lisible.

[Fooshi]

@ Diogene : Oui c'est voulu, c'est d'ailleurs plus clean je trouve.