Issue with array was not long enough

Question

I am calling this function several thousands times per minute to get an average of latency but after running for about 20 or 30 minutes my app crashes.. any idea why is this happening?

Source array was not long enough. Check srcIndex and length, and the array's lower bounds.

Dim iLatency_Average As New List(Of Long)
Public Function Average_Latency(myLatency As Long) As Long

    iLatency_Average.Insert(0, myLatency)

    If iLatency_Average.Count = 25 Then
        Dim allLatency() As Long = iLatency_Average.ToArray()

        Array.Sort(allLatency)

        Dim iAverage As Long
        For i As Integer = 5 To 19
            iAverage += allLatency(i)
        Next

        iLatency_Average.RemoveAt(24)

        Return CLng(iAverage / 15)
    Else
        Return 0
    End If
End Function

Sounds like you may be calling it from multiple threads... `List` isn't thread-safe. — Jon Skeet, Mar 24 '15 at 17:19
yes this is correct, what do you recommend for a thread safe function? — PaulWill, Mar 24 '15 at 17:21
Well you could either use synchronization, or use a thread-safe collection. To be honest, you could probably make this code considerable more efficient anyway - calling `Insert(0, ...)` on each call is a bad start... — Jon Skeet, Mar 24 '15 at 17:23
I guess you could use lock on the List(see http://stackoverflow.com/questions/1362995/properly-locking-a-listt-in-multithreaded-scenarios) object but well, that would make your app a lot slower. As @JonSkeet mentioned, there are many better approaches to solve this and organize the code. — Hozikimaru, Mar 24 '15 at 17:25
Have a look at [`System.Collections.Concurrent.BlockingCollection`](https://msdn.microsoft.com/en-us/library/dd267312%28v=vs.110%29.aspx?f=255&MSPPError=-2147217396) and at the other collections in this namespace. — Olivier Jacot-Descombes, Mar 24 '15 at 17:28
I came up with a version based on BlockingCollection, I posted the answer is that what you guys think its better? — PaulWill, Mar 24 '15 at 17:59

score 2 · Accepted Answer · answered Mar 24 '15 at 21:31

Since it looks like you want to guarantee a sliding window of the last 25 observations, it could become problematic to not use a lock. It's a relatively cheap lock, though.

And as @the_lotus mentioned, it is significantly faster to split the Queue operations from the actual averaging.

See code below which has 10 tasks each adding 100,000 latencies. I'm averaging about 400ms per 1,000,000 operations. An ArraySegment(Of Long) and a LINQ option is included for comparison.

Private RecentLatencies As New Queue(Of Long)

Public Sub Main()
    With New Stopwatch
        .Start()

        Task.WhenAll(Enumerable.Range(1, 10).Select(Function(x) Task.Run(Sub()
                                                                            For m = 1 To 100000
                                                                                LogLatencyAndGetSlidingAverage(m)
                                                                            Next
                                                                         End Sub))).Wait()

        .Stop()

        Console.WriteLine("All done...{0}ms", .ElapsedMilliseconds)
        Console.ReadKey()
    End With
End Sub

Private Function LogLatencyAndGetSlidingAverage(latency As Long) As Long
    Dim l As Long() = Nothing

    SyncLock RecentLatencies
        RecentLatencies.Enqueue(latency)

        If RecentLatencies.Count = 25 Then
            l = RecentLatencies.ToArray()
            RecentLatencies.Dequeue()
        End If
    End SyncLock

    'option 1
    If l IsNot Nothing Then
        Array.Sort(l)

        Dim sum As Long

        For i = 5 To 19
            sum += l(i)
        Next

        Return sum \ 15
    Else
        Return 0
    End If

    'option 2 - ArraySegment
    'If l IsNot Nothing Then
    '    Array.Sort(l)
    '    Return CLng(New ArraySegment(Of Long)(l, 5, 15).Average)
    'Else
    '    Return 0
    'End If

    'option 3 - LINQ
    'Return CLng(If(l IsNot Nothing, l.OrderBy(Function(n) n).Skip(5).Take(15).Average, 0))
End Function

the_lotus · Answer 2 · 2015-03-24T18:29:51.197

You could use a SyncLock. Also, you seem to be using the property of a Queue, not a list. Note that everytime you use the collection you have to call SyncLock.

Dim iLatency_Average As New Queue(Of Long)

Public Function Average_Latency(ByVal myLatency As Long) As Long

    Dim allLatency() As Long = Nothing

    ' Add the new latency
    SyncLock iLatency_Average
        iLatency_Average.Enqueue(myLatency)

        If iLatency_Average.Count = 25 Then
            allLatency = iLatency_Average.ToArray()
            iLatency_Average.Dequeue()
        End If
    End SyncLock

    ' Calculate average
    If allLatency IsNot Nothing Then
        Array.Sort(allLatency)

        Dim iAverage As Long
        For i As Integer = 5 To 19
            iAverage += allLatency(i)
        Next

        Return CLng(iAverage / 15)
    Else
        Return 0
    End If
End Function

I also split your logic into two part. One part that adds the new value, the other part that calculate the average.

score 0 · Answer 3 · answered Mar 24 '15 at 17:58

this version doesn't use SyncLock which is slow but its using the recommendation posted on comment for BlockingCollection.

Dim iLatency_Average As New Concurrent.BlockingCollection(Of Long)(25)
Public Function Average_Latency(myLatency As Long) As Long

    iLatency_Average.TryAdd(myLatency)

    If iLatency_Average.Count = 25 Then
        Dim allLatency() As Long = iLatency_Average.ToArray

        Array.Sort(allLatency)

        Dim iAverage As Long
        For i As Integer = 5 To 19
            iAverage += allLatency(i)
        Next

        iLatency_Average.TryTake(0)

        Return CLng(iAverage / 15)
    Else
        Return 0
    End If
End Function

I would check the size of the array instead. Chances are very small but it's possible that the collection gets cleared between the check for Count and the call to ToArray. Depends also on the rest of the code. — the_lotus, Mar 24 '15 at 18:05

Issue with array was not long enough

3 Answers3