Skip to content

Homework - dining philosophers problem with visualization #50

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lukasz-plewa
Copy link

Modify number of philosophers in main() as parameter to Table object
Modify number of dishes to be eaten by every philosopher in Philosopher
class declaration.

Modify number of philosophers in main() as parameter to Table object
Modify number of dishes to be eaten by every philosopher in Philosopher
class declaration.
@ziobron
Copy link
Contributor

ziobron commented Jan 11, 2021

Let me check this later in next week.

@ziobron ziobron self-requested a review January 11, 2021 21:03
Copy link
Contributor

@ziobron ziobron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Udało mi się złapać wyścig:
https://pastebin.pl/view/300df1dc

Niestety tylko raz się pojawił w trybie release. Potem ani w debug ani realease się nie pojawił, więc nie mam więcej info.

Podoba mi się rozwiązanie. Można jeszcze trochę zoptymalizować wyciągając niektóre rzeczy z sekcji krytycznych. Może nawet niektóre rzeczy można bez blokowania robić.

#include "philosopher.h"

class Fork {
int id_;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

U mnie był błąd:

dining_table.h:19:9: error: private field 'id_' is not used
      [-Werror,-Wunused-private-field]
    int id_;

private:
std::mt19937 m_generator;
std::uniform_int_distribution<int> m_distribution;
static EatTimeEngine *pinstance_;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

polecam implementować singletony Meyersa - https://stackoverflow.com/questions/17712001/how-is-meyers-implementation-of-a-singleton-actually-a-singleton

Najprostsze i bezpieczne bez dodatkowych mechanizmów.


void Table::joinPhilosopherToTheFeast(std::shared_ptr<Philosopher> p) {
std::thread t = std::thread(&Philosopher::threadStartTheFeast, &(*p));
p->setThread(t);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
p->setThread(t);
p->setThread(std::move(t));

PhilStatus getStatus();
void threadStartTheFeast();

void setThread(std::thread &t) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
void setThread(std::thread &t) {
void setThread(std::thread &&t) {


int getId() {
std::lock_guard<std::mutex> lock(mtx_);
return id_;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gdyby id_ było const to do odczytania danych nie trzeba by było blokować mutexa

Comment on lines +87 to +92
void endThread() {
std::lock_guard<std::mutex> lock(mtx_);
if (m_t.joinable()) {
m_t.join();
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nieużywane?

class Table {
int seats_;
int wait_for;
std::shared_ptr<Table> self_;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wygląda na niepotrzebne

drawHeader();
do {
std::unique_lock<std::mutex> mlock(m_update_mtx);
m_cond_update.wait(mlock);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dobry pomysł z synchronizacją wyświetlania za pomocą condition_variable :)

@lukasz-plewa
Copy link
Author

Udało mi się złapać wyścig:
https://pastebin.pl/view/300df1dc

Niestety tylko raz się pojawił w trybie release. Potem ani w debug ani realease się nie pojawił, więc nie mam więcej info.

Podoba mi się rozwiązanie. Można jeszcze trochę zoptymalizować wyciągając niektóre rzeczy z sekcji krytycznych. Może nawet niektóre rzeczy można bez blokowania robić.

Dzięki Łukaszu za tak wnikliwe review. Kulawy i niedojrzały ten mój kod. Postaram się gu ulepszyć wg Twoich wskazówek no i poszukam źródła tego wyścigu.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants