NikoOstrouhov / PPlab2

1 stars 0 forks source link

Lab2 замечания 2 #2

Open Andrewbooq opened 4 days ago

Andrewbooq commented 4 days ago

В целом всё хорошо интересное решение с std::vector<std::tuple<int, int, int>, но, что не нравится: 1 Что у нас ThreadPool вызовом waitForCompletion блокируется. Т.е. позвав ThreadPool::waitForCompletion у нас происходит блокировка пока что-то там не произойдет и в main на это никак не можем повлиять. Так делать нехорошо. Надо сделать метод, который просто возврящает m_jobs.empty()/m_jobs.size() (назвать его соответственно коду), а вот в main крутится и ждать. Иначе использование объекта ThreadPool накладывает ограничения. Я бы не просто возвращал empty(), а размер очереди, более информативно.

int main()
{
...
    while (pool.getJobCnt !=0 )
    {
        std::this_thread::sleep_for(std::chrono::milliseconds(100));
    }
...
}

2 Если m_jobs.empty(), означает ли это, что у нас все потоки закончили рассчеты? Такое ощущение, что нам перед for (auto i : completed_jobs) надо подождать, чтобы количество добавленных job в очередь равно размерности completed_jobs (Все добавленные джобы не только запущены, о чем свидетельстыует нулевой размер очереди, а еще и завершены, о чем должен свидетельствовать соответствующи размер completed_jobs). Плюс обращение к completed_jobs даже в main должно быть из под jobs_mutex, Так как main - это тоже поток, который асинхронно получает доступ к данным наряду с джобами.

3 Разве обрашение к m_jobs в waitForCompletion не надо под m_mutex?

4 Написать Makefile, чтобы проект собирался под Linux.

PS: Вопрос на засыпку: если main типа int, разве не надо в конце делать return ? И почему компилятор на это не ругается?

NikoOstrouhov commented 23 hours ago

Исправил ошибки и добавил makefile. Я не совсем понял про "Плюс обращение к completed_jobs даже в main должно быть из под jobs_mutex, Так как main - это тоже поток, который асинхронно получает доступ к данным наряду с джобами". Мы ведь main потоком обращаемся к completed_jobs, но при этом никак его не изменяем, а просто читаем из него информацию. Я добавил туда мьтекс, но хочу понять зачем он там нужен.

{
        std::lock_guard<std::mutex> lg(jobs_mutex);
        for (auto i : completed_jobs)
        {
            os << "Thread " << std::get<0>(i) << " completed job from index " << std::get<1>(i) << " to index " << std::get<2>(i) << std::endl;
        }
        std::cout << "\nThreads ended all work and wrote information in " << filename << std::endl;
 }

Касательно вопроса про return 0; .Как я понял это особенность main, компилятор по умолчанию предполагает return 0 в конце int main() даже если return не написан, однако все же стоит писать return 0 в конце, для совместимости с более старыми версиями c++, у которых у main эта особенность отсутствует.

Andrewbooq commented 8 hours ago

1 waitForCompletion переделал. Такой вариант годится. 2 completed_jobs теперь ждет размерности == getJobCount(). Не идеально, но пойдет, можно оставить. Если интересно - то я бы завел переменную, и посчитал, а сколько же я добавил в очередь задач и ждал этого количества в completed_jobs. и getJobCount не очень удачное название, как буд-то мы возвращаем размер очереди. 3 больше не существует 4 Makefile не проверял?

ostroukhov/PPlab2$ make 
make: *** No rule to make target 'main.cpp', needed by 'main.o'.  Stop.

У тебя в проекте нет main.cpp. И еще имена файлов с пробелами, это не относится к лабе, поэтому подскажу, вот так я сделал Makefile,

.PHONY: all clean
all: lab2PPLinux

lab2PPLinux: PP\ lab2.o ThreadPool.o
    g++ PP\ lab2.o ThreadPool.o -o lab2PPLinux

PP\ lab2.o: PP\ lab2.cpp
    g++ -c PP\ lab2.cpp

ThreadPool.o: ThreadPool.cpp
    g++ -c ThreadPool.cpp

clean:
    rm lab2PPLinux *.o *.txt

PS: image На самом деле, еще в стандарте С99 говорится, что main можно завершеть без return и это эквивалентно return 0. https://open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf Но лучше, конечно, явно писать возврат значения.

Andrewbooq commented 8 hours ago

На счет твоего вопроса про доступ из main для чтения - напомни, объясню на лабе сегодня.

Andrewbooq commented 8 hours ago

Все таки надо добавить немного юзер френдли к твоей программе. Ну представь, ты написал прогу, и отдаешь .exe файл своему отцу - папа, зацени какую я классную прогу написл. Предположим он не знает ничего про с++ вообще, исходники для него - абракадабра. Он запускает и видит сообщение, даже может посмотреть Data.txt. Но, наверняка, будет вопрос - а что делает твоя прога? И что лежит в Data.txt? И пока ты ему не объянишь - ничего не понятно. Надо добавить вывод информации и подсказок таким образом, чтобы пользователь, не знающий с++ и не посещающий наши лабы понял что происходит.

Threads start work

Threads ended all work and wrote information in Data.txt
Thread 2014 completed job from index 57336 to index 66891
Thread 8369 completed job from index 66892 to index 76441
Thread 8700 completed job from index 47780 to index 57335
Thread 8671 completed job from index 38224 to index 47779
Thread 7827 completed job from index 28668 to index 38223
Thread 7274 completed job from index 9556 to index 19111
Thread 7158 completed job from index 19112 to index 28667
Thread 2764 completed job from index 0 to index 9555