Optimize nested for loop

Question

I'm using GMAP.Net library for a mapping windows application. I have about 17000 polygons on my Sql Server database. In form load event I select All polygons from database and fill a datatabale then draw polygons one by one from datatable. I also have a treeview which I add all 17000 polygon names to that treeview. Now when I check select all checkbox on treeview I call a function in Treeview node_AfterCheck event like this:

Private Sub node_AfterCheck(sender As Object, e As TreeViewEventArgs)     Handles TreeView1.AfterCheck

   If e.Action <> TreeViewAction.Unknown Then
      Task.Factory.StartNew(Sub()
                                  GetPolygons(e.Node)
                              End Sub, TaskCreationOptions.LongRunning)
   End If
End Sub



Private Sub GetPolygons(node As TreeNode)
            Dim objectId As String
            Dim _polygon As GMapPolygon
            For Each node1 As TreeNode In node.Nodes
                objectId = node1.Name

                For Each _polygon In polyOverlay.Polygons.AsParallel
                    itemTag = _polygon.Tag.ToString.Split("|")
                    If itemTag (0) = node1.Name Then
                        _polygon.IsVisible = node.Checked
                        Exit For
                    End If
                Next
            Next
End sub

this code takes about 40 seconds to run completely. Is there any way to optimize this code to complete in shorter time?

Also there's some trouble with the order of `End If` and `Exit For`. — rdoubleui, Jan 14 '16 at 10:06

score 1 · Accepted Answer · edited May 23 '17 at 10:27

The one thing that I can see that is expensive codewise is the call of Split on the tags of the polygon. But this would be subject to measure.

To circumvent the Split you could e.g. try to use:

If _polygon.Tag.StartsWith(node1.Name) Then

Consider this question to find out if IsPrefix is even faster in your case.

I'm assuming, however, the main trouble is the constant refresh/redraw of the map ("auto refresh") while setting each polygon's visibility. One thing I found in the sources is to set Invalidation on hold:

// docs: stops immediate markers/route/polygon invalidations; call Refresh to perform single refresh and reset incalidation state
gmap2.HoldInvalidation = true;

// do your update loop within here

// docs: call this to stop HoldInvalidation and perform single forced instant refresh
gmap2.Refresh();

Don't have the the opportunity to give it a try right now, but I guess you can go ahead and try and see if that makes a difference.

score 0 · Answer 2 · answered Jan 14 '16 at 14:11

0

thanks for your quick response I changed my code like this:

Private Sub GetPolygons(node As TreeNode)

        myMap.HoldInvalidation = True

        Dim objectId As String
        Dim _polygon As GMapPolygon
        For Each node1 As TreeNode In node.Nodes
            objectId = node1.Name

            For Each _polygon In polyOverlay.Polygons.AsParallel
                itemTag = _polygon.Tag.ToString.Split("|")
                If itemTag (0) = node1.Name Then
                    _polygon.IsVisible = node.Checked
                    Exit For
                End If
            Next
        Next

        myMap.Refresh()

End sub

but code is taking more period of time to complete than before.

answered Jan 14 '16 at 14:11

Behnam Faghih

164
1
17

by replacing "_polygon.Tag.ToString.Split("|")" with "_polygon.Tag.StartsWith(node1.Name)" now code takes about 20 seconds rather than 40 seconds. – Behnam Faghih Jan 14 '16 at 14:33
1

Ok, it was worth a try. 17000 polygons are quite a bit, glad to see that removal of the `Split` operation helped, still probably even better w/o any operation on each string, think that's possible? How much time does that operation actually take? Find out by leaving out the visibility setting at all for a try to narrow down where the bottle neck is located. Let's see where we get from there, I want to know. – rdoubleui Jan 14 '16 at 15:36
Without operation on each string it takes about 2 seconds to complete! – Behnam Faghih Jan 16 '16 at 12:38
1

Good news, time to refactor that `string` op out of the loop. Now that you're down to actual fractions of seconds that could matter, I'd be interested if `HoldInvalidation` does matter. – rdoubleui Jan 16 '16 at 20:53
1

No HoldInvalidation doesn't matter. but good news! I placed polygonId in polygon.name instead polygon.tag now I just check: "If string.CompareOrdinal(_polygon.Name, node1.Name) = 0 Then _polygon.IsVisible = node.Checked Exit For End If" now it takes about 5 seconds to complete. Now if I can remove that If I think it will be more optimized – Behnam Faghih Jan 17 '16 at 06:58

Optimize nested for loop

2 Answers2