Un consumidor de productores múltiples con C ++ 11 -- ++ campo con c++11 campo con concurrency campo con producer-consumer camp codereview Relacionados El problema

A multi-threaded Producer Consumer with C++11


32
vote

problema

Español

Estoy tratando de aprender la programación concurrente en C ++ 11. Intenté escribir código para un problema clásico de concurrencia al consumidor de productores. ¿Podría revisar y hacer algún comentario al respecto?

  Or1  
Original en ingles

I am trying to learn concurrent programming in C++11. I tried to write code for a classic producer consumer concurrency problem. Would you please review and make any comments about it?

#include <iostream> #include <thread> #include <deque> #include <mutex> #include <chrono> #include <condition_variable>  using std::deque; std::mutex mu,cout_mu; std::condition_variable cond;  class Buffer { public:     void add(int num) {         while (true) {             std::unique_lock<std::mutex> locker(mu);             cond.wait(locker, [this](){return buffer_.size() < size_;});             buffer_.push_back(num);             locker.unlock();             cond.notify_all();             return;         }     }     int remove() {         while (true)         {             std::unique_lock<std::mutex> locker(mu);             cond.wait(locker, [this](){return buffer_.size() > 0;});             int back = buffer_.back();             buffer_.pop_back();              locker.unlock();             cond.notify_all();             return back;         }     }     Buffer() {} private:     deque<int> buffer_;     const unsigned int size_ = 10; };  class Producer { public:     Producer(Buffer* buffer)     {         this->buffer_ = buffer;     }     void run() {         while (true) {             int num = std::rand() % 100;             buffer_->add(num);             cout_mu.lock();             std::cout << "Produced: " << num << std::endl;             std::this_thread::sleep_for(std::chrono::milliseconds(50));             cout_mu.unlock();         }     } private:     Buffer *buffer_; };  class Consumer { public:     Consumer(Buffer* buffer)     {         this->buffer_ = buffer;     }     void run() {         while (true) {             int num = buffer_->remove();             cout_mu.lock();             std::cout << "Consumed: " << num << std::endl;             std::this_thread::sleep_for(std::chrono::milliseconds(50));             cout_mu.unlock();         }     } private:     Buffer *buffer_; };  int main() {     Buffer b;     Producer p(&b);     Consumer c(&b);      std::thread producer_thread(&Producer::run, &p);     std::thread consumer_thread(&Consumer::run, &c);      producer_thread.join();     consumer_thread.join();     getchar();     return 0; } 
           
         
         

Lista de respuestas

29
 
vote

Prefiero referencia sobre puntero

Dado que el productor y el consumidor deben tienen un búfer que debe pasar por referencia (en lugar de puntero). Esto también se asegura de que no haya confusión sobre la propiedad del búfer (el propietario de un puntero es responsable de eliminarlo). Al utilizar un puntero RAW, no puede decirle al propietario, sino mediante el uso de una referencia que está declarando explícitamente que no está pasando la propiedad.

  class Producer {     Buffer&  buffer_; public:     Producer(Buffer& buffer)         : buffer_(buffer)     {}   

Prefiero evitar esto- & gt;

Cuando usa this-> significa que tiene problemas de alcance con sus variables (que es un olor a código para un diseño malo). Use nombres de variables precisos para que no haya confusión en el lugar donde pertenecen las variables.

Variables miembros sobre globals.

Parece que su encapsulación del mu y 9988776655544333 está mal.

  std::mutex mu,cout_mu;   

Porque son globales, usas un solo mutex / condición para todos los objetos de búfer. Esto parece un defecto de diseño. Parece que el mutex / condición pertenece a la clase para que solo esté bloqueando el búfer que está manipulando (para que pueda usar múltiples buffers en la misma aplicación).

Dormir dentro de la cerradura

          cout_mu.lock();         std::cout << "Produced: " << num << std::endl;         std::this_thread::sleep_for(std::chrono::milliseconds(50));         cout_mu.unlock();   

@morwenn ya sugirió usar un bloqueo alcoplado para asegurarse de que el mutex estuviera bloqueado y desbloqueado correctamente. Pero también movería el sueño afuera de la cerradura. Esto hace que el hilo de corriente se bloquee, pero el otro rosca no puede continuar a medida que se mantiene la cerradura mientras está durmiendo.

          {             std::unique_lock<std::mutex> locker(cout_mu);             std::cout << "Produced: " << num << std::endl;         }         std::this_thread::sleep_for(std::chrono::milliseconds(50));   

Preferir ' n' sobre std::endl

  std::cout << "Produced: " << num << std::endl;   

La diferencia entre los dos es un al ras. Por lo general, no desea al ras manual (la corriente tiene buenas técnicas de lavado incorporadas). Por lo general, suele ser mejor dejar que las corrientes se enjuaguen cuando sea apropiado.


En respuesta a los comentarios:

  class Buffer { public:     void add(int num) {         while (true) {             std::unique_lock<std::mutex> locker(mu);             cond.wait(locker, [this](){return buffer_.size() < size_;});             buffer_.push_back(num);             locker.unlock();             cond.notify_all();             return;         }     }     int remove() {         while (true)         {             std::unique_lock<std::mutex> locker(mu);             cond.wait(locker, [this](){return buffer_.size() > 0;});             int back = buffer_.back();             buffer_.pop_back();             locker.unlock();             cond.notify_all();             return back;         }     }     Buffer() {} private:    // Add them as member variables here     std::mutex mu;     std::condition_variable cond;     // Your normal variables here     deque<int> buffer_;     const unsigned int size_ = 10; };   
 

Prefer Reference over Pointer

Since the producer and consumer must have a buffer you should pass it by reference (rather than pointer). This also makes sure there is no confusion over ownership of the buffer (the owner of a pointer is responsible for deleting it). By using a RAW pointer you can not tell the owner but by using a reference you are explicitly stating that you are not passing ownership.

class Producer {     Buffer&  buffer_; public:     Producer(Buffer& buffer)         : buffer_(buffer)     {} 

Prefer to avoid this->

When you use this-> it means you have scoping issues with your variables (which is a code smell for bad design). Use accurate variable names so that there is no confusion on where the variables belongs.

Member variables over globals.

Looks like your encapsulation of the mu and cond is wrong.

std::mutex mu,cout_mu; 

Because they are global, you use a single mutex/condition for all buffer objects. This looks like a design flaw. It seems like the mutex/condition belongs to the class so that you are just locking the buffer you are manipulating (so you can use multiple buffers in the same application).

Sleeping inside lock

        cout_mu.lock();         std::cout << "Produced: " << num << std::endl;         std::this_thread::sleep_for(std::chrono::milliseconds(50));         cout_mu.unlock(); 

@Morwenn already suggested using scoped lock to make sure the mutex were correctly locked and unlocked. But I would also move the sleep outside the lock. This causes the current thread to stall but the other thread can not continue as the lock is still held while it is sleeping.

        {             std::unique_lock<std::mutex> locker(cout_mu);             std::cout << "Produced: " << num << std::endl;         }         std::this_thread::sleep_for(std::chrono::milliseconds(50)); 

Prefer '\n' over std::endl

std::cout << "Produced: " << num << std::endl; 

The difference between the two is a flush. Usually you do not want to manually flush (the stream have good flushing techniques built in). So it is usually best to let the streams flush themselves when appropriate.


In answer to comments:

class Buffer { public:     void add(int num) {         while (true) {             std::unique_lock<std::mutex> locker(mu);             cond.wait(locker, [this](){return buffer_.size() < size_;});             buffer_.push_back(num);             locker.unlock();             cond.notify_all();             return;         }     }     int remove() {         while (true)         {             std::unique_lock<std::mutex> locker(mu);             cond.wait(locker, [this](){return buffer_.size() > 0;});             int back = buffer_.back();             buffer_.pop_back();             locker.unlock();             cond.notify_all();             return back;         }     }     Buffer() {} private:    // Add them as member variables here     std::mutex mu;     std::condition_variable cond;     // Your normal variables here     deque<int> buffer_;     const unsigned int size_ = 10; }; 
 
 
       
       
15
 
vote

Tengo algunos comentarios sobre su código:

  • USO this->0 es ideal para manejar mutexes ya que automáticamente desbloquea el mutex adquirido al salir del alcance. Esa es una verdadera herramienta. Realmente deberías usarlo en todas partes que puedas. Usándolo constantemente se asegurará de que no se puede olvidar de desbloquear cualquier mutex. Además, se asegura de que los mutexes se desbordan incluso cuando se lanza una excepción.

      this->1  
  • this->2 es seguro por la norma y muchas plataformas no se molestan en hacerlo a la caja fuerte, porque los mutexes de bloqueo y desbloqueo tienen un precio. Por lo tanto, debe usar los nuevos generadores de números pseudo-aleatorios de this->3 para asegurarse de que puede usar varios productores a la vez:

      this->4  

    Este módulo tiene una curva de aprendizaje más alta que this->5 , pero es más potente, más flexible y puede 99887766555443316 los objetos para hacer el hilo de generación de números pseudo-aleatorios. Seguro sin perder el tiempo necesario para bloquear / desbloquear mutexes mantener la seguridad del hilo.

  • Use las listas de inicialización de constructor para construir sus objetos cuando pueda asignar a las variables miembros en el cuerpo del constructor:

      this->7  
 

I have a few comments about your code:

  • Using std::lock_guard is great to handle mutexes since it automatically unlocks the acquired mutex when leaving the scope. That's a real great tool. You should really use it everywhere you can. Using it consistently will make sure that you can't forget to unlock any mutex. Moreover, it makes sure that mutexes are unlocked even when an exception is thrown.

    while (true) {     int num = buffer_->remove();     std::lock_guard<std::mutex> lock(cout_mu);     std::cout << "Consumed: " << num << std::endl;     std::this_thread::sleep_for(std::chrono::milliseconds(50)); } 
  • std::rand is not guaranteed to be thread-safe by the standard and many platforms don't bother to make it thread-safe because locking and unlocking mutexes has a price. Therefore you should use the new pseudo-random number generators from <random> instead to make sure that you can use several producers at once:

    void run() {     // Non-deterministic pseudo-random numbers generator     thread_local std::random_device rd;     // Pseudo-random engine     thread_local std::mt19937 engine(rd());     // Linear distribution in [0, 100[     thread_local std::uniform_int_distribution<int> dist(0, 99);      while (true) {         int num = dist(engine);         buffer_->add(num);         std::lock_guard<std::mutex> lock(cout_mu);         std::cout << "Produced: " << num << std::endl;         std::this_thread::sleep_for(std::chrono::milliseconds(50));     } } 

    This module has a higher learning curve than std::rand but it is more powerful, more flexible and you can thread_local the objects to make the pseudo-random number generation thread-safe without losing the time needed to lock/unlock mutexes keep the thread safety.

  • Use constructor initialization lists to construct your objects when you can instead of assigning to the member variables in the body of the constructor:

    Producer(Buffer* buffer):     buffer_(buffer) {} 
 
 

Relacionados problema

6  Implementación del productor / consumidor para el procesamiento paralelo en el TPL  ( Producer consumer implementation for parallel processing in the tpl ) 
Me he quitado esto a sus huesos desnudos eliminando la lógica de parada / cancelación, etc. para mantenerlo simple. El Producer es una clase muy simple qu...

6  Productor genérico-consumidor en C ++ 11  ( Generic producer consumer in c11 ) 
Escribí el siguiente productor simple: 1 problema del consumidor, en intento de aprender algunos enhebrados / genéricos C ++ 11. #include <iostream> #inclu...

2  Boost :: Hilo productor consumidor  ( Boostthread producer consumer ) 
Soy nuevo en boost::thread y estoy haciendo un productor-consumidor con un Monitor . Así es como lo he codificado hasta ahora: //{ Declarations in heade...

3  Opción de implementación óptima para el consumidor productor en Java  ( Optimal implementation choice for producer consumer in java ) 
Estoy buscando una implementación óptima del consumidor de productores en Java con los siguientes aspectos: El productor lee desde el socket Server1, y es...

2  Consumidor del productor usando tpl flujo de datos  ( Producer consumer using tpl dataflow ) 
He escrito la siguiente clase que se supone que debe actuar como un consumidor de mensajes que se transmiten de los productores y los envían a Amazon SQS, he ...

2  Usando bloqueo de bloqueo con paralelo  ( Using blockingcollection with parallel foreach ) 
Tengo un proceso que hace un montón de procesamiento de documentos. Una de las tareas que se realizan es OCR, que es la parte más lenta del proceso, por lo qu...

5  Cola delimitada con múltiples consumidores y múltiples productores  ( Bounded queue with multiple consumers and multiple producers ) 
Fue una pregunta de entrevista: debe implementar una clase - BlockingQueue con esas restricciones: 2 métodos: agregar un artículo y tomar un artículo de ...

7  Cola tamponada para el registro  ( Buffered queue for logging ) 
Me gustaría usar una cola amortiguada para registrar a través de una WEBAPI que maneja múltiples aplicaciones. Este ayudante debe reducir el bloqueo que se pr...

5  Colaboración de las dos funciones a través de MultiPhreading  ( Collaboration of the two functions through multithreading ) 
Voy a convertirlos (multi-roscado) a la primera función después del final de la operación para notificar la segunda función que ya ha terminado el trabajo y l...

2  Productor - Interacción del consumidor  ( Producer consumer interaction ) 
Tengo una implementación de la interacción multinfilada productora-consumidor. Funciona, pero siento que durante los estados de espera de ejecución ocurren co...




© 2022 respuesta.top Reservados todos los derechos. Centro de preguntas y respuestas reservados todos los derechos