Loop optimization for image processing

Question

I have this piece of code that is running too slow. I was wondering if anyone can help me optimize it as I can't seem to find any more shortcuts. I'm not sure if using List<> is going to help me but I need complex operation such as Union and Overlap.

Also, List is desirable because I don't know how many unique partitions an Image Region is going to be before running.

u1.length = 13254 which is the number of distinct elements in RevisedListMeanH RevisedListMeanH.Count = 90000

The purpose of the first piece of code is to group together pixels via horizontal comparison and vertical comparison. This runs for about 70 seconds.

The second portion combines both vertical and horizontal pixel blocks into a 2D-block. This section runs for about 120 seconds. My goal is to have both of these loops complete under 10 seconds.

These numbers are from a 300x300 pixel region comparisons of a 4000x3000 image.

watch.Start();            
for (int s = 0; s < u1.Length; s++ )//iterate through uniquelist
{
    List<int> ConnectedBlocksH = new List<int>();
    List<int> ConnectedBlocksV = new List<int>();

    float[] RH = RevisedListMeanH.ToArray();
    float[] RV = RevisedListMeanV.ToArray();

    for (int a = 0; a < RevisedListMeanH.Count; a++)//iterate through bitmap with res
    {
        if (u1[s] == RH[a])
        {
            ConnectedBlocksH.Add(a);//add the index

        }
    }
    for (int a = 0; a < RevisedListMeanV.Count; a++)//iterate through bitmap with res
    {
        if (u1[s] == RV[a])
        {
            ConnectedBlocksV.Add(a);//add the index

        }
    }
    ArrayOfConnectedBlocksH[s] = ConnectedBlocksH;//where the data IS
    ArrayOfConnectedBlocksV[s] = ConnectedBlocksV;//where the data IS
}
watch.Stop();
long asasasdda = watch.ElapsedMilliseconds; //71 sec
watch.Reset();

 watch.Start();
//if intersects then union the two lists
//iterate through both list
List<List<int>> ListOfConnectedBlocks = new List<List<int>>();
for (int a = 0; a < ArrayOfConnectedBlocksH.Length; a++ )
{
    HashSet<int> i = new HashSet<int>(ArrayOfConnectedBlocksH[a]);
    //trigger means it scanned and there was no overlap to add to the group
    while (true)
    {

        bool trigger = true;
        for (int b = 0; b < ArrayOfConnectedBlocksV.Length; b++)
        {
            if (i.Overlaps(ArrayOfConnectedBlocksV[b]))
            {
                i.UnionWith(ArrayOfConnectedBlocksV[b]);//combines all overlaps into one group
                ArrayOfConnectedBlocksV[b].Clear();//merged so just remove
                trigger = false;
            }

        }
        if (trigger)
        {
            break;
        }
        trigger = true;
        for (int c = 0; c < ArrayOfConnectedBlocksH.Length; c++)//now cycle through horizontal
        {
            if (i.Overlaps(ArrayOfConnectedBlocksH[c]))
            {
                i.UnionWith(ArrayOfConnectedBlocksH[c]);//combines all overlaps into one group
                ArrayOfConnectedBlocksH[c].Clear();//merged so just remove
                trigger = false;
            }

        }//first cycle to get T0

        if(trigger)
        {
            break;
        }
    }

    if (i.Count != 0)
    {
        ListOfConnectedBlocks.Add(i.ToList<int>());
    }
}

watch.Stop();
long asasasda = watch.ElapsedMilliseconds;//122 seconds

Answer 1

Some suggestions which might make it faster:

---

Move your ToArray() statements outside the loop so that you only execute them once and then reuse them:

float[] RH = RevisedListMeanH.ToArray();
float[] RV = RevisedListMeanV.ToArray();

for (int s = 0; s < u1.Length; s++ )//iterate through uniquelist
{
    ... etc ...

---

In the second loop, you convert a List to a HashSet:

HashSet<int> i = new HashSet<int>(ArrayOfConnectedBlocksH[a]);

You could avoid that by making ArrayOfConnectedBlocksH[a] i.e. ConnectedBlocksH a HashSet instead of a List to begin with.

---

Might it be any faster if you cached the Length and Count property values instead of calling them repeatedly? For example:

for (int s = 0, int length = u1.Length; s < length; s++ )

---

The basic problem in your first loop is that you are iterating a lot of elements, and doing u1.Length * (RevisedListMeanH.Count + RevisedListMeanV.Count) comparisons.

It might be faster although more complicated to do the following.

Convert your u1 list to a list of index/value pairs:

List<KeyValuePair<int,float>> u1Values = new List<KeyValuePair<int,float>>();
for (int s = 0; s < u1.Length; s++ )
    u1Values.Add(new KeyValuePair<int,float>(s, u1[s]));

Sort your index/value pairs by value:

u1Values.Sort((kvp1, kvp2) => kvp1.Value.CompareTo(kvp2.Value));

Do the same with your RevisedListMeanH and RevisedListMeanV lists.

Now that your all arrays are sorted by value, it is easier/cheaper to find which elements match: you can do it by iterating through all the arrays once; something like:

int u = 0; // index into u1Values
int v = 0; // index into vValues

for (;;)
{
    int i = u1Values[u].Value.CompareTo(vValues[v].Value);
    if (i == 0)
    {
        // matches!
        int s = u1Values[u].Key;
        ArrayOfConnectedBlocksV[s].Add(vValues[v].Key);//add the index
        // which do we increment now: ++u or ++v?
    }
    else if (i > 0)
    {
        // u1Values[u] is too big
        if (++v == u1Values.Count)
            break;
    }
    else
    {
        if (++u == vValues.Count)
            break;
    }
}

Answer 2

First of, I'm gonna give you a basic idea...

Instead of doing 2 loops over two different arrays, check their sizes first. Create a loop for the smaller of both array sizes. In that loop you will process that Array, and then create another loop for the remainder of unprocessed data.

This can be done for your two double fors you have in there. This way you could reduce the complexity from O(n+m) to O(n+(m-n))

The pseudo code is this...

if(Arr1 > Arr2)
{ 
   process(Arr1, Arr2);
}
else if (Arr2 > Arr1)
{
   process(Arr2, Arr1);
}
else //They're the same size!
{
  for(idx = 0; idx != count(any will do); ++idx)
  {
     //process Arr1
     //process Arr2
   }
}

Answer 3

Your mileage may vary, but LINQifying a bit makes it a heck of a lot more readable...

        watch.Start();

        // if intersects then union the two lists
        // iterate through both list
        var listOfConnectedBlocks = new List<List<int>>();

        foreach (var i in ArrayOfConnectedBlocksH.Select(t1 => new HashSet<int>(t1)))
        {
            // trigger means it scanned and there was no overlap to add to the group
            while (true)
            {
                var trigger = true;

                foreach (var t in ArrayOfConnectedBlocksV.Where(i.Overlaps))
                {
                    i.UnionWith(t); // combines all overlaps into one group
                    t.Clear(); // merged so just remove
                    trigger = false;
                }

                if (trigger)
                {
                    break;
                }

                trigger = true;

                // now cycle through horizontal
                foreach (var t in ArrayOfConnectedBlocksH.Where(i.Overlaps))
                {
                    i.UnionWith(t); // combines all overlaps into one group
                    t.Clear(); // merged so just remove
                    trigger = false;
                }

                // first cycle to get T0
                if (trigger)
                {
                    break;
                }
            }

            if (i.Any())
            {
                listOfConnectedBlocks.Add(i.ToList());
            }
        }

        watch.Stop();
        var asasasda = watch.ElapsedMilliseconds; // ? seconds
    }