Home > Software design >  Using Only One ForEach Loop
Using Only One ForEach Loop

Time:02-03

Please see the following Code:-

 Private Sub PictureBox1_Paint(sender As Object, e As PaintEventArgs) Handles PictureBox1.Paint
           Parallel.ForEach(Segments.OfType(Of Segment),
                             Sub(segment)
                                 Try
                                      segment.DrawLine(e.Graphics)
                                 Catch ex As Exception
                                 End Try
                             End Sub
                             )
        
          Parallel.ForEach(Ellipses.OfType(Of Ellipse),
                             Sub(ellipse)
                                 Try
                                      ellipse.DrawEllipse(e.Graphics)
                                 Catch ex As Exception
                                 End Try
                             End Sub
                             )
      End Sub

Is it Possible to use only One ForEach loop instead of Two as shown above? If No, is there a way to Enhance the Speed of Execution by using async and await?

CodePudding user response:

It would be possible to use only one loop if all types of shapes would be in the same collection. To enable this, you would need to have an abstract base class for shapes.

Public MustInherit Class Shape
    Public Property ForeColor As Color

    ... other common properties

    Public MustOverride Sub Draw(g As Graphics)
End Class

Then you can derive the concrete shapes like this (with Line as an example):

Public Class Line
    Inherits Shape

    Public Property StartPoint As PointF
    Public Property EndPoint As PointF

    Public Overrides Sub Draw(g As Graphics)
        Using pen As New Pen(ForeColor)
            g.DrawLine(pen, StartPoint, EndPoint)
        End Using
    End Sub
End Class

The you can add all the kinds of shapes to a List(Of Shape) and loop it like this

Parallel.ForEach(Shapes,
    Sub(shape)
        shape.Draw(e.Graphics)
    End Sub
)

The Draw methods should not throw exceptions. They would, e.g., if the pen was Nothing. But this would be a programming error that has to be fixed and not something that you should be caught at runtime. So, remove the Try-Catch.

CodePudding user response:

Here is a complete solution, using the same idea as @OlivierJacot-Descombes answer. Plus some additional inherited functionality. This solution means to test the speed differences between the methods discussed in the question and existing answer.

First, the classes

Public MustInherit Class Shape
    Public Property Name As String
    Public Property ForeColor As Color
    Public Sub Draw(g As Graphics)
        drawShape(g)
    End Sub
    Public Sub DrawSyncLock(g As Graphics)
        SyncLock g
            drawShape(g)
        End SyncLock
    End Sub
    Protected MustOverride Sub drawShape(g As Graphics)
    Public Sub New(foreColor As Color)
        Me.ForeColor = foreColor
    End Sub
End Class

Public Class Line
    Inherits Shape
    Public Sub New(foreColor As Color, startPoint As PointF, endPoint As PointF)
        MyBase.New(foreColor)
        Me.StartPoint = startPoint
        Me.EndPoint = endPoint
    End Sub
    Public Property StartPoint As PointF
    Public Property EndPoint As PointF
    Protected Overrides Sub Drawshape(g As Graphics)
        Using pen As New Drawing.Pen(ForeColor)
            g.DrawLine(pen, StartPoint, EndPoint)
        End Using
    End Sub
End Class

Public Class Ellipse
    Inherits Shape
    Public Sub New(foreColor As Color, rect As RectangleF)
        MyBase.New(foreColor)
        Me.Rect = rect
    End Sub
    Public Property Rect As RectangleF
    Protected Overrides Sub Drawshape(g As Graphics)
        Using pen As New Drawing.Pen(ForeColor)
            g.DrawEllipse(pen, Rect)
        End Using
    End Sub
End Class

The methods in the abstract class, Draw and DrawSyncLock only need to be one method, but they both exist for testing purposes.

To set up, I declare a list of shape and populate it with 10000 Lines and 10000 Ellipses. And set up the bitmap in PictureBox1

Public Class Form1

    Private shapes As New List(Of Shape)()

    Private Sub Form1_Load(sender As Object, e As EventArgs) Handles MyBase.Load
        PictureBox1.SizeMode = PictureBoxSizeMode.StretchImage
        PictureBox1.Dock = DockStyle.Fill

        For i As Integer = 0 To 99
            For j As Integer = 0 To 99
                shapes.Add(New Line(Color.White, New PointF(i   j, i   j), New PointF(j   10   4, j   10   4)))
            Next
        Next
        For i As Integer = 0 To 99
            For j As Integer = 0 To 99
                shapes.Add(New Ellipse(Color.White, New RectangleF(i   j, i   j, 10, 10)))
            Next
        Next
        PictureBox1.Image = New Bitmap(500, 500)
        Using g = Graphics.FromImage(PictureBox1.Image)
            Dim r As New Rectangle(0, 0, 500, 500)
            g.FillRectangle(New SolidBrush(Color.Black), r)
        End Using
    End Sub

Then I ran three different methods inside the Paint handler, with a stopwatch to time them

Private Sub PictureBox1_Paint(sender As Object, e As PaintEventArgs) Handles PictureBox1.Paint
    Dim sw As New Stopwatch()

    sw.Restart()
    Parallel.ForEach(shapes,
                        Sub(s)
                            Try
                                s.Draw(e.Graphics)
                            Catch
                            End Try
                        End Sub)
    sw.Stop()
    Console.WriteLine($"PForEachTry: {sw.ElapsedMilliseconds}")

    sw.Restart()
    For Each s In shapes
        s.Draw(e.Graphics)
    Next
    sw.Stop()
    Console.WriteLine($"ForEach: {sw.ElapsedMilliseconds}")

    sw.Restart()
    Parallel.ForEach(shapes, Sub(s) s.DrawSyncLock(e.Graphics))
    sw.Stop()
    Console.WriteLine($"PForEachSync: {sw.ElapsedMilliseconds}")

End Sub

What I found is that the Parallel.ForEach with a Try... empty Catch is extremely slow, not even in the same ballpark as the other two methods. About 10x! By the way, this was your original method, so no wonder you were trying to speed things up. Here is one sample:

PForEachTry: 1188
ForEach: 149
PForEachSync: 177

It's not even worth continuing to test that method, so I'll remove it and test just the other two. Here are the results, generated by resizing the form:

sw.Restart()
For Each s In shapes
    s.Draw(e.Graphics)
Next
sw.Stop()
Console.WriteLine($"ForEach: {sw.ElapsedMilliseconds}")

sw.Restart()
Parallel.ForEach(shapes, Sub(s) s.DrawSyncLock(e.Graphics))
sw.Stop()
Console.WriteLine($"PForEachSync: {sw.ElapsedMilliseconds}")

ForEach: 68
PForEachSync: 229
ForEach: 75
PForEachSync: 121
ForEach: 89
PForEachSync: 139
ForEach: 79
PForEachSync: 140
ForEach: 74
PForEachSync: 140
ForEach: 83
PForEachSync: 138
ForEach: 75
PForEachSync: 137
ForEach: 79
PForEachSync: 124
ForEach: 128
PForEachSync: 164
ForEach: 63
PForEachSync: 127

In case the order in which they are called matters, I reversed the order, and checked again:

sw.Restart()
Parallel.ForEach(shapes, Sub(s) s.DrawSyncLock(e.Graphics))
sw.Stop()
Console.WriteLine($"PForEachSync: {sw.ElapsedMilliseconds}")

sw.Restart()
For Each s In shapes
    s.Draw(e.Graphics)
Next
sw.Stop()
Console.WriteLine($"ForEach: {sw.ElapsedMilliseconds}")

PForEachSync: 303
ForEach: 189
PForEachSync: 149
ForEach: 89
PForEachSync: 241
ForEach: 79
PForEachSync: 138
ForEach: 86
PForEachSync: 140
ForEach: 77
PForEachSync: 146
ForEach: 75
PForEachSync: 237
ForEach: 159
PForEachSync: 143
ForEach: 97
PForEachSync: 128
ForEach: 69
PForEachSync: 141
ForEach: 86

Order doesn't matter. It's always slower (with 20000 shapes...)

The fact that you have settled on a Parallel.ForEach and SyncLock - the only thing it does is indicate that you shouldn't be running in parallel. Graphics instance methods are not thread-safe, so they won't benefit from multithreading when they are the only operation being performed in the thread. The additional overhead created when using Parallel.ForEach can be ignored when performing long-running tasks, but numerous short-lived operations are so quick that the overhead of delegating 20000 tasks starts to be counter-active. It makes sense when you have a few long-running tasks.

In short, parallel a single locked operation is code-stink, and it's beneficial to understand why.

  •  Tags:  
  • Related