0

I am trying to optimize my code, I want to call Parallel.For inside a Parallel.ForEach(). I am not sure how can I do so. If I do see that the results are not correct. My code reverses the word within a sentence.

changed that for loop and it works.

for (int i = word.Length - 1; i >= 0; i--)

Here is my original code which fails.

    public string Test()
    {
        string s = "Hello how are you";

        if (s.Length > 0)
        {
            StringBuilder reverseS = new StringBuilder(s.Length);
            string[] words = s.Split(' ');

            Parallel.ForEach(words, word =>
            {
                StringBuilder builder = new StringBuilder(word.Length);
                Parallel.For(0, word.Length - 1, i =>
                //for (int i = word.Length - 1; i >= 0; i--)
                {
                    builder.Append(word[i]);
                });
                reverseS.Append(builder);
                reverseS.Append(" ");
            });

            return reverseS.ToString();
        }

        else
        {
            return "";
        }
    }

olleH woh era uoy

Prawn Hongs
  • 441
  • 1
  • 5
  • 17
  • 1
    `StringBuilder` is not thread-safe (this impacts `builder` and `reverseS`). This code is not correct. – mjwills Jan 24 '19 at 00:00
  • Possible duplicate of [Is .NET's StringBuilder thread-safe](https://stackoverflow.com/questions/8831385/is-nets-stringbuilder-thread-safe) – mjwills Jan 24 '19 at 00:02
  • If you are just experimenting, then you can ignore this comment, but in terms of performance this code will perform much worse than simple synchronous code. **Parallelism != better performance** – Yeldar Kurmangaliyev Jan 24 '19 at 00:02
  • It will perform slower, yes. But it also is not reliable (i.e. it won't always work) - which is far worse. – mjwills Jan 24 '19 at 00:03
  • `StringBuilder reverseS = new StringBuilder(s.Length);` You should use `s.Length + words.Length` as the capacity (not `s.Length`). – mjwills Jan 24 '19 at 00:06
  • 1
    By the way, even if it was thread-safe, then your letters would be in random order. – Yeldar Kurmangaliyev Jan 24 '19 at 00:06
  • Why do you want to optimise the code? How fast is it? How fast do you need it to be? – mjwills Jan 24 '19 at 00:13

1 Answers1

0

I see you have to reverse the chars but keep word positions. I don't know how good is the code below in comparison to your solution, so you can have some performance tests.

The idea is to reverse the words in array in the main thread and start a parallel one if we encounter a very long word.

private void Reverse()
{
    const int extremelyLongWordLength = 100000;

    var tasks = new List<Task>();
    var wordStart = 0;
    var arr = "Hello how are you".ToCharArray();
    for (var i = 0; i < arr.Length; i++)
    {
        if (arr[i] == ' ')
        {
            var wordEnd = i - 1;
            if (wordEnd - wordStart >= extremelyLongWordLength)
            {
                tasks.Add(ReverseWordAsTask(arr, wordStart, wordEnd));
            }
            else
            {
                ReverseWord(arr, wordStart, wordEnd);
            }

            wordStart = i + 1;
        }
    }

    if (wordStart != arr.Length - 1)
    {
        if (arr.Length - 1 - wordStart > extremelyLongWordLength)
        {
            tasks.Add(ReverseWordAsTask(arr, wordStart, arr.Length - 1));
        }
        else
        {
            ReverseWord(arr, wordStart, arr.Length - 1);
        }
    }

    Task.WaitAll(tasks.ToArray());

    var modifiedString = new string(arr);
}

private static Task ReverseWordAsTask(char[] arr, int start, int end)
{
    return Task.Run(() =>
    {
        var halfWordIndex = start + (end - start) / 2;
        for (var i = start; i < halfWordIndex; i++)
        {
            var temp = arr[i];
            var opposite = end - (i - start);
            arr[i] = arr[opposite];
            arr[opposite] = temp;
        }
    });
}

private static void ReverseWord(char[] arr, int start, int end)
{
    var halfWordIndex = start + (end - start) / 2;
    for (var i = start; i < halfWordIndex; i++)
    {
        var temp = arr[i];
        var opposite = end - (i - start);
        arr[i] = arr[opposite];
        arr[opposite] = temp;
    }
}
opewix
  • 4,993
  • 1
  • 20
  • 42