Using a ConcurrentBag vs a simple Array in a Parallel for

Question

This is a hypothetical question/use-case based on the benefits of using (in this case) a ConcurrentBag over a simple array from within a Parallel for loop for this particular use-case.

The scenario is based on using a common Pipeline pattern to analyse numbers from 1 to total results, storing the result based on the output from one of the Pipeline operations not being null.

The actual order of the resultant list is important, thus using a simple list(of type string).add would result in oddness based on when each thread decides to return a result.

I have the following working code:

    public IList<string> Execute(int total)
    {
        var items = new ConcurrentBag<AnalyzerResult>();

        Parallel.ForEach(Iterate(1, (total + 1)), d =>
        {
            foreach (IOperation<T> operation in operations)
            {
                var result = operation.Execute(d);
                if (result != null)
                {
                    items.Add(new AnalyzerResult(d, result));
                    break;
                }
            }
        });

        return items.OrderBy(o=>o.SortOrder).Select(d => d.Result).ToList();
    }

AnalyzerResult is a simple immutable class, and the code only ever pushes new items to the bag (so no danger of something in the items list being changed, in theory).

Based on this, would a simple Array be sufficient (and contain less code noise)? or would the use of a concurrent type be considered better practice/more performant? For example:

    public IList<string> Execute(int total)
    {
        var items = new string[total];

        Parallel.ForEach(Iterate(1, (total + 1)), d =>
        {
            foreach (IOperation<T> operation in operations)
            {
                var result = operation.Execute(d);
                if (result != null)
                {
                    items[(d - 1)] = result;
                    break;
                }
            }
        });

        return items.ToList();
    }

Note: This is not a concurrency issue, both methods are legal and produce the desired result without issue.

Answer 1

I initially went with a "you need concurrency protection" answer, but then reread the second part of your question.

This looks like it should work since you're not going to attempt to write to the same location in memory from two different threads. So, eliminating locks and thread affinity (which the ConcurrentBag gives) should provide a significant performance increase.

Really the question is - how much of an increase is it and is it a needed increase (need to profile), and will you ever change this setup in the future such that you would then need concurrency protection.

As is, it should be fine, and quite readable. You would probably want to comment this code as to why you did it this way to make sure someone doesn't casually skim it and think "concurrency issue" (like I just did) and "fix" it.