2013-01-24 63 views
6

मैं Parallel.ForEach पैटर्न को लागू करने और प्रगति को ट्रैक करने की कोशिश कर रहा हूं, लेकिन मुझे लॉकिंग के बारे में कुछ याद आ रहा है। निम्न उदाहरण 1000 पर गिना जाता है जब threadCount = 1, लेकिन जब threadCount> 1. ऐसा करने का सही तरीका क्या है?उचित समानांतर कैसे करें। रिपोर्टिंग, लॉकिंग और प्रगति रिपोर्टिंग

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; 
     } 
     } 
    } 
} 

उत्तर

16

count++ से अधिक थ्रेड (जो साझा count चर) से की तरह कुछ बुला के साथ सामान्य समस्या यह है कि घटनाओं के इस क्रम भी हो सकता है है:

  1. थ्रेड एक count का मूल्य पढ़ता है।
  2. थ्रेड बी count के मान को पढ़ता है।
  3. थ्रेड ए अपनी स्थानीय प्रतिलिपि बढ़ाता है।
  4. थ्रेड बी अपनी स्थानीय प्रतिलिपि बढ़ाता है।
  5. थ्रेड ए बढ़ी हुई मान को count पर वापस लिखता है।
  6. थ्रेड बी बढ़ी हुई मान को count पर वापस लिखता है।

इस तरह, थ्रेड ए द्वारा लिखे गए मान को थ्रेड बी द्वारा अधिलेखित किया जाता है, इसलिए मूल्य वास्तव में केवल एक बार बढ़ता है।

आपका कोड संचालन 1, 2 (get) और 5, 6 (set) के आसपास ताले जोड़ता है, लेकिन यह घटनाओं के समस्याग्रस्त अनुक्रम को रोकने के लिए कुछ भी नहीं करता है।

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

क्या आप जानते हैं कि आप केवल होगा यदि:

आप क्या करने की जरूरत, पूरे ऑपरेशन लॉक करने के लिए इतना है कि, जबकि धागा एक मूल्य incrementing है, धागा यह बिल्कुल तक नहीं पहुँच सकता है बढ़ने की आवश्यकता है, आप Progress पर एक विधि बना सकते हैं जो इसे समाहित करता है।

+0

धन्यवाद, वास्तव में क्या हो रहा है। मैंने लागू लॉकिंग के साथ काउंटर बढ़ाने के लिए प्रगति पर एक अतिरिक्त विधि जोड़ा। –

+0

मुझे लगता है कि परिवर्तनीय प्रगति लॉक "प्रगति" होनी चाहिए? – eschneider

0

गुण से निकालें ताला बयानों और मुख्य शरीर को संशोधित:

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

क्या आप समझा सकते हैं क्यों? – svick

+0

मुझे लगता है कि लूप बॉडी में लॉक स्टेटमेंट परमाणु है। गुणों में लॉक स्टेटमेंट एक अलग है, क्योंकि प्रोपरी विधि में संकलित है। – chameleon86

+0

क्योंकि वृद्धि ऑपरेटर में गेटटर और सेटर दोनों को कॉल शामिल है - x ++ x = x + 1 के लिए शॉर्टेंड है - इसलिए मान को पढ़ने से पहले लॉक लिया जाता है और अपडेट होने के बाद रिलीज़ किया जाता है। लेकिन मुझे अभी भी लगता है कि एक क्षेत्र बेहतर होगा। :) –

0

यहां मुद्दा यह है कि ++ परमाणु नहीं है - एक धागा इसे पढ़ा और मूल्य पढ़ने एक और धागा के बीच मूल्य को बढ़ा सकते हैं और (अब गलत) वृद्धि मूल्य को संग्रहीत करना। यह शायद इस तथ्य से मिश्रित है कि आपकी int लपेटने वाली संपत्ति है।

उदा।

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

सेटर और गेटर चारों ओर ताले यह मदद नहीं करते हैं, वहाँ के रूप में lock ब्लॉक themseves आदेश से बाहर बुलाया जा रहा है को रोकने के लिए कुछ भी नहीं।

आमतौर पर, मैं Interlocked.Increment का उपयोग करने का सुझाव देता हूं, लेकिन आप इसे किसी संपत्ति के साथ उपयोग नहीं कर सकते हैं।

इसके बजाय, आप _lock का पर्दाफाश कर सकते हैं और progress.CurrentCount++; कॉल के आसपास lock ब्लॉक हो सकता है।

+0

मुझे लगता है कि ताला खोलना एक बुरा विचार है, क्योंकि इसका मतलब है कि कोई भी इसका उपयोग कर सकता है, जो आसानी से डेडलॉक्स का कारण बन सकता है। मुझे लगता है कि एक बेहतर समाधान लॉक को सीधे 'मुख्य()' में रखना होगा। – svick

+0

यदि आपके पास कहीं और कोड है जो 'int' के साथ कुछ और कर रहा है, तो उस कोड के लिए एक ही ऑब्जेक्ट को लॉक करना आसान है, बजाय सब कुछ के बीच एक अलग लॉक ऑब्जेक्ट साझा करने का प्रयास करना। – Rawling

1

सबसे आसान समाधान वास्तव में एक क्षेत्र के साथ संपत्ति को बदलने के लिए होता है, और

lock { ++progress.CurrentCount; } 

(मैं व्यक्तिगत रूप से, postincrement से अधिक preincrement की नज़र पसंद करते हैं के रूप में "++।" में बात झड़पें मेरा दिमाग! लेकिन पोस्टिनक्रिकमेंट निश्चित रूप से वही काम करेगा।)

इसमें ओवरहेड और विवाद को कम करने का अतिरिक्त लाभ होगा, क्योंकि एक फ़ील्ड अपडेट करने से एक फ़ील्ड अपडेट करने वाली विधि को कॉल करने से तेज़ होता है।

बेशक, इसे संपत्ति के रूप में encapsulating भी फायदे हो सकता है। आईएमओ, चूंकि क्षेत्र और संपत्ति सिंटैक्स समान है, एक क्षेत्र में संपत्ति का उपयोग करने का एकमात्र लाभ, जब संपत्ति स्वचालित या समकक्ष होती है, तब जब आपके पास ऐसा परिदृश्य होता है जहां आप एक असेंबली को तैनात और तैनात किए बिना एक असेंबली को तैनात करना चाहते हैं असेंबली नया। अन्यथा, आप तेजी से क्षेत्रों का भी उपयोग कर सकते हैं! यदि कोई मूल्य जांचने या साइड इफेक्ट जोड़ने की आवश्यकता उत्पन्न होती है, तो आप बस फ़ील्ड को किसी संपत्ति में परिवर्तित कर देते हैं और फिर से बनाते हैं। इसलिए, कई व्यावहारिक मामलों में, किसी क्षेत्र का उपयोग करने के लिए कोई दंड नहीं है।

हालांकि, हम ऐसे समय में रह रहे हैं जहां कई विकास दल dogmatically संचालित होते हैं, और स्टाइलकॉप जैसे टूल का उपयोग अपने dogmatism को लागू करने के लिए करते हैं। कोडर के विपरीत, ऐसे उपकरण, जो कि किसी क्षेत्र का उपयोग करते समय न्याय करने के लिए पर्याप्त स्मार्ट नहीं होते हैं, इसलिए अनिवार्य रूप से "नियम जो स्टाइलकॉप को जांचने के लिए पर्याप्त सरल है" बनता है "गुणों के रूप में फ़ील्ड को समाहित करता है", "सार्वजनिक फ़ील्ड का उपयोग न करें" et cetera ...

15

पुराना सवाल, लेकिन मुझे लगता है कि एक बेहतर जवाब है।

आप Interlocked.Increment(ref progress) का उपयोग करके प्रगति की रिपोर्ट कर सकते हैं इस तरह आपको लिखने के लिए ऑपरेशन को लॉक करने की चिंता करने की आवश्यकता नहीं है।

0

किसी भी डेटाबेस या फ़ाइल सिस्टम ऑपरेशन को स्थानीय लॉफर वैरिएबल में लॉक करने के बजाय स्टोर करना बेहतर है। लॉकिंग प्रदर्शन कम कर देता है।