I have a spreed sheet and its something like 550+ columns and I need to pull URLs from it. Previously I was using LinqToExcel when the sheet was less than 255 columns a recent update has produced this monstrosity, so I have moved on to interop with excel to parse the data.

Needless to say I have a list of a string I want to name all the images to, plus the index of that image for this item(uniqueness of file names). Currently I use 10 threads to do the queries for the data. 10 HttpClients, 1 on each thread, and then I use a paralle.ForEach and Partitioner to dole out batches of 100 items to each parallel thread. This is not a Image pull from 1 website, many websites may be covered in the items.

Is This the Fastest way to do it? Old Code

Relevant new code:

Imports System.Collections
Imports System.Collections.Generic
Imports System.Collections.Concurrent
Imports System.Data
Imports System.Linq
Imports System.Net
Imports System.Net.Http
Imports System.IO
Imports System.Text
Imports System.Xml.Linq
Imports Microsoft.Office.Interop.Excel

Public Class MDI
Public gtin As String
Public lUrls As List(Of String) = New List(Of String)()
End Class
Public Shared Function GetXlsxFiles(targetFolder As String) As String()
Dim files As String()
If targetFolder = vbNullString Then
files = System.IO.Directory.GetFiles(System.IO.Directory.GetCurrentDirectory(), "Export*.xlsx")
Else
If System.IO.Directory.Exists(targetFolder) = True Then
files = System.IO.Directory.GetFiles(targetFolder, "Export*.xls")
Else
files = System.IO.Directory.GetFiles(System.IO.Directory.GetCurrentDirectory(), "Export*.xlsx")
End If
End If
Return files
End Function
Public Shared Sub ProcessFolder(targetFolder As String)
Dim ListParams As New List(Of ImgThreadParams)()
Dim gtin As String = vbNullString
Dim files = GetXlsxFiles(targetFolder)
Dim excel As Application = New Application()
Try

For Each fileName As String In files
'Dim book = New LinqToExcel.ExcelQueryFactory(System.IO.Path.GetFileName(fileName))
Dim w As Workbook = excel.Workbooks.Open(fileName)
' Get sheet.
Dim sheet As Worksheet = CType(w.Sheets(1), Worksheet)
' Get range.
Dim r As Range = sheet.UsedRange()
Dim rArray(,) As Object = CType(r.Value(XlRangeValueDataType.xlRangeValueDefault), Object(,))
Dim hl As Integer = 0
Dim hn As Integer = 0
Dim rt As Integer = 0
Dim GtinC As Integer = 0
Dim Img1 As Integer = 0
Dim Img2 As Integer = 0
Dim Img3 As Integer = 0
Dim Img4 As Integer = 0
Dim Img5 As Integer = 0
Dim Img6 As Integer = 0
Dim Img7 As Integer = 0
Dim Img8 As Integer = 0
Dim Img9 As Integer = 0
Dim Img10 As Integer = 0
Dim ic As Integer = 0
Dim ic3 As Integer = 0
For j As Integer = 1 To r.Columns.Count()

If rArray(4, j).ToString().StartsWith("GTIN") Then
GtinC = j
ElseIf rArray(4, j).Equals("Record type") Then
rt = j
ElseIf rArray(4, j).ToString.Equals("Hierarchy number") Or rArray(4, j).ToString.Equals("Hierarchy level") Then
ic3 = ic3 + 1
If (ic3 = 1) Then
hn = j
ElseIf ic3 = 2 Then
hl = j
End If
ElseIf rArray(4, j).ToString.StartsWith("Image") And Not rArray(4, j).ToString.EndsWith("Description") And Not rArray(4, j).ToString.Contains("File") Then
ic = ic + 1
Select Case ic
Case 1
Img1 = j
Case 2
Img2 = j
Case 3
Img3 = j
Case 4
Img4 = j
Case 5
Img5 = j
Case 6
Img6 = j
Case 7
Img7 = j
Case 8
Img8 = j
Case 9
Img9 = j
Case 10
Img10 = j
Case Else
Exit Select
End Select
End If
Next
Dim additem As Boolean = False
For i As Integer = 5 To r.Rows.Count()
'check Hierarchy Number Level and record Type to filter for only top level GTIN's to download
If (CType(rArray(i, hn), Integer) = 1 And CType(rArray(i, hl), Integer) = 1 And CType(rArray(i, rt), Integer) = 1) Then
GoTo ContinueLoop
Else
Continue For
End If

ContinueLoop:
itp.gtin = CType(rArray(i, GtinC), String)
If (rArray(i, Img1) IsNot Nothing) Then
If (Uri.IsWellFormedUriString(CType(rArray(i, Img1), String), UriKind.Absolute)) Then
End If
End If
If (rArray(i, Img2) IsNot Nothing) Then
If (Uri.IsWellFormedUriString(CType(rArray(i, Img2), String), UriKind.Absolute)) Then
End If
End If
If (rArray(i, Img3) IsNot Nothing) Then
If (Uri.IsWellFormedUriString(CType(rArray(i, Img3), String), UriKind.Absolute)) Then
End If
End If
If (rArray(i, Img4) IsNot Nothing) Then
If (Uri.IsWellFormedUriString(CType(rArray(i, Img4), String), UriKind.Absolute)) Then
End If
End If
If (rArray(i, Img5) IsNot Nothing) Then
If (Uri.IsWellFormedUriString(CType(rArray(i, Img5), String), UriKind.Absolute)) Then
End If
End If
If (rArray(i, Img6) IsNot Nothing) Then
If (Uri.IsWellFormedUriString(CType(rArray(i, Img6), String), UriKind.Absolute)) Then
End If
End If
If (rArray(i, Img7) IsNot Nothing) Then
If (Uri.IsWellFormedUriString(CType(rArray(i, Img7), String), UriKind.Absolute)) Then
End If
End If
If (rArray(i, Img8) IsNot Nothing) Then
If (Uri.IsWellFormedUriString(CType(rArray(i, Img8), String), UriKind.Absolute)) Then
End If
End If
If (rArray(i, Img9) IsNot Nothing) Then
If (Uri.IsWellFormedUriString(CType(rArray(i, Img9), String), UriKind.Absolute)) Then
End If
End If
If (rArray(i, Img10) IsNot Nothing) Then
If (Uri.IsWellFormedUriString(CType(rArray(i, Img10), String), UriKind.Absolute)) Then
End If
End If
End If
Next
rArray = Nothing
w.Close()
Runtime.InteropServices.Marshal.ReleaseComObject(r)
Runtime.InteropServices.Marshal.ReleaseComObject(sheet)
Runtime.InteropServices.Marshal.ReleaseComObject(w)
Runtime.InteropServices.Marshal.ReleaseComObject(excel)

Dim Folder As DirectoryInfo = System.IO.Directory.CreateDirectory(System.IO.Directory.GetCurrentDirectory() + "\Image")
If ListParams.Count > 10 Then
Dim tCount As Integer = CInt(Math.Round(ListParams.Count / 10, MidpointRounding.ToEven))
Dim L1 As New List(Of ImgThreadParams)
For i As Integer = 0 To tCount
Next
Dim L2 As New List(Of ImgThreadParams)
For i As Integer = tCount + 1 To tCount * 2
Next
Dim L3 As New List(Of ImgThreadParams)
For i As Integer = tCount * 2 + 1 To tCount * 3
Next
Dim L4 As New List(Of ImgThreadParams)
For i As Integer = tCount * 3 + 1 To tCount * 4
Next
Dim L5 As New List(Of ImgThreadParams)
For i As Integer = tCount * 4 + 1 To tCount * 5
Next
Dim L6 As New List(Of ImgThreadParams)
For i As Integer = tCount * 5 + 1 To tCount * 6
Next
Dim L7 As New List(Of ImgThreadParams)
For i As Integer = tCount * 6 + 1 To tCount * 7
Next
Dim L8 As New List(Of ImgThreadParams)
For i As Integer = tCount * 7 + 1 To tCount * 8
Next
Dim L9 As New List(Of ImgThreadParams)
For i As Integer = tCount * 8 + 1 To tCount * 9
Next
Dim L10 As New List(Of ImgThreadParams)
For i As Integer = tCount * 9 + 1 To ListParams.Count - 1
Next
Dim t1, t2, t3, t4, t5, t6, t7, t8, t9, t10 As Threading.ParameterizedThreadStart
t1.Invoke(L1)
t2.Invoke(L2)
t3.Invoke(L3)
t4.Invoke(L4)
t5.Invoke(L5)
t6.Invoke(L6)
t7.Invoke(L7)
t8.Invoke(L8)
t9.Invoke(L9)
t10.Invoke(L10)
Else
t1.Invoke(ListParams)
End If
Next
Catch ex As Exception
Exit Sub
End Try
End Sub

Dim wc As HttpClient = New HttpClient()
Const ItemBatch As Integer = 100
Parallel.ForEach(Partitioner.Create(0, Params.Count, ItemBatch), Async Sub(range)
Dim imgCount As Integer = 1
For i As Integer = range.Item1 To range.Item2 - 1
Dim item = Params(i)
imgCount = 1
For Each surl In item.lUrls
Try
Dim response As HttpResponseMessage = Await wc.GetAsync(surl)
If (response.StatusCode = 200) Then
Dim sContentType As String = response.Content.Headers.ContentType.MediaType.ToString()
If (sContentType.Length > 0) Then
Dim sImage As String = sContentType.Split("/")(0).ToLower
Dim sImageType As String = sContentType.Split("/")(1).ToLower
If sImage.Equals("image") = True Or sImage.Equals("text") Then
Dim sFileExtension As String
Select Case sImageType
Case "jpeg"
sFileExtension = ".jpg"
Case "png"
sFileExtension = ".png"
Case "bmp"
sFileExtension = ".bmp"
Case "gif"
sFileExtension = ".gif"
Case "tiff"
sFileExtension = ".Tiff"
Case "html"
sFileExtension = ".html"
Case Else
Continue For
End Select
Dim sFolder = Directory.GetCurrentDirectory() + "\Image"
Using Fi As FileStream = New FileStream(sFolder + "\" + item.gtin + "_" + imgCount.ToString + sFileExtension, FileMode.OpenOrCreate)
Await Fi.WriteAsync(d, 0, d.Length)
Await Fi.FlushAsync()
End Using
imgCount = imgCount + 1
End If
End If
End If
Catch
Continue For
End Try
Next
Next
End Sub)

End Sub
Public Shared Sub Main()
ServicePointManager.DefaultConnectionLimit = 10000
ProcessFolder(System.IO.Directory.GetCurrentDirectory())
End Sub

End Class

• My initial thought is that excel at 550+ columns, it's a good sign you're probably using the wrong tool for the job. Can you tell us: where is your code slow? What is the bottleneck (memory, disk, network?) ...or is that the part you don't know how to determine? – James Snell Sep 15 '16 at 16:25
• Just going to throw this link: Using Task.WhenAll That should hopefully help to understand how to use the async API properly. – Ext3h Sep 15 '16 at 17:13
• @james Snell: well the code is not slow it gets over 2k images in about 10 mins, but I want it to run as fast as possible given any machine that runs it. – BanMe Sep 15 '16 at 17:31
• @Ext3h there is to much overhead created by tasks. my last implemention used Task.WhenAll. :) Thank you though. – BanMe Sep 15 '16 at 17:56
• Mere 2k images, and the overhead from using tasks was already too much? Find that hard to believe. Besides, your previous solution had the same mess with manually defining a "threadpool". Strip that manual threading, keep the async implementation from your second attempt (instead of Task.Factory.StartNew wrapping synchronous code!), go back to only a single image per task. There is simply no way you are CPU bound during download, so don't even bother to thread. Going async alone is sufficient. – Ext3h Sep 15 '16 at 18:34

Just a few quick remarks (for now):

• vbNullString is a remnant of VB6 (which I think would be under the Microsoft.VisualBasic.Constants namespace), and used to be a null string pointer often confused with "", an empty string. In VB.NET vbNullString is Nothing, so when you do this:

If targetFolder = vbNullString Then


You're actually verifying whether targetFolder contains a reference at all... but you probably mean to check whether the provided string is null or empty. A more idiomatic way to test for this is String.IsNullOrEmpty:

If String.IsNullOrEmpty(targetFolder) Then


VB.NET's String type overrides the = operator such as Not String.IsNullOrEmpty(s), s <> "", s <> String.Empty and s <> vbNullString are all equivalent.

• The following only apply if your project defaults options aren't customized:

• You're not specifying Option Explicit explicitly, but that's fine because in VB.NET that's the default behavior, contrary to VB6/VBA where implicit declarations are allowed by default.
• You're not specifying Option Strict either, and that's less fine, because by default, the VB.NET compiler won't enforce strict data typing - VB6/VBA was very loose with type conversions, and without Option Strict, VB.NET keeps a lot of that looseness. Note that Option Strict implies Option Explicit, so you don't have to specify both.
• You're specifying Imports statements, but then you fully-qualify things in these imported namespaces - this defeats the purpose: either you Imports namespaces, or you fully-qualify stuff. For example, you have Imports System.IO, so this code:

files = System.IO.Directory.GetFiles(System.IO.Directory.GetCurrentDirectory(), "Export*.xlsx")


files = Directory.GetFiles(Directory.GetCurrentDirectory(), "Export*.xlsx")


There really isn't much of a reason to directly work with a Thread anymore, especially given Imports System.Threading.Tasks. Let the framework deal with the metal-level threading stuff: it's very unlikely that starting 10 threads is buying you anything in reality - no computer has 10 CPU cores anyway.

It's impossible to read ProcessFolder and know what's going on at a glance. The procedure is doing WAY too many things, at a WAY too low abstraction level. Extract methods out of code blocks, reduce the nesting: this is a clear sign that something isn't right:

That ThreadedGetGtinImages procedure is running on a thread already - and then you start a Parallel.ForEach loop inside that to start an Async Sub that does its thing... Get rid of the threads, and launch asynchronous tasks instead: I suspect your code has a TON of threading overhead that's nullifying the benefits of it all.

Also... whenever you feel the need to name something t1, t2, t3, ..., t12 ...it means you've missed an opportunity to use an appropriate data structure, e.g. some array or some List(Of Something) - that would have eliminated all that huge chunk of repeated code there, where you populate the Ln lists - IMO that should be a function in its own right.

The identifiers you choose aren't consistently meaningful - I get ProcessFolder and excel and sheet, but t7 and itp and r and w and hn and hlare terrible names that will definitely make you want to pull your hair off if you drop this and come back to it in 6 months from now. By the way hl is pure evil, especially in a code base where it might just as well be h1.

• I will take all of what you say under advisement and update to the code to use more meaningful names for variables.I have reworked this code many different ways before, but this method seems to accomplish the goal the fastest so far. I will look at what I can do to reduces the nested complexity of it. – BanMe Sep 15 '16 at 15:15
• You say, "it's very unlikely that starting 10 threads is buying you anything in reality - no computer has 10 CPU cores anyway"; but for networking stuff, where connection handshakes and packet retries can take entire milliseconds and are very much network- rather than CPU-bound, concurrency can help a lot even on single processor machines. I can't read VB very well, though; so it's possible this is not what OP is using the threads for, in which case I apologize for my useless tangent. – Daniel Wagner Sep 15 '16 at 19:09
• @DanielWagner it's a valid point... to a certain extent - non-CPU bound work should be asynchronous, not threaded: OP doesn't need threads, he needs tasks - and let the framework determine if a task would be more efficiently executed on its own thread or not. – Mathieu Guindon Sep 15 '16 at 19:36
• So i have turn it to all tasks and did a whenall and I get numerous TaskCancelationExceptions as well HttpRequestException. Is there a task Limit!? or request limit that I am hitting? i am doing 1786 concurrent calls it works some of the time but fails alot.. – BanMe Sep 16 '16 at 4:55
• You're doing something wrong, if you can reproduce the issue with minimal code that will make a good SO question. – Mathieu Guindon Sep 16 '16 at 11:19