-1

I'm trying to add multiple values to a ConcurrentBag, but no values actually get inside. At first I tried using List, but that apparently isn't "Thread-Safe", so I searched around and it seems like people suggest using ConcurrentBag. I tried using Thread.Sleep(100) with List and that worked, but it's slower. How can I properly add values? The debuger always shows "Count:0". Here's my code:

 foreach (KeyValuePair<string, string> entry in test_Words)
            {
                Form1.fr.progressBar1.Value++;
                new Thread(delegate () {
                    switch (test_Type)
                    {
                        case "Definitions":
                            bagOfExercises.Add(Read(Definitions.get(entry.Value, entry.Key)));
                            break;
                        case "Examples":
                            bagOfExercises.Add(Read(Examples.get(entry.Value, entry.Key)).Replace(entry.Key, new string('_', entry.Key.Length)));
                            break;
                    }
                }).Start();           
            }
  • What is the value of `test_Type`? If you put breakpoints on the `bagOfExercises.Add` lines do they get hit? If you put breakpoints on the `break;` lines do they get hit? – mjwills Mar 02 '18 at 02:31
  • what's the question? – Kevin Gosse Mar 02 '18 at 07:16
  • Kevin, I'm asking how do I properly add values to the bag. test_Type is a string. No values get added to the bag when debugging, it just stays at 0. – Simo Aleksandrov Mar 02 '18 at 10:37
  • Possible duplicate of [What is the correct usage of ConcurrentBag?](https://stackoverflow.com/questions/15521584/what-is-the-correct-usage-of-concurrentbag) – orhtej2 Mar 02 '18 at 11:45
  • What does your `Read` method do and why do you need multithreading? – nvoigt Mar 02 '18 at 11:51
  • @nvoigt The Read method just returns a string after formatting it https://hastebin.com/eqorigedad.cs (reads every other line) and multithreading makes the program much faster - it can send multiple API requests at the same time, instead of one by one. To orhtej2 I've seen that thread, I tried Parrarel.ForEach and it's slower than what I currently have. Locking a List also isn't a good option, as that also hurts performance. – Simo Aleksandrov Mar 02 '18 at 11:57
  • 1
    You are starting a thread for each entry... and then you don't wait for any of them to finish. No wonder it's super fast right now and you don't get results. I'd say give `Parallel.ForEach` another try – nvoigt Mar 02 '18 at 12:20
  • @nvoigt why would I have to wait for them to finish? Isn't the point to use them at the same time? It's just that with List you can't fill it from multiple threads, unlike ConcurrentBag, which is thread safe, but It doesn't seem like that is the proper way to add items to it. – Simo Aleksandrov Mar 02 '18 at 12:38
  • 1
    If you don't wait for them to finish... you won't get results. I never said you should wait for each one sequentially, but at one point you will have to wait for *all* of them. If you don't... you don't have results. Again: threading is kinda hard, I'd suggest you give `Parallel.ForEach` another try. Or maybe even `.AsParallel()` from PLinq. That's a lot easier than handling threads yourself. – nvoigt Mar 02 '18 at 12:45

1 Answers1

1

Example for PLinq:

Func<KeyValuePair<string, string>, string> get;

if(test_Type == "Definitions") 
{
    get = kvp => Read(Definitions.get(kvp.Value, kvp.Key));
}
else
{
    get = kvp => Read(Examples.get(kvp.Value, kvp.Key)).Replace(entry.Key, new string('_', kvp.Key.Length)));
}

var results = test_Words.AsParallel()
                        .WithDegreeOfParallelism(test_Words.Count())
                        .Select(get)
                        .ToList();

This tries to use one thread per entry. Normally, PLinq will decide what is the best use of resources, but in this case we know something PLinq cannot know: we wait a lot on external resources, this can be done massively parallel.

nvoigt
  • 75,013
  • 26
  • 93
  • 142