Berapa banyak pekerjaan yang harus saya tempatkan di dalam pernyataan kunci?

Saya adalah pengembang junior yang sedang menulis pembaruan untuk perangkat lunak yang menerima data dari solusi pihak ketiga, menyimpannya dalam database, dan kemudian mengkondisikan data untuk digunakan oleh solusi pihak ketiga lainnya. Perangkat lunak kami berjalan sebagai layanan Windows.

Melihat kode dari versi sebelumnya, saya melihat ini:

        static Object _workerLocker = new object();
        static int _runningWorkers = 0;
        int MaxSimultaneousThreads = 5;

        foreach(int SomeObject in ListOfObjects)
        {
            lock (_workerLocker)
            {
                while (_runningWorkers >= MaxSimultaneousThreads)
                {
                    Monitor.Wait(_workerLocker);
                }
            }

            // check to see if the service has been stopped. If yes, then exit
            if (this.IsRunning() == false)
            {
                break;
            }

            lock (_workerLocker)
            {
                _runningWorkers++;
            }

            ThreadPool.QueueUserWorkItem(SomeMethod, SomeObject);

        }

Logikanya tampak jelas: Tunggu ruang di kumpulan utas, pastikan layanan belum dihentikan, lalu tambahkan penghitung utas dan antrekan pekerjaan. _runningWorkersdi SomeMethod()dalam dalam lockpernyataan yang kemudian memanggil Monitor.Pulse(_workerLocker).

Pertanyaan saya adalah: Apakah ada manfaat dalam mengelompokkan semua kode di dalam satu lock, seperti ini:

        static Object _workerLocker = new object();
        static int _runningWorkers = 0;
        int MaxSimultaneousThreads = 5;

        foreach (int SomeObject in ListOfObjects)
        {
            // Is doing all the work inside a single lock better?
            lock (_workerLocker)
            {
                // wait for room in ThreadPool
                while (_runningWorkers >= MaxSimultaneousThreads) 
                {
                    Monitor.Wait(_workerLocker);
                }
                // check to see if the service has been stopped.
                if (this.IsRunning())
                {
                    ThreadPool.QueueUserWorkItem(SomeMethod, SomeObject);
                    _runningWorkers++;                  
                }
                else
                {
                    break;
                }
            }
        }

Sepertinya, itu mungkin menyebabkan sedikit lebih banyak menunggu utas lainnya, tetapi kemudian sepertinya mengunci berulang kali dalam satu blok logis juga akan agak memakan waktu. Namun, saya baru multi-threading, jadi saya berasumsi bahwa ada masalah lain di sini yang tidak saya sadari.

Satu-satunya tempat lain di mana _workerLockerterkunci adalah SomeMethod(), dan hanya untuk tujuan pengurangan _runningWorkers, dan kemudian di luar foreachuntuk menunggu jumlah _runningWorkersuntuk pergi ke nol sebelum masuk dan kembali.

Terima kasih atas bantuannya.

EDIT 4/8/15

Terima kasih kepada @delnan atas rekomendasi untuk menggunakan semaphore. Kode tersebut menjadi:

        static int MaxSimultaneousThreads = 5;
        static Semaphore WorkerSem = new Semaphore(MaxSimultaneousThreads, MaxSimultaneousThreads);

        foreach (int SomeObject in ListOfObjects)
        {
            // wait for an available thread
            WorkerSem.WaitOne();

            // check if the service has stopped
            if (this.IsRunning())
            {
                ThreadPool.QueueUserWorkItem(SomeMethod, SomeObject);
            }
            else
            {
                break;
            }
        }

WorkerSem.Release()disebut di dalam SomeMethod().

Ini bukan masalah kinerja. Ini adalah pertanyaan tentang kebenaran yang pertama dan terutama. Jika Anda memiliki dua pernyataan kunci, Anda tidak dapat menjamin atomicity untuk operasi yang tersebar di antara mereka, atau sebagian di luar pernyataan kunci. Disesuaikan untuk versi lama kode Anda, ini berarti:

Antara akhir while (_runningWorkers >= MaxSimultaneousThreads)dan _runningWorkers++, apa pun bisa terjadi , karena kode menyerah dan mendapatkan kembali kunci di antaranya. Sebagai contoh, utas A mungkin mendapatkan kunci untuk pertama kalinya, tunggu sampai ada beberapa utas lain keluar, lalu keluar dari lingkaran dan lock. Itu kemudian didahului, dan thread B memasuki gambar, juga menunggu kamar di kolam thread. Karena kata thread lain berhenti, ada adalah ruang sehingga tidak menunggu sangat lama sekali. Baik utas A maupun utas B sekarang berjalan dalam urutan tertentu, masing-masing bertambah _runningWorkersdan memulai pekerjaan mereka.

Sekarang, tidak ada ras data sejauh yang saya bisa lihat, tetapi secara logis itu salah, karena sekarang ada lebih dari MaxSimultaneousThreadspekerja yang berjalan. Pemeriksaan (kadang-kadang) tidak efektif karena tugas mengambil slot di kolam ulir tidak atomik. Ini harus Anda perhatikan lebih dari optimasi kecil di sekitar kunci granularity! (Perhatikan bahwa sebaliknya, mengunci terlalu dini atau terlalu lama dapat dengan mudah menyebabkan kebuntuan.)

Cuplikan kedua memperbaiki masalah ini, sejauh yang saya bisa lihat. Perubahan yang kurang invasif untuk memperbaiki masalah mungkin menempatkan ++_runningWorkerstepat setelah whiletampilan, di dalam pernyataan kunci pertama.

Sekarang, selain benar, bagaimana dengan kinerja? Ini sulit dikatakan. Secara umum, penguncian untuk waktu yang lebih lama ("kasar") menghambat konkurensi, tetapi seperti yang Anda katakan, ini perlu diseimbangkan dengan overhead dari sinkronisasi tambahan penguncian berbutir halus. Umumnya satu-satunya solusi adalah benchmarking dan menyadari bahwa ada lebih banyak pilihan daripada "mengunci semuanya di mana-mana" dan "kunci hanya yang minimum". Ada banyak pola dan primitif konkurensi dan struktur data yang aman. Sebagai contoh, ini sepertinya semaphore aplikasi yang sangat diciptakan, jadi pertimbangkan untuk menggunakan salah satu dari mereka, bukan konter yang dikunci dengan tangan ini.

IMHO Anda mengajukan pertanyaan yang salah - Anda tidak harus terlalu peduli tentang efisiensi trade-off, tetapi lebih banyak tentang kebenaran.

Varian pertama memastikan _runningWorkershanya diakses selama kunci, tetapi merindukan case di mana _runningWorkersmungkin diubah oleh utas lain di celah antara kunci pertama dan yang kedua. Sejujurnya, kode itu terlihat bagi saya jika seseorang telah mengunci secara membuta semua titik akses _runningWorkerstanpa memikirkan implikasi dan potensi kesalahan. Mungkin penulis memiliki ketakutan takhayul tentang mengeksekusi breakpernyataan di dalam lockblok, tetapi siapa yang tahu?

Jadi, Anda seharusnya menggunakan varian kedua, bukan karena lebih atau kurang efisien, tetapi karena (semoga) lebih benar daripada yang pertama.

Jawaban lainnya cukup baik dan jelas membahas masalah kebenaran. Biarkan saya menjawab pertanyaan Anda yang lebih umum:

Berapa banyak pekerjaan yang harus saya tempatkan di dalam pernyataan kunci?

Mari kita mulai dengan saran standar, yang Anda singgung dan singgung di paragraf terakhir dari jawaban yang diterima:

• Lakukan sesedikit mungkin pekerjaan sambil mengunci objek tertentu. Kunci yang ditahan untuk waktu yang lama tunduk pada pertikaian, dan pertikaian lambat. Perhatikan bahwa ini menyiratkan bahwa jumlah total kode dalam kunci tertentu dan jumlah total kode dalam semua pernyataan kunci yang mengunci pada objek yang sama keduanya relevan.

• Memiliki kunci sesedikit mungkin, untuk membuat kemungkinan deadlock (atau livelocks) lebih rendah.

Pembaca yang pandai akan mencatat bahwa ini bertolak belakang. Poin pertama menyarankan memecah kunci besar menjadi banyak kunci yang lebih kecil, berbutir halus untuk menghindari pertikaian. Yang kedua menyarankan untuk menggabungkan kunci-kunci yang berbeda ke dalam objek kunci yang sama untuk menghindari kebuntuan.

Apa yang dapat kita simpulkan dari fakta bahwa saran standar terbaik sepenuhnya bertentangan? Kami benar-benar mendapatkan saran yang bagus:

• Jangan pergi ke sana sejak awal. Jika Anda berbagi memori di antara utas, Anda membuka diri untuk dunia kesakitan.

Saran saya adalah, jika Anda menginginkan konkurensi, gunakan proses sebagai unit konkurensi Anda. Jika Anda tidak dapat menggunakan proses maka gunakan domain aplikasi. Jika Anda tidak dapat menggunakan domain aplikasi, maka utas Anda dikelola oleh Task Parallel Library dan tulis kode Anda dalam hal tugas tingkat tinggi (pekerjaan) daripada utas tingkat rendah (pekerja).

Jika Anda benar-benar harus menggunakan primitif konkurensi tingkat rendah seperti utas atau semafor, maka gunakan konkret untuk membangun abstraksi tingkat tinggi yang menangkap apa yang benar-benar Anda butuhkan. Anda mungkin akan menemukan bahwa abstraksi level yang lebih tinggi adalah sesuatu seperti "melakukan tugas secara asinkron yang dapat dibatalkan oleh pengguna", dan hei, TPL sudah mendukungnya, jadi Anda tidak perlu menjalankan tugas Anda sendiri. Anda mungkin akan menemukan bahwa Anda memerlukan sesuatu seperti inisialisasi malas aman utas; jangan roll sendiri, gunakan Lazy<T>, yang ditulis oleh para ahli. Gunakan koleksi threadsafe (tidak dapat diubah atau sebaliknya) yang ditulis oleh para ahli. Pindahkan level abstraksi ke atas setinggi mungkin.