Сотрудничество двух функций через многопоточность


Я собираюсь превратить их (многопоточность) в первой функции после окончания работы уведомлять вторую функцию, которая уже закончила работу, а вторая функция получила через многопоточность.

#include "stdafx.h"
#include <cstdint>
#include <iostream>
#include <mutex>
#include <queue>
#include <condition_variable>
#include <fstream> 
#include <sstream>
#include <algorithm>
std::mutex mutex;
std::queue<std::string> queue;
std::condition_variable conditionVariable;

void ProduceData()
{
   const char* pathToFile = "D:\\text.txt";
   std::ifstream stream(pathToFile);
   if (!stream)
   {
      std::cout << "Can not open file\n";
      return;
   }

   const std::uint8_t maxWordCount = 5;
   std::string word;
   while (stream >> word)
   {
      std::lock_guard<std::mutex> lockGuard(mutex);
      for (std::uint8_t count = 0; (count < maxWordCount) || (count != '\0'); ++count)
        queue.push(word);
      conditionVariable.notify_one();
   }
 }


 void ConsumeData()
 {
   while (true)
   {
    std::unique_lock<std::mutex> uniqueLock(mutex);
    conditionVariable.wait(uniqueLock, [] {return !queue.empty(); });
    while (!queue.empty())
    {
        const std::string& str = queue.front();
        std::size_t numVowels = std::count_if(str.begin(), str.end(), isVowel);
        std::cout << str << "-" << numVowels;
        queue.pop();
    }
    uniqueLock.unlock();
   }
}




int main()
 {
std::thread t1(ProduceData);
std::thread t2(ConsumeData);
t1.join();
t2.join();

  return 0;
 }


366
5
задан 14 февраля 2018 в 05:02 Источник Поделиться
Комментарии
2 ответа

Вашей потребительской нить блокирует производителя в течение длительных периодов времени.

void ConsumeData()
{
while (true)
{
std::unique_lock<std::mutex> uniqueLock(mutex);
conditionVariable.wait(uniqueLock, [] {return !queue.empty(); });
while (!queue.empty())
{
// DO WORK
queue.pop();
}
uniqueLock.unlock();
}
}

Я хотел бы видеть потребитель захватить один элемент из очереди, затем отпустите очереди (так производитель может добавлять работы, а потребитель обработка одного задания.

void ConsumeData()
{
while (true)
{
{
std::unique_lock<std::mutex> uniqueLock(mutex);
conditionVariable.wait(uniqueLock, [] {return !queue.empty(); });

// Note: Don't need to test queue.empty()
// If it is empty then the thread is re-queued on the
// condition variable it is only released when the test
// above is true.
work = queue.front();
queue.pop();
}
// You have the work item so you don't need the lock.
// So out here you work on the item you just retrieved from
// queue. When done you loop back around to get more work.

// DO WORK
}
}
}

Ваш производитель добавляет много элементов в очереди. Но сигналы только один раз. Вы должны после сигнала для каждого элемента добавляется в очередь.

  for (std::uint8_t count = 0; (count < maxWordCount) || (count != '\0'); ++count) {
queue.push(word);
conditionVariable.notify_one();
}

Как Примечание стороны:

В абзаце 2 немного на низкой стороне (приемлемый некоторые). Но на мой взгляд делает код слишком сложен.

6
ответ дан 14 февраля 2018 в 05:02 Источник Поделиться

Я вижу ряд вещей, которые могут помочь вам улучшить вашу программу.

Изолировать конкретной платформы код

Если вы должны иметь stdafx.hрассмотрим заворачивая ее так, что код является портативным:

#ifdef WINDOWS
#include "stdafx.h"
#endif

Убедитесь, что у вас есть все необходимое #includeС

Код использует std::thread но не #include <thread>. Это было не трудно сделать вывод, но техническое обслуживание упрощается, если все требуемые #includeы присутствуют. В то время как код может компилироваться на вашем компьютере, если он делает это неявно опираясь на один из упомянутых заголовков для включения отсутствует один. Это не портативная и в конце концов становится головной болью обслуживания. Лучше использовать документированные #includeы с самого начала.

Используйте только необходимое #includeС

Включают только файлы, которые действительно нужны. Ничего из <sstream> заголовок представляется необходимым здесь, так что я бы рекомендовал опуская его.

Поставить недостающие isVowel функция

Это больше вопрос помогает рецензентов, а не обязательно какой-то изъян в коде, но без реализации isVowel код, рецензенты вынуждены догадываться, что этот код делает.

Позволить пользователю задать имя входного файла

Имя входного файла находится в настоящее время жестко, что, безусловно, значительно ограничивает полезность программы. Рассмотрите возможность использования argc и argv чтобы позволить пользователю указать имена файлов в командной строке.

Избежать "гонки данных"

В ProduceData и ConsumeData функции как потенциально получить доступ к std::cout но вот один ресурс, который может одновременно использоваться другими потоками. Одним из способов исправить это, чтобы использовать std::mutex как это:

std::mutex cout_mutex;

// wherever cout is used:
some_function() {
std::lock_guard<std::mutex> lock(cout_mutex);
std::cout << "Now we can do this safely!\n";
}

Обратите внимание, что std::lock_guard предназначено для использования через РАИИ так, что замок получается, когда объект создан и выпущен, когда она уничтожена, что делает его легким, чтобы избежать ошибок забывания, чтобы разблокировать мьютекс.

Избегать использования глобальных переменных

Текущий код mutex, queue и conditionVariable как глобальные переменные. Это обычно лучше, чтобы явно передавать переменные функции нужно или объявлять их в соответствующее минимально возможный объем, а не через туманные неявные взаимосвязи глобальной переменной. Кроме того, они очень универсальные имена, которые не очень полезны для понимания того, что делает код. Например, я обычно предпочитаю назвать mutex после того, что он защищает, чтобы сделать его легче увидеть, почему он используется на всех; по этой причине, в этом случае я бы посоветовал q_mutex или как.

Свести к минимуму время блокировки

Кодекс в настоящее время содержит такие строки:

while (!queue.empty())
{
const std::string& str = queue.front();
std::size_t numVowels = std::count_if(str.begin(), str.end(), isVowel);
std::cout << str << "-" << numVowels;
queue.pop();
}
uniqueLock.unlock();

Это эффективно держит очереди заблокирована до тех пор, пока очередь полностью опустели, и все выходные понаписали. Это потенциально длительного времени, а также не обязательно. Это хороший дизайн, чтобы попытаться всегда свести к минимуму время блокировки. В этом случае я бы, наверное, просто пережить один раз, а не изнурительные очереди и сделать печать вне защиты замка. Если дублирование строки достаточно быстро, можно переписать его так:

std::string str;
if (!queue.empty())
{
str = queue.front();
queue.pop();
}
uniqueLock.unlock();
std::size_t numVowels{std::count_if(str.begin(), str.end(), isVowel)};
std::cout << str << "-" << numVowels;

Лучше, конечно, было бы воспользоваться советом, а также получить блокировку для std::cout.

Исправить ошибку

В настоящее время производитель имеет этот любопытный кусок кода

for (std::uint8_t count = 0; (count < maxWordCount) || (count != '\0'); ++count)
queue.push(word);

Проблема в том, что даже для count >= maxWordCount вторая часть || положение будет истинным, ведущие в бесконечный цикл. Лучше было бы просто нажать на слово и просто использовать queue.push(word); без петли.

3
ответ дан 14 февраля 2018 в 09:02 Источник Поделиться