-1

I am trying to find a fast way to speed the following for loops up. (just fun theory stuff)

I have the following program

using System;
using System.Collections;
using System.Diagnostics;

namespace SpeedTest
{
    class Program
    {
        static void Main(string[] args)
        {
            Stopwatch timer = new Stopwatch();
            timer.Start();

            ArrayList widgets = new ArrayList(50000);

            for (int i = 0; i < widgets.Capacity; i++)
            {
                widgets.Add(new Widget(i));
            }

            string ProcessingMessages = "";

            for (int i = 0; i < widgets.Count; i++)
            {
                Widget widgetToProcess = (Widget)widgets[i];
                ProcessingMessages += widgetToProcess.ProcessWidget();
                ProcessingMessages += "\r\n";
            }

            Console.WriteLine(ProcessingMessages);

            timer.Stop();

            Console.WriteLine(timer.Elapsed);
            Console.ReadLine();
        }
    }

    class Widget
    {
        public Widget()
        {
            this.CreatedDateTime = DateTime.Now;
        }

        public Widget(int ID) : this()
        {
            this.ID = ID;
        }

        public DateTime CreatedDateTime { get; set; }
        public int ID { get; set; }

        public string ProcessWidget()
        {
            int Z = this.ID + this.ID;
            string Message = "Widget ID " + this.ID;
            Message += " was created on ";
            Message += CreatedDateTime.Year + "-" + CreatedDateTime.Month + "-" + CreatedDateTime.Day + ", at ";
            Message += CreatedDateTime.Hour + ":" + CreatedDateTime.Minute + ", with a value of ";
            Message += Z.ToString();
            return Message;
        }
    }
}

How can I speed these for loops up within itself? I know that implementing parallelism would be useful, such as

for (int i = 0; i < widgets.Count; i += 10)
            {
                Widget widgetToProcess1 = (Widget)widgets[i];
                Widget widgetToProcess2 = (Widget)widgets[i+1];
                Widget widgetToProcess3 = (Widget)widgets[i+2];
                Widget widgetToProcess4 = (Widget)widgets[i+3];
                Widget widgetToProcess5 = (Widget)widgets[i+4];
                Widget widgetToProcess6 = (Widget)widgets[i + 5];
                Widget widgetToProcess7 = (Widget)widgets[i + 6];
                Widget widgetToProcess8 = (Widget)widgets[i + 7];
                Widget widgetToProcess9 = (Widget)widgets[i + 8];
                Widget widgetToProcess10 = (Widget)widgets[i + 9];
                ProcessingMessages += widgetToProcess1.ProcessWidget() + "\r\n" +
                    widgetToProcess2.ProcessWidget() + "\r\n" +
                    widgetToProcess3.ProcessWidget() + "\r\n" +
                    widgetToProcess4.ProcessWidget() + "\r\n" +
                    widgetToProcess5.ProcessWidget() + "\r\n" +
                    widgetToProcess6.ProcessWidget() + "\r\n" +
                    widgetToProcess7.ProcessWidget() + "\r\n" +
                    widgetToProcess8.ProcessWidget() + "\r\n" +
                    widgetToProcess9.ProcessWidget() + "\r\n" +
                    widgetToProcess10.ProcessWidget() + "\r\n";
             }

However that is all manually written. I am trying to find a way to dynamically break apart a large task/array size and run those chunks faster.

Looking for some simple thoughts/concepts on this. Or, if there are better alternatives than using a for loop for this test.

Austin
  • 3,010
  • 23
  • 62
  • 97
  • 1
    A couple of comments: The loop itself will not be the bottleneck. The work you're doing inside the loop might be. You're using dynamic string concat which produce a bunch of overhead. Additionally, you're using `ArrayList` which is not the best approach these days. C# has generic so use them. – Brian Rasmussen Aug 01 '14 at 16:00
  • @Brian is right, right, right. Please learn [*this absurdly simple technique*](http://stackoverflow.com/a/2474118/23771), and you will very quickly become a performance guru. – Mike Dunlavey Aug 01 '14 at 16:04

3 Answers3

4

Well for starters you are doing string concatenation inside a tight loop - bad, look into StringBuilder.

Secondly, you're including the set up your test scenario in your timing.

And thirdly, it's not clear what the relative cost is between the processing of the for loop and the cost of the ProcessWidget() method. Surely any optimisation would take place inside this method?

Having scrolled down to look at the ProcessWidget() method you would also benefit from removing all string concatenation and replacing either with one big string.Format() call or using StringBuilder. Also, ID and CreatedDateTime look like immutable fields, maybe it would be worth defining them as readonly and creating the string description up front in the constructor (if it ProcessWidget() might be called multiple times).

Slugart
  • 4,535
  • 24
  • 32
2

You might also consider using:

widgets.Cast<Widget>().AsParallel().ForAll(widgetToProcess => {...});

This will improve the performance of your process, as is, from over 4 minutes to under 30 seconds (depending on CPU configuration).

Rob Epstein
  • 1,450
  • 9
  • 11
0

You can improve the overall speed by, using StringBuilder and moving your console output into the ProcessWidget call, then putting the array population and processing into a Parallel.For The total running time is ~7 sec

Here's the modified code:

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Text;
using System.Threading.Tasks;

using System.Collections;
using System.Diagnostics;

namespace parallelFor
{
    class Program
    {
        static void Main(string[] args)
        {
            Stopwatch timer = new Stopwatch();
            timer.Start();
            int size = 50000;
            ArrayList widgets = new ArrayList(size);

            Parallel.For(0, size, i =>
            {
                Widget w = new Widget((int)i);
                widgets.Add(w);
                w.ProcessWidget();
            });


            timer.Stop();

            Console.WriteLine(timer.Elapsed);
            Console.ReadLine();
        }
    }

    class Widget
    {
        public Widget()
        {
            this.CreatedDateTime = DateTime.Now;
        }

        public Widget(int ID)
            : this()
        {
            this.ID = ID;
        }

        public DateTime CreatedDateTime { get; set; }
        public int ID { get; set; }

        public void ProcessWidget()
        {
        int Z = this.ID + this.ID;
        string Message = "Widget ID " + this.ID;
        Message += " was created on ";
        Message += CreatedDateTime.Year + "-" + CreatedDateTime.Month + "-" + CreatedDateTime.Day + ", at ";
        Message += CreatedDateTime.Hour + ":" + CreatedDateTime.Minute + ", with a value of ";
        Message += Z.ToString();
        Console.WriteLine(Message);
        }
    }
}
Chad Dienhart
  • 5,024
  • 3
  • 23
  • 30
  • Downvote: StringBuilder is not faster for shorter strings. If you take out the Console.WriteLine and increase iterations to 2,000,000, then direct string concatenation is 15% faster for the short strings. – Loathing Aug 01 '14 at 19:13
  • @Loathing Yes when only concatenating a small number of strings it is faster to use + instead of incurring the StringBuilder object creation cost. Given that there are ~10 + in the string concatenation I jumped on using StringBuilder. I ran the analysis to verify that concatenation is faster in this case and have updated my answer to reflect your feedback. Thanks for pointing that out. – Chad Dienhart Aug 02 '14 at 01:31
  • You have removed functionality from his example, the results of all the widgets are no longer combined. – Slugart Aug 02 '14 at 07:20