🚀 go-pugleaf

On Monday, January 29, 2018 at 1:22:58 PM UTC+1, Borneq wrote:
> Mam gist:
> https://gist.github.com/borneq/f63cf132abef474dc9e25491d3115351
> void FinderForBlock::execute(vector<InputTx> *inputVec, vector<OutputTx> 
> *outputVec)
> {
>      FinderForTx finder;
>      for (size_t i = 0; i<inputVec->size(); i++)
>      {
>          outputVec->at(i) = finder.findPred(inputVec->at(i));
>      }
> }
> 
> vector<thread> vec_thr;
> for (int i=0; i<nThreads; i++)
>      vec_thr.emplace_back(&execute, &inputVec, &outputVec);
> for (auto& t : vec_thr)
>      t.join()
> 
> (pytałem się na 
> https://social.msdn.microsoft.com/Forums/en-US/37c1cf02-6a54-47d3-ba1c-a631239f81e8/how-to-share-thread-data?forum=vcgeneral 
> ale nie chcę TLS)
> 
> Chodzi o to, że execute dostaje jako parametry dwa wektory a ma jeszcze 
> dostać indeks wspólny i muteks, który ma nie być globalny.
> Kiedyś robiłem to tak, że definiowałem strukturę z muteksem, indeksem i 
> innymi rzeczami i dawałem wskaźnik na nią do wszystkich wątków:
> void FinderForBlock::execute(Structure *structure)
> {
>      FinderForTx finder;
>      while(true)
>      {
>        structure->mutex.lock();
>         if (structure->index>=structure->vecSizes)
>        {
>          structure->mutex.unlock();
>          break;
>        }
>        structure->index++;
>        structure->mutex.unlock();
>        outputVec->at(structure->index-1) = 
> finder.findPred(inputVec->at(structure->index-1));
>      }
> }
> 
> Czy sposób ze strukturą jest dobry? A zamiast niej chciałbym użyć klasy 
> FindeForBlock - jak?

Po pierwsze masz poważny błąd FinderForBlock::execute. W obrębie
mutexu zwiększasz index, ale po wyjściu z blokady czytasz to pole
i to dwa razy. Po mutex.unlock() inne wątki mogą zmienić index.

Poprawnie byłoby:

    structure->mutex.lock();
    if (structure->index>=structure->vecSizes)
    {
      structure->mutex.unlock();
      break;
    }
    const auto index = structure->index++;
    structure->mutex.unlock();
    outputVec->at(index) = finder.findPred(inputVec->at(index))

Ale ogólnie to nie potrzebujesz mutexa. Wątki mają działać na rozłącznych
zakresach wektorów, wiesz na ilu wątkach to robisz (masz nThreads, nie).
Wyznacz sobie zakresy dla każdego wątku i tyle. Wtedy metoda execute miałaby
postać:

void FinderForBlock::execute(vector<InputTx> *inputVec, vector<OutputTx> 
*outputVec, size_t start, size_t end)
{
     FinderForTx finder;
     for (size_t i = start; i < end; i++)
     {
         outputVec->at(i) = finder.findPred(inputVec->at(i));
     }
}

Tylko wyznaczanie zakresów wymaga trochę uwagi, bo jesteś w dziedzinie
liczb całkowitych.

w.

W dniu 29.01.2018 o 18:57, Wojciech Muła pisze:
> Po pierwsze masz poważny błąd FinderForBlock::execute. W obrębie
> mutexu zwiększasz index, ale po wyjściu z blokady czytasz to pole
> i to dwa razy. Po mutex.unlock() inne wątki mogą zmienić index.

Nie zwróciłem uwagi, chyba w oryginale mam przepisanie do zmiennej 
lokalnej i nie ++ ale robię +=5, ale ważniejsze następne:

> Ale ogólnie to nie potrzebujesz mutexa. Wątki mają działać na rozłącznych
> zakresach wektorów, wiesz na ilu wątkach to robisz (masz nThreads, nie).
> Wyznacz sobie zakresy dla każdego wątku i tyle. Wtedy metoda execute miałaby
> postać:

Dzięki za dobry pomysł!, to upraszcza sprawę. Muteksy mogłyby być 
użyteczne gdybyśmy nie mogli oszacować czy jeden wątek będzie dużo 
różnił się w czasie trwania od innych, ale średnio transakcje są dość 
równego rozmiary, gdy jedna większa to sto innych ją wyrówna.