current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 4 down vote favorite

I have both the regular for each loop and its equivalent paralleled version. The loop simply iterates through a collection, looking for the rate key in a local db instance and then updating the topic code.

using (var db = new SomeEntities())
{
    foreach (var measure in measures)
    {
        var keytofind= measure.RateKey.ToString(CultureInfo.InvariantCulture);
        var firstOrDefault = db.RatesMappings.FirstOrDefault(r => r.RateKey == keytofind);
        if (firstOrDefault != null)
            measure.TopicCode = firstOrDefault.TopicCode;
    }

    Parallel.ForEach(measures, measure=>
    {
        var keytofind= rawDataMeasure.RateKey.ToString(CultureInfo.InvariantCulture);

        lock(measure)
        {
            var firstOrDefault = db.RatesMappings.FirstOrDefault(r => r.RateKey == keytofind);
            if (firstOrDefault != null)
                measure.TopicCode = firstOrDefault.TopicCode;
        }
    });
}

Is the paralleled code correct and has the lock been placed correctly?

share|improve this question
up vote 6 down vote accepted

No, when you use the lock like that, it's completely useless. Each lock would get its own identifier, so none of he locks would keep any other thread out. To make the lock work at all, you need to use the same identifier for all locks. Demonstration:

object sync = new Object();
var keytofind = rawDataMeasure.RateKey.ToString(CultureInfo.InvariantCulture);
Parallel.ForEach(measures, measure => {
  lock(sync) {
    var firstOrDefault = db.RatesMappings.FirstOrDefault(r => r.RateKey == keytofind);
    if (firstOrDefault != null) {
      measure.TopicCode = firstOrDefault.TopicCode;
    }
  }
});

However, if you need a lock around the code in a parallel loop, then the parallel loop is pointless. The reason that you use the parallel loop is that several threads can run at once, and the lock would prevent that.

share|improve this answer
    
so, do I need the lock to begin with, or should I just put the lock around the measure.TopicCode = firstOrDefault.TopicCode? – user38158 May 25 '14 at 9:26
    
I researched the following article link, and it seems each thread should get its own copy of measure so a lock is not required to begin with – user38158 May 25 '14 at 9:49
1  
If you only need the lock to protect each measure item to be used from more than one thread, then you don't need the lock at all. Each thread will have its own item. Unless of course you have multiple references to the same object in measures. – Guffa May 25 '14 at 9:52

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.