Я работаю над программой, которая имитирует заправку. У каждого вагона на вокзале своя нить. Каждая машина должна пройти через одну битовую маску, чтобы проверить, открыт ли насос, и, если это так, обновить битовую маску, заполнить и сообщить другим машинам, что насос теперь открыт. Мой текущий код работает, но есть некоторые проблемы с балансировкой нагрузки. В идеале все насосы используются в одинаковом количестве, и все машины получают одинаковую заправку.
РЕДАКТИРОВАТЬ: моя программа в основном требует несколько машин, насосов и времени, чтобы выполнить тест для. В течение этого времени автомобили будут проверять наличие открытого насоса, постоянно вызывая эту функцию.
int Station::fillUp()
{
// loop through the pumps using the bitmask to check if they are available
for (int i = 0; i < pumpsInStation; i++)
{
//Check bitmask to see if pump is open
stationMutex->lock();
if ((freeMask & (1 << i)) == 0 )
{
//Turning the bit on
freeMask |= (1 << i);
stationMutex->unlock();
// Sleeps thread for 30ms and increments counts
pumps[i].fillTankUp();
// Turning the bit back off
stationMutex->lock();
freeMask &= ~(1 << i);
stationCondition->notify_one();
stationMutex->unlock();
// Sleep long enough for all cars to have a chance to fill up first.
this_thread::sleep_for(std::chrono::milliseconds((((carsInStation-1) * 30) / pumpsInStation)-30));return 1;
}
stationMutex->unlock();
}
// If not pumps are available, wait until one becomes available.
stationCondition->wait(std::unique_lock<std::mutex>(*stationMutex));
return -1;
}
Я чувствую, что проблема связана с блокировкой битовой маски, когда я ее читаю. Нужно ли иметь какой-нибудь мьютекс или блокировать проверку if?
Похоже, что каждая машина сначала проверяет наличие насоса № 0, а если этот насос занят, то проверяет насос № 1 и так далее. Учитывая это, мне кажется, ожидалось, что насос № 0 будет обслуживать большинство автомобилей, а затем насос № 1, обслуживающий вторые по величине автомобили, вплоть до насоса № (pumpInStation-1), который используется только в ( относительно редкая) ситуация, когда все насосы используются одновременно во время подъёма нового автомобиля.
Если вы хотите улучшить балансировку нагрузки, вам, вероятно, следует выбрать для каждой машины свой произвольный случайный порядок для перебора насосов вместо того, чтобы все они проверяли доступность насосов в одном и том же порядке.
Обычно я бы не предлагал рефакторинг, так как это довольно грубо и не дает прямой ответ, но здесь я думаю, что это поможет вам немного разбить вашу логику на три части, например, чтобы лучше показать, где спор лежит :
int Station::acquirePump()
{
// loop through the pumps using the bitmask to check if they are available
ScopedLocker locker(&stationMutex);
for (int i = 0; i < pumpsInStation; i++)
{
// Check bitmask to see if pump is open
if ((freeMask & (1 << i)) == 0 )
{
//Turning the bit on
freeMask |= (1 << i);
return i;
}
}
return -1;
}
void Station::releasePump(int n)
{
ScopedLocker locker(&stationMutex);
freeMask &= ~(1 << n);
stationCondition->notify_one();
}
bool Station::fillUp()
{
// If a pump is available:
int i = acquirePump();
if (i != -1)
{
// Sleeps thread for 30ms and increments counts
pumps[i].fillTankUp();
releasePump(i)
// Sleep long enough for all cars to have a chance to fill up first.
this_thread::sleep_for(std::chrono::milliseconds((((carsInStation-1) * 30) / pumpsInStation)-30));
return true;
}
// If no pumps are available, wait until one becomes available.
stationCondition->wait(std::unique_lock<std::mutex>(*stationMutex));
return false;
}
Теперь, когда у вас есть код в этой форме, возникает проблема с балансировкой нагрузки, которую важно исправить, если вы не хотите «выхлопывать» один насос или если он тоже может иметь блокировку внутри. Вопрос заключается в acquirePump
где вы проверяете наличие свободных насосов в одинаковом порядке для каждого автомобиля. Простой твик, который вы можете сделать, чтобы сбалансировать его лучше, выглядит так:
int Station::acquirePump()
{
// loop through the pumps using the bitmask to check if they are available
ScopedLocker locker(&stationMutex);
for (int n = 0, i = startIndex; n < pumpsInStation; ++n, i = (i+1) % pumpsInStation)
{
// Check bitmask to see if pump is open
if ((freeMask & (1 << i)) == 0 )
{
// Change the starting index used to search for a free pump for
// the next car.
startIndex = (startIndex+1) % pumpsInStation;
// Turning the bit on
freeMask |= (1 << i);
return i;
}
}
return -1;
}
Еще я должен спросить, действительно ли необходимо (например, для повышения эффективности памяти) использовать битовые флаги, чтобы указать, используется ли насос. Если вы можете использовать массив bool
вместо этого вы сможете избежать полной блокировки и просто использовать атомарные операции для приобретения и выпуска насосов, и это позволит избежать пробок из заблокированных потоков.
Представьте, что с мьютексом связана очередь, содержащая ожидающие потоки. Теперь одному из ваших потоков удается получить мьютекс, который защищает битовую маску занятых станций, и проверяет, свободно ли одно конкретное место. Если это не так, он снова освобождает мьютекс и зацикливается, только чтобы вернуться в конец очереди потоков, ожидающих мьютекс. Во-первых, это несправедливо, потому что первый ожидающий не гарантированно получит следующий свободный слот, только если этот слот окажется тем, который находится на его счетчике цикла. Во-вторых, это вызывает огромное количество переключений контекста, что негативно сказывается на производительности. Обратите внимание, что ваш подход все же должен давать правильные результаты, так как при доступе к одной заправочной станции не сталкиваются две машины, но поведение неоптимально.
Вместо этого вы должны сделать следующее:
На тот случай, если эта часть вам не ясна, ожидание переменной условия неявно освобождает мьютекс во время ожидания и повторно запрашивает его!