Link to home
Start Free TrialLog in
Avatar of Philippe Renaud
Philippe RenaudFlag for Canada

asked on

Speeding up a Loop

Hello EE,

In my VB.NET Form, I have the following :

        For x As Integer = 0 To last.Count - 1
            Dim arrString = last(x).Split(New String() {","}, StringSplitOptions.RemoveEmptyEntries)
            Dim qx = (From m In MyItems
                      Where arrString.All(Function(v) m.Value.Contains(v))
                      Order By m.Key Descending
                      Select m).ToDictionary(Function(y) y.Key, Function(z) z.Value)
            i += 1
            percentComplete = (i / Convert.ToDecimal(last.Count())) * 100.0
            finalLoop.Add(last(x), Math.Abs(DateDiff(DateInterval.Day, Now, qx.Keys(0))))
            bgw2.ReportProgress(Math.Round(percentComplete, 2), Math.Round(percentComplete, 2))
        Next

Open in new window


LAST  contains about 50,000 elements.
MyItems contains about 9,000 elements.


it takes about 8 minutes to run all this.
What I am doing is, I check if all element of arrString is in the current index of myItem, take it and do something otherwise skip and continue.


My question is, do I do this the most efficient way or it could be more faster ?
right now 8 mins is not bad, but I might have more soon in "LAST" it could go up to 100,000 and it could take alot of time...

any idea ?
Avatar of Peter Hutchison
Peter Hutchison
Flag of United Kingdom of Great Britain and Northern Ireland image

1. You can use UShort (0-65,536) instead of Integer variables (0-2 million).
2. Use the Dim statements _before_ the loop to define the variables
e.g Dim arrString as String, qx as Object
3. Store last.Count() to a variable, and use the variable in the For Next statements and percentComplete statement.
Avatar of Philippe Renaud

ASKER

I tried all this... but it's not faster (not to know any difference with the eye)
move DateInterval.Day, Now, outside of the loop as you are calling it every iteration
remove last.count out of the loop and assign it to a variable and then remove the Convert.ToDecimal(last.count) function
Instead of calling bgw2.ReportProgress() for every single iteration, simply set the value of "percentComplete" and press on.  Get rid of the line that calls ReportProgress().  Now add a Timer to your form and set its Interval() property to one second (or maybe a half second).  In the Tick() event, update your label (or whatever you're tracking progress with).  Be sure to turn on the Timer when you start the BackgroundWorker, and then turn it back off in the RunWorkerCompleted() event.
Mike, I did all your stuff, it "may" be faster but I dont see the diff.

Maybe im just dreaming and Its not possible to considerably speed that up.
I know the biggest part of the work in term of speed is the LINQ working like hell on all iterations. But I think it's well coded...

David.. I will try yours also
>>My question is, do I do this the most efficient way or it could be more faster ?

Before you start making changes (which well meant might bring nothing) check where the time is being consumed first.  Then you know if you have a chance to succeed. eg. Use a profiling tool to analyse your current code.

I once had a loop that took a long time.  Making changes like those well meant suggestions did bring trivial improvements.  After some time I (kicked myself and did what I should have done and) analysed the code with a profiler and found 95% of the time was on one system call.  In other words no matter how much I improved my code I was still faced with it taking 95%+ of the original time because that was code I had no influence on.
I'm fairly certain your issues lie here:

Dim arrString = last(x).Split(New String() {","}, StringSplitOptions.RemoveEmptyEntries)
Dim qx = (From m In MyItems
          Where arrString.All(Function(v) m.Value.Contains(v))
          Order By m.Key Descending
          Select m).ToDictionary(Function(y) y.Key, Function(z) z.Value)

Open in new window


...but without knowing what is in those collections or what the goal of that block of code is, I doubt we could help you optimize it.

P.S.

The UShort suggestion above would have zero impact on performance here.
in arrString i will receive 3 values something like:     11, 20, 66

then in the LINQ, i check : if a row contains ALL those 3 values put it into the dictionnary and I do OrderBY Descending because there is a date as well. so at the end the index 0 would be the latest date.

Thats about it honestly...
You think it would be faster if in my LINQ statement I say something like :

add to QX only if what you found is greater (as a date) than what you previously found otherwise skip ?
i would not be sure how to write it tho.

Also, maybe i dont have to go through all MYItems.
if I start to search by the latest, i can stop as soon as I find an element...
ASKER CERTIFIED SOLUTION
Avatar of Philippe Renaud
Philippe Renaud
Flag of Canada image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Seems your LINQ query is a bit overloaded. As I understood all you need is a max Date? Why do you create Dictionary?
Dim MaxDate = (From m In MyItems
               Where arrString.All(Function(v) m.Value.Contains(v))
               Select m.Value).Max()

Open in new window

Or, even better, sort MyItems by values  descending BEFORE loop and use FirstOrDefault() instead of Max()
i found it myself