2013-01-24 2 views
6

Sto provando a implementare il modello Parallel.ForEach e il progresso della traccia, ma mi manca qualcosa per quanto riguarda il blocco. L'esempio seguente conta fino a 1000 quando threadCount = 1, ma non quando threadCount> 1. Qual è il modo corretto per farlo?Come fare Parallel.ForEssere corretto, blocco e reportistica dell'avanzamento

class Program 
{ 
    static void Main() 
    { 
     var progress = new Progress(); 
     var ids = Enumerable.Range(1, 10000); 
     var threadCount = 2; 

     Parallel.ForEach(ids, new ParallelOptions { MaxDegreeOfParallelism = threadCount }, id => { progress.CurrentCount++; }); 

     Console.WriteLine("Threads: {0}, Count: {1}", threadCount, progress.CurrentCount); 
     Console.ReadKey(); 
    } 
} 

internal class Progress 
{ 
    private Object _lock = new Object(); 
    private int _currentCount; 
    public int CurrentCount 
    { 
     get 
     { 
     lock (_lock) 
     { 
      return _currentCount; 
     } 
     } 
     set 
     { 
     lock (_lock) 
     { 
      _currentCount = value; 
     } 
     } 
    } 
} 

risposta

16

Il solito problema con chiamare qualcosa come count++ da più thread (che condividono la variabile count) è che questa sequenza di eventi può accadere:

  1. Discussione Una legge il valore di count.
  2. Il thread B legge il valore di count.
  3. Il thread A incrementa la copia locale.
  4. Il thread B incrementa la copia locale.
  5. Il thread A riporta il valore incrementato a count.
  6. Il thread B scrive il valore incrementale su count.

In questo modo, il valore scritto dal thread A viene sovrascritto dal thread B, quindi il valore viene effettivamente incrementato una sola volta.

Il codice aggiunge blocchi attorno alle operazioni 1, 2 (get) e 5, 6 (set), ma questo non fa nulla per impedire la sequenza problematica di eventi.

Quello che dovete fare è quello di bloccare l'intera operazione, in modo che, mentre filo A è incrementare il valore, filo B non può accedere a tutti:

lock (progressLock) 
{ 
    progress.CurrentCount++; 
} 

Se si sa che solo tu necessario incrementare, è possibile creare un metodo su Progress che lo incapsula.

+0

Grazie, esattamente quello che sta succedendo. Ho aggiunto un metodo extra su Avanzate per aumentare il contatore con il blocco applicato. –

+0

Penso che la variabile progressLock dovrebbe essere "progress"? – eschneider

0

istruzioni lock Togli dalla proprietà e modificare il corpo principale:

object sync = new object(); 
     Parallel.ForEach(ids, new ParallelOptions {MaxDegreeOfParallelism = threadCount}, 
         id => 
          { 
           lock(sync) 
           progress.CurrentCount++; 
          }); 
+0

Puoi spiegare perché? – svick

+0

Penso che l'istruzione di blocco nel corpo del ciclo sia atomica. L'istruzione di blocco nelle proprietà è separata, poiché propery è compilato nel metodo. – chameleon86

+0

Poiché l'operatore di incremento coinvolge sia la chiamata al getter che al setter - x ++ è una scorciatoia per x = x + 1 - quindi il blocco viene eseguito prima che il valore venga letto e rilasciato dopo il suo aggiornamento. Ma penso ancora che un campo sarebbe migliore. :) –

0

Il problema qui è che ++ non è atomica - un thread può leggere e incrementare il valore tra un altro thread lettura del valore e memorizzando il valore incrementale (ora errato). Questo è probabilmente aggravato dal fatto che c'è una proprietà che racchiude il tuo int.

ad es.

Thread 1  Thread 2 
reads 5   . 
.    reads 5 
.    writes 6 
writes 6!  . 

Le serrature in tutto il setter e getter non aiutano questo, come non c'è nulla per fermare i blocchi lock themseves chiamati fuori uso.

In genere, suggerirei di utilizzare Interlocked.Increment, ma non è possibile utilizzarlo con una proprietà.

Invece, è possibile esporre _lock e il blocco lock essere intorno alla chiamata progress.CurrentCount++;.

+0

Penso che esporre il lucchetto sia una cattiva idea, perché significa che chiunque può usarlo, il che può facilmente portare a deadlock. Penso che una soluzione migliore sarebbe avere il blocco direttamente in 'Main()'. – svick

+0

Se hai del codice altrove che sta facendo qualcos'altro con 'int', è più facile per quel codice bloccare lo stesso oggetto piuttosto che provare a condividere un oggetto di blocco separato tra tutto. – Rawling

1

La soluzione più semplice sarebbe in realtà stata quella di sostituire la proprietà con un campo, e

lock { ++progress.CurrentCount; } 

(io personalmente preferisco il look del preincremento sopra il post-incremento, come il "++." Cosa scontri a la mia mente! Ma il postincremento funzionerebbe ovviamente allo stesso modo.)

Ciò avrebbe l'ulteriore vantaggio di ridurre il sovraccarico e la contesa, poiché l'aggiornamento di un campo è più rapido di un metodo che aggiorna un campo.

Naturalmente, incapsulare come proprietà può avere anche vantaggi. IMO, poiché la sintassi di campo e proprietà è identica, l'UNICO vantaggio di utilizzare una proprietà su un campo, quando la proprietà è autoimplementata o equivalente, è quando si dispone di uno scenario in cui si desidera distribuire un assembly senza dover creare e distribuire dipendente assemblee di nuovo. Altrimenti, potresti usare anche campi più veloci! Se si verifica la necessità di verificare un valore o aggiungere un effetto collaterale, è sufficiente convertire il campo in una proprietà e creare nuovamente. Pertanto, in molti casi pratici, non c'è penalità per l'utilizzo di un campo.

Tuttavia, viviamo in un periodo in cui molti team di sviluppo operano in modo dogmatico e utilizzano strumenti come StyleCop per rafforzare il loro dogmatismo. Tali strumenti, a differenza dei programmatori, non sono abbastanza intelligenti da giudicare quando l'uso di un campo è accettabile, quindi invariabilmente la "regola che è abbastanza semplice da consentire a StyleCop di controllare" diventa "campi incapsulati come proprietà", "non utilizzare campi pubblici" et cetera ...

15

Vecchia domanda, ma penso che ci sia una risposta migliore.

È possibile segnalare i progressi utilizzando Interlocked.Increment(ref progress) in questo modo non è necessario preoccuparsi di bloccare l'operazione di scrittura in corso.

0

È meglio memorizzare qualsiasi operazione di database o file system in una variabile di buffer locale anziché bloccarla. il blocco riduce le prestazioni.