0
static void Main(string[] args)
{
    var repo = new StudentRepository();

    List<Student> students = new List<Student>();


    var myLock = new object();
    //version 1-----------------------------------------------
    for (int i = 1; i <= 20; i++)
    {
        var stud = repo.GetStudentById(i);
        students.Add(stud);

    }

    //version 2-----------------------------------------------
    var ids = new int[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20 };
    Parallel.ForEach(ids, id =>
    {
        var stud = repo.GetStudentById(id);
        lock (myLock)
        {
            students.Add(stud);
        }
    });

    //version 3------------------------------------------------ -
    List<Task> tasks = new List<Task>();
    for (int i = 1; i <= 20; i++)
    {
        tasks.Add(Task.Factory.StartNew(() =>
        {
            var stud = repo.GetStudentById(i);
            lock (myLock)
            {
                students.Add(stud);
            }
        }));

    }
    Task.WaitAll(tasks.ToArray());


    students.ForEach(s => Console.WriteLine(s.Id + "---" + s.Name));
    Console.ReadLine();
}

Output With version 1 Output With version 1

Output With version 2
Output With version 2

  1. Is there any difference between version 1 and version 2 (performance wise)?
  2. While version 2 gives proper results version 3 stores null in student list(i.e. students). What wrong I am doing here?
Alexei Levenkov
  • 98,904
  • 14
  • 127
  • 179
  • You need to create a local variable inside the loop or pass the value as an argument to the task – Eldar Mar 19 '20 at 18:35
  • Ah right what @Eldar said. You need to instantiate `i` before you pass it inside there. – SpiritBob Mar 19 '20 at 18:36
  • Please avoid asking multiple unrelated questions in a single post as it makes question "too broad" (making it likely to get closed). "Which is faster" part can only be answered by you (by measuring whatever metrics you care about), also "which way of running operations taking similar time is faster - two operations in parallel or one after another" seem to be a bit silly... So the second one is likely (as you did not provide [MCVE]) is due to closure over loop variable as @Eldar pointed out which alredy have multiple answers - I picked one as duplicate. – Alexei Levenkov Mar 19 '20 at 18:52
  • Can you show the code of the `GetStudentById` method? We can't really tell which version should be faster without knowing what code is running. If the code inside `GetStudentById` is not computationally intensive, or if it's serialized because of a lock or other reason, then the single-thread approach should be faster than the multi-threaded approach because multithreading adds overhead. – Theodor Zoulias Mar 19 '20 at 19:39
  • @AlexeiLevenkov as you pointed out this post is asking multiple unrelated questions, so the reason for closing it should be: "needs more focus". I think it's wrong to close it as a duplicate of a random of the multiple questions asked. – Theodor Zoulias Mar 19 '20 at 19:40
  • @TheodorZoulias someone is likely already editing the post to focus it on non-duplicate part (and make it on-topic with single question) as it has re-open vote. I see no problem to re-open it after the edit is done. If you feel that providing detailed answer to OP via duplicate is worse than closing as too broad please bring it up on meta. – Alexei Levenkov Mar 19 '20 at 19:47
  • @AlexeiLevenkov the reopen vote is mine. I have never participated on meta. I would appreciate if you could provide a link to a meta discussion about the reasoning of closing questions in general. – Theodor Zoulias Mar 19 '20 at 19:53
  • @TheodorZoulias one generally asks new question on meta to discuss something. Rules are similar to main - some research expected (i.e. https://meta.stackoverflow.com/search?tab=newest&q=reopen to see what question asked around re-opening). If you decide to ask make sure to decide if you are looking for discussion on "tag:specific-question" or about general policy (either is fine just to shape answers/duplicates)... Generally answer to "reopen because I think it should be closed for another reason" is "don't unless question should be re-opened and stay open"... – Alexei Levenkov Mar 19 '20 at 21:19
  • @AlexeiLevenkov here is the exact question I would like to ask in meta: [Should I vote to reopen questions that are closed for the wrong reason?](https://meta.stackoverflow.com/questions/364899/should-i-vote-to-reopen-questions-that-are-closed-for-the-wrong-reason) Ironically this question has been closed as a duplicate of a [related but different question](https://meta.stackoverflow.com/questions/262211/should-i-vote-to-reopen-when-i-believe-the-reason-for-closing-no-longer-applies), containing a single confusing answer. – Theodor Zoulias Mar 19 '20 at 23:57
  • @Eldar @SpiritBob It worked after creating a local variable,Thanks,previously `i` was evaluated 21 for all the Tasks. @TheodorZoulias `GetStudentById()`'s internal implementation is `return new StudentEntities().Students.Where(s => s.Id == id ).FirstOrDefault()` where `StudentEntities` is derived from `DBContext` class(EntityFramework). I know this is not the right way of doing but question here is performance between single threaded and multi threaded with DBContext class.@AlexeiLevenkov will take care of the points while posting questions. – Satya Narayan Patel Mar 20 '20 at 08:27
  • Since the `GetStudentById` hits the database, it's not trivial, so parallelizing the requests may bring enough performance benefits to overcome the overhead of parallelism. But you should really measure it if you want to know for sure. The `Parallel.ForEach` is intended mainly for CPU-bould operations (calculations/computations), while hitting a database is considered an I/O operation. So much depends on the capabilities and the configuration of your database, and the underlying physical storage that is used by your database. – Theodor Zoulias Mar 20 '20 at 22:22

0 Answers0