Discussion :

[germinolegrand]

Bonjour,
je travaille actuellement avec les sf::Thread sur un système de réseau.
Mais les mutexes ne font que ralentir la procédure de traitement, de réception et d'envoi. En fouillant un peu dans la STL comme à mon habitude, les spécifications de std::list m'ont apparues comme extrêmement favorable au thread-safe lorsqu'on les utilise comme des FIFO. Je m'explique :
push_back n'invalide aucune référence ni aucun itérateur.
pop_front n'invalide que les références et les itérateurs sur l'objet supprimé (autrement dit il invalide begin()).
Or push_back n'utilise que end() ?
Et pop_front seulement begin() ?
la fonction de liaison qui permet de savoir si la file contient un élément est empty() qui se contente de retourner begin() == end().

A votre avis est-ce suffisant pour ne pas avoir de collision entre les threads ? ou bien pop et push invalident temporairement les itérateurs avant de les rendre valides ?

[Kalith]

Ça ne sera pas suffisant non.

Ton thread qui va appeler pop_front() doit savoir avant si empty() retourne false, et il y a de grande chance que cette fonction là utilise des données qui pourraient être atteintes/modifiées simultanément par l'autre thread via push_back().

Personnellement, j'ai utilisé le concept de cet article de Herb Sutter. Ce conteneur n'utilise a priori aucun mécanisme de lock, mais l'ajout de std::atomic<T> fait par le C++11.

Voici le code que j'utilise (celui de l'article est un peu ancien et ne compile pas directement) :

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
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
#ifndef LOCK_FREE_QUEUE_HPP
#define LOCK_FREE_QUEUE_HPP
 
#include <atomic>
 
// Note : implementation is from
// http://www.drdobbs.com/parallel/writing-lock-free-code-a-corrected-queue/210604448
 
template<typename T>
class lock_free_queue
{
    struct node
    {
        node() : next(nullptr) {}
        node(const T& t) : data(t), next(nullptr) {}
 
        T     data;
        node* next;
    };
 
    node* first;
    std::atomic<node*> dummy, last;
 
public :
 
    lock_free_queue() {
        // Always keep a dummy separator between head and tail
        first = last = dummy = new node();
    }
 
    ~lock_free_queue() {
        // Clear the whole queue
        while (first != nullptr)
        {
            node* temp = first;
            first = temp->next;
            delete temp;
        }
    }
 
    // Disable copy
    lock_free_queue(const lock_free_queue& q) = delete;
    lock_free_queue& operator = (const lock_free_queue& q) = delete;
 
    // To be called by the 'producer' thread
    void push(const T& t) {
        // Add the new item to the queue
        (*last).next = new node(t);
        last = (*last).next;
 
        // Clear consumed items
        while (first != dummy)
        {
            node* temp = first;
            first = temp->next;
            delete temp;
        }
    }
 
    // To be called by the 'consumer' thread
    bool pop(T& t) {
        // Return false if queue is empty
        if (dummy != last)
        {
            // Consume the value
            t = (*dummy).next->data;
            dummy = (*dummy).next;
            return true;
        }
        else
            return false;
    }
 
    // To be called by the 'consumer' thread
    bool empty() const {
        return dummy == last;
    }
 
    // This method should not be used in concurrent situations
    void clear() {
        while (first != dummy)
        {
            node* temp = first;
            first = temp->next;
            delete temp;
        }
 
        first = dummy.load();
 
        while (first->next != nullptr)
        {
            node* temp = first;
            first = temp->next;
            delete temp;
        }
 
        last = dummy = first;
    }
};
 
#endif

A noter donc :

• le thread "consommateur" n'a le droit d'appeler que pop() et empty(),
• le thread "producteur" n'a le droit d'appeler que push(),
• les autres méthodes et constructeurs doivent être appelés dans un environnement non concurrent (i.e. quand on sait qu'un seul thread a accès au conteneur).

À voir si ça améliore les performances de ton code.

[gbdivers]

en théorie, si ton thread qui remplit ta liste le fait que lorsqu'il a fini de travailler sur l’élément et fait juste un push et que le thread qui vide la liste en faisant bien un pop avant de lire les données, le risque de collision est faible (ou pas, sur le long terme), faudrait un push et un pop en même temps sur une liste vide

Mais très clairement, faire ça est une très mauvaise pratique, ça peut provoquer des bugs silencieux et non reproductible, donc très complexe à corriger

Et normalement, un mutex, c'est juste un flag atomique. Ca devrait pas tuer les perfs. Si c'est le cas, c'est probablement que tes threads ne font pas juste push et pop, mais travaille sur la liste pendant le lock. Et là, supprimer le mutex serait une catastrophe

Bref, a priori donc, tu devrais utiliser les mutex mais ça devrait pas tuer les perfs

[germinolegrand]

Voilà le code pour le thread interne :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
if(!m_pendingToSendMessages.empty())
    {
        for(auto &s : m_otherServers)
        {
            if(s.second)
            {
                if(s.second->send(*m_pendingToSendMessages.front()) != sf::Socket::Done)
                    throw s.first;
 
                m_pendingToSendMessages.pop_front();
            }
        }
    }

et pour les threads externes :

Code :

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

1
2
std::unique_ptr<sf::Packet> uni(new sf::Packet(packet));
    m_pendingToSendMessages.push_back(std::move(uni));

Le problème des mutexes c'est qu'elles synchronisent les threads or ceci ralentit considérablement le thread de gestion des messages qui est lui critique (il traite en interne bien plus de messages que ce que les autres threads vont traiter). Dans l'idéal il me faudrait un try_lock, mais bon... c'est pas encore supporté dans ma version de g++

[gbdivers]

boost.thread pour try lock ?

Sinon tu utilises quel libs de thread ? SFML ?
Tu peux regarder aussi du côté de boost.asio ou de TBB (http://threadingbuildingblocks.org/d...s_overview.htm) pour les conteneurs threads safe

Code :

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

1
2
std::unique_ptr<sf::Packet> uni(new sf::Packet(packet));
m_pendingToSendMessages.push_back(std::move(uni));

utilise emplace à la place

[germinolegrand]

Tiens, merci pour le emplace, j'avais mis push parce que je faisais quelques manips dessus avant, mais en refactorisant je suis passé à côté ^^.

J'utilise les threads de la SFML 2.0, ainsi que g++4.7 (pas accès à boost).
TBB a l'air bien mais la licence est prohibitive.

@Kalith : J'ai été voir l'article sur drdobb's ainsi que ton code mais l'usage de std::atomic n'est pas implémenté dans ma version de g++ (d'après la doc : Order and consistency : Not yet, section 29 de cette page : http://gcc.gnu.org/onlinedocs/libstd...tatus.iso.200x ), mais je me trompe peut-être et les parties manquante ne me concernent pas ?

Pour le try_lock j'ai fait ma propre version avec un bool est-ce correcte ?

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
48
 
class Mutex
{
public:
	Mutex() = default;
	Mutex(const Mutex&) = delete;
	~Mutex() = default;
 
	bool try_lock()
	{
	    if(m_prim)
            return false;
 
        m_prim = true;
        return true;
	}
 
	void lock()
	{
	    while(!try_lock());
	}
 
	void unlock()
	{
	    m_prim = false;
	}
 
private:
    bool m_prim = false;
};
 
class Lock
{
public:
	Lock(Mutex &m): m_mut(m)
	{
	    m_mut.lock();
	}
 
	Lock(const Lock&) = delete;
 
	~Lock()
	{
	    m_mut.unlock();
	}
private:
    Mutex &m_mut;
};

[Kalith]

J'ai également gcc 4.7 et ça compile, tout du moins. Maintenant je ne sais pas si tout fonctionne comme prévu, mais ça m'étonnerais que les gars de chez gcc sortent sciemment une version buggée de leur compilateur. De mon côté, je n'ai pas vu de problème quoi qu'il en soit.

Edit : Sur cette page, il est écrit que les opérations atomiques sont supportées depuis gcc 4.4