Discussion :

[Bktero]

Hello,

J'ai besoin d'une fonction pour générer des UID de manière thread-safe. Je me suis dis que je devais pouvoir faire ça avec un std::atomic :

Code :

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

1
2
3
4
5
6
using uid_type = std::uint32_t;
 
uid_type getNextUID() {
	static std::atomic<uid_type> uid;
	return uid++;
}

Est-ce que c'est bon ? Est-ce que ça fait la même chose que cette version avec un mutex ?

Code :

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

1
2
3
4
5
6
7
uid_type getNextUID2() {
	static uid_type uid;
	static std::mutex mutex;
 
	std::lock_guard guard{mutex};
	return uid++;
}

[Bktero]

Du coup j'ai fait un test, je lance 100 threads qui font 5000 fois l'action suivante : générer un UID, locker un mutex, pousser l'UID dans un vecteur, déverrouiller le mutex.

Dans le main(), je join() tous les threads et je regarde que tous les UID dans le vecteur sont différents.

L'implémentation avec std::atomic passe le test. Une version sans le std::atomic ne passe pas du tout le test. J'en déduis que ça fait ce que je veux mais si quelqu'un à des remarques intelligentes à faire je suis preneur !

[Bousk]

Combiner atomic et lock me parait redondant.
Atomic serait pour moi l'équivalent de lock + variable volatile.
Du coup autant utiliser atomic si tu l'as.

[Bktero]

Une erreur s'est glissée dans getNextUID2(), la variable n'est pas atomique J'ai corrigé mon code.

Pour toi, mon première implémentation est bonne ?

[dalfab]

L'atomic sert justement à garantir le thread-safe sur une variable simple, il est même plus puissant qu'un mutex car il garanti aussi l'atomicité entre une interruption et un thread, ce que le mutex ne peut pas faire. Utiliser un mutex par dessus n'a absolument aucun intérêt, lui il n'apporte quelque chose que quand il y a plusieurs actions sur des entités que l'on veut voir comme insécables.

[Bousk]

Pour toi, mon première implémentation est bonne ?

Vue qu'elle marche, est simple, compréhensible et facilement lisible, oui elle est bonne et peut-être même la bonne.

[Bktero]

Allez je valide ce code Merci !

[Médinoc]

Je suis surpris que la version qui utilise un mutex au lieu de atomic foire. Peut-être est-ce lié au fait que ce soit spécifiquement une post-incrémentation au retour?

[Bktero]

La version sans std::atomic de mon test, c'était ça :

Code :

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

1
2
3
4
5
6
using uid_type = std::uint32_t;
 
uid_type getNextUID() {
	static uid_type uid;
	return uid++;
}

Transformer la variable statique uid en std::atomic<uid_type> rend le code fonctionnel.

[Bousk]

Je suis surpris que la version qui utilise un mutex au lieu de atomic foire. Peut-être est-ce lié au fait que ce soit spécifiquement une post-incrémentation au retour?

En regardant l'assembleur on verrait sûrement que la variable est chargée, puis le lock se met en place. Quand il continue, la variable déjà chargée a une ancienne valeur et pas du tout celle après incrémentation. D'où ma mention de volatile qui est sencé contrer ça, mais utiliser std::atomic est plus simple je trouve.

[JolyLoic]

Non, volatile en C++ n'est absolument pas sensé être utiliser pour synchroniser plusieurs threads. Jamais. volative est destiné aux variables dont la valeur peut changer en, dehors de tout contrôle du programme (typiquement, du matériel qui va automatiquement modifier de la mémoire sans que tu le saches).

La version avec un mutex ne foire pas, et n'a pas besoin de volatile pour marcher. On parle dans ce thread de 3 versions:
- Aucune protection, ça ne marche pas, et c'est normal
- mutex et lock : ça marche
- atomic : ça marche.

[Bktero]

Scott Meyers a un item sur std::atomic vs volatile dans "Effective Modern C++". Le titre est explicite : "Use std::atomic for concurrency, volatile for special memory"

Je confirme les 3 possibilités exprimées par JolyLoic. Les autres sont des erreurs de ma part ou des mauvaises interprétations dues à un manque de clarté de mes propos

[Médinoc]

Ah, je comprends mieux. Merci.