2013-01-24 22 views
6

Tôi đang cố triển khai mẫu Parallel.ForEach và theo dõi tiến trình, nhưng tôi thiếu một số thứ liên quan đến khóa. Ví dụ sau tính đến 1000 khi số threadCount = 1, nhưng không phải khi threadCount> 1. Cách chính xác để thực hiện việc này là gì?Cách thực hiện đúng Parallel.ForEach, khóa và báo cáo tiến độ

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

Trả lời

16

Vấn đề thông thường với gọi cái gì đó như count++ từ nhiều chủ đề (mà chia sẻ biến count) là chuỗi sự kiện này có thể xảy ra:

  1. Chủ đề A đọc giá trị của count.
  2. Chủ đề B đọc giá trị của count.
  3. Chủ đề A tăng bản sao cục bộ của nó.
  4. Chủ đề B tăng bản sao cục bộ của nó.
  5. Chủ đề A ghi giá trị gia tăng trở lại count.
  6. Chủ đề B ghi giá trị gia tăng trở lại count.

Bằng cách này, giá trị được viết bởi chuỗi A được ghi đè bởi chuỗi B, do đó giá trị thực sự chỉ tăng một lần.

Mã của bạn thêm khóa xung quanh hoạt động 1, 2 (get) và 5, 6 (set), nhưng điều đó không có gì để ngăn chặn chuỗi sự kiện có vấn đề.

Những gì bạn cần làm là để khóa toàn bộ hoạt động, do đó trong khi chủ đề A được incrementing giá trị, thread B không thể truy cập nó ở tất cả:

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

Nếu bạn biết rằng bạn sẽ chỉ cần tăng dần, bạn có thể tạo phương thức trên Progress để gói gọn điều này.

+0

Cảm ơn, chính xác những gì đang xảy ra. Tôi đã thêm một phương thức bổ sung vào Progress để tăng bộ đếm với khóa được áp dụng. –

+0

Tôi nghĩ rằng progressLock biến nên là "tiến bộ"? – eschneider

0

báo cáo Remove khóa từ tính và sửa đổi cơ thể chính:

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

Bạn có thể giải thích lý do không? – svick

+0

Tôi nghĩ rằng tuyên bố khóa trong cơ thể vòng lặp là nguyên tử. Tuyên bố khóa trong các thuộc tính được tách ra, bởi vì tính thích hợp được biên dịch trong phương thức. – chameleon86

+0

Bởi vì toán tử gia tăng liên quan đến cả lệnh gọi đến getter và setter - x ++ là viết tắt của x = x + 1 - nên khóa được lấy trước khi giá trị được đọc và phát hành sau khi nó được cập nhật. Nhưng tôi vẫn nghĩ rằng một lĩnh vực sẽ tốt hơn. :) –

0

Vấn đề ở đây là ++ không phải là nguyên tử - một thread có thể đọc và tăng giá trị giữa thread khác đọc giá trị và nó lưu trữ giá trị tăng dần (hiện không chính xác). Điều này có thể được kết hợp bởi thực tế có một tài sản gói int của bạn.

ví dụ:

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

Các khóa xung quanh setter và getter không giúp đỡ này, vì không có gì để ngăn chặn sự lock khối themseves được gọi ra khỏi trật tự là.

Thông thường, tôi khuyên bạn nên sử dụng Interlocked.Increment, nhưng bạn không thể sử dụng tính năng này với thuộc tính.

Thay vào đó, bạn có thể phơi bày _lock và có khối lock quanh cuộc gọi progress.CurrentCount++;.

+0

Tôi nghĩ rằng việc lộ khóa là một ý tưởng tồi, bởi vì nó có nghĩa là bất cứ ai cũng có thể sử dụng nó, điều này có thể dễ dàng dẫn đến deadlocks. Tôi nghĩ rằng một giải pháp tốt hơn sẽ là có khóa trực tiếp trong 'Main()'. – svick

+0

Nếu bạn có mã ở nơi khác đang làm điều gì đó khác với 'int', thì mã đó sẽ khóa trên cùng một đối tượng dễ dàng hơn là cố gắng chia sẻ một đối tượng khóa riêng biệt giữa mọi thứ. – Rawling

1

Giải pháp đơn giản nhất sẽ thực sự đã đến thay thế tài sản với một lĩnh vực, và

lock { ++progress.CurrentCount; } 

(Cá nhân tôi thích giao diện của preincrement qua postincrement, là "++." Đụng độ điều trong Tuy nhiên, hậu kỳ dĩ nhiên sẽ hoạt động tương tự.)

Điều này sẽ có lợi ích bổ sung khi giảm chi phí và tranh chấp, vì việc cập nhật trường nhanh hơn gọi phương thức cập nhật trường.

Tất nhiên, việc đóng gói nó làm tài sản cũng có thể có lợi thế. IMO, vì cú pháp trường và thuộc tính giống hệt nhau, lợi thế ONLY của việc sử dụng thuộc tính trên một trường, khi thuộc tính được tự động thực hiện hoặc tương đương, là khi bạn có một kịch bản nơi bạn có thể triển khai một assembly mà không phải xây dựng và triển khai phụ thuộc lắp ráp lại. Nếu không, bạn cũng có thể sử dụng các trường nhanh hơn! Nếu nhu cầu phát sinh để kiểm tra giá trị hoặc thêm hiệu ứng phụ, bạn chỉ cần chuyển trường sang thuộc tính và tạo lại. Vì vậy, trong nhiều trường hợp thực tế, không có hình phạt để sử dụng một lĩnh vực.

Tuy nhiên, chúng tôi đang sống trong một thời gian mà nhiều nhóm phát triển hoạt động một cách giáo điều và sử dụng các công cụ như StyleCop để thực thi chủ nghĩa giáo điều của họ. Các công cụ như vậy, không giống như các lập trình viên, không đủ thông minh để đánh giá khi sử dụng trường là chấp nhận được, do đó, luôn luôn "quy tắc đủ đơn giản để StyleCop kiểm tra" trở thành "đóng gói các trường dưới dạng thuộc tính", "không sử dụng trường công khai" et cetera ...

15

Câu hỏi cũ, nhưng tôi nghĩ có một câu trả lời hay hơn.

Bạn có thể báo cáo tiến trình bằng cách sử dụng Interlocked.Increment(ref progress) theo cách đó bạn không phải lo lắng về việc khóa thao tác ghi để tiến hành.

0

Tốt hơn là lưu trữ bất kỳ cơ sở dữ liệu hoặc hệ thống tệp nào hoạt động trong biến bộ đệm cục bộ thay vì khóa nó. khóa giảm hiệu suất.

Các vấn đề liên quan