optimization tips

[mr_doles]

Let me preface this by saying I am not a programmer, I write code to
make my like easier. So at the end of the day when I press the button
as long as 1 + 1 = 2, I don't care how ugly the code is. Now I am
writing some code for someone else and I have a window that takes
forever to open. I know that it is performing 3 large tasks in the
onLoad event. It works great just takes a while to load, any
optimization tips would be appreciated (no laughing at the code
because it does work).

We have a 3rd party app that sucks in info from all our computers and
puts it in a SQL database. One thing we pull in is uptime.exe
(support.microsoft.com/kb/232243) information. What I look for is
STOP errors and when they occurred and present them in a tree view
with the computer name as the parent node and each date as the child
node.

Here are the three steps (all in the onLoad event):
Variables:
Dim lo_sql_command As New SqlCommand()
Dim dsUptime As New DataSet()
Dim da As New SqlDataAdapter(lo_sql_command)
Dim s_line As String
Dim la_bluescreen As New ArrayList

1) Connect to SQL run query to get station name (string) and log file
(text) and put them in a DataSet

Dim lo_sql_command As New SqlCommand()
Dim dsUptime As New DataSet()
Dim da As New SqlDataAdapter(lo_sql_command)
Dim s_line As String

With lo_sql_command
.Connection = MyGlobals.trackitConnection
.CommandType = CommandType.Text
.CommandText = BuildBootQuery()
End With

Try
dsUptime.Clear()
da.Fill(dsUptime)
Catch ex As Exception
MessageBox.Show(ex.Message.ToString)
Exit Sub
Finally
lo_sql_command.Dispose()
da.Dispose()
End Try

2) Look at each rows text line by line for each match on STOP and
Bluescreen and add them to an array. Then add it to a hash table with
computer name as key and array as value.

For Each row As DataRow In dsUptime.Tables(0).Rows
Dim sr As New StringReader(row.Item(1).ToString)
la_bluescreen.Clear()

Try
'fill array
While sr.Peek <> -1
s_line = sr.ReadLine
If (Regex.IsMatch(s_line, "Bluescreen") = True)
And (Regex.IsMatch(s_line, "STOP") = True) Then
la_bluescreen.Add(Trim(s_line))
End If
End While

'Add to hastable
If (la_bluescreen.Count <> 0) Then
lh_bluescreens.Add(row.Item(0).ToString,
la_bluescreen)
End If

Catch ex As Exception
Throw ex
Finally
sr.Dispose()
dsUptime.Dispose()
End Try
Next

3)Create the tree view based on the hashtable and sort them
alphabetically

' Suppress repainting the TreeView until all the objects have
been created.
TreeView1.BeginUpdate()
TreeView1.Nodes.Clear()

' Add a root TreeNode for each Computer in the ArrayList -
Append the count
Dim nodecount As Integer = 0

For Each de As DictionaryEntry In lh_bluescreens

TreeView1.Nodes.Add(de.Key.ToString & " - {" &
DirectCast(de.Value, ArrayList).Count & "}")

' Add a child TreeNode for each stop error for current
computer.
Dim value As String
For Each value In DirectCast(de.Value, ArrayList)
TreeView1.Nodes(nodecount).Nodes.Add(value.ToString)
Next value
nodecount += 1

Next de

TreeView1.Sort()
' Begin repainting the TreeView.
TreeView1.EndUpdate()

'Cleanup
la_bluescreen = Nothing
lh_bluescreens = Nothing

 

[Cor Ligthert [MVP]]

Hi,

mr_doles,

While your samples look goods some points I saw in your program:

I see you love the underscore however the way you do it, is not done
anymore.
Setting everything to nothing while just before it goes out of scope has no
sense. The same for dispossing for classes which have that build in by
instance: Dataset, (Data) Command, DataAdapter. (The most which we use have
that build in). Casting from 2 classes which both implement the same
interface is in my idea without sense (arraylist, hashtable).

You don't use the very handy method 'for' but replace that for a while or
whatever, I (and I have seen most in this newsgroup) use the for index.

Using the word "value" as dataname is not wrong in basic, however because
that it is often a reserved name in other languages I would try to avoid
that.

I saw "Bluescreen has its sense, therefor I am in doubt which name I would
use, however most likely it would be blueScreen

Just some things that I saw, which gives you no optimilisations. However
optimimalisations should be forgotten in this millenium, maintainability is
much more important.

There is no laughing at the code, however just some things are to much.

Cor

 

[mr_doles]

Setting everything to nothing while just before it goes out of scope has no

sense. The same for dispossing for classes which have that build in by
instance: Dataset, (Data) Command, DataAdapter. (The most which we use have
that build in).

Actually I have seen a major decrease in memory usage by calling the
dispose methond when I am finsihed with an object rather then waiting
for the GC to do it for me.

You don't use the very handy method 'for' but replace that for a while or
whatever, I (and I have seen most in this newsgroup) use the for index.

Not sure what you mean here at all.

Just some things that I saw, which gives you no optimilisations. However
optimimalisations should be forgotten in this millenium, maintainability is
much more important.

I completely disagree with this. When you program for someone else
they only care about 2 things. It does what it should and it does
what it should quickly.

I actually was able to make this run twice as fast then it did
before. Instead of creating the hashtable then filling the TreeView
from the hastable I fill the tree after I find what I need. This cuts
out a big step and it went from 80 seconds to load to 30 seconds to
load.

 

[Cor Ligthert [MVP]]

Actually I have seen a major decrease in memory usage by calling the

dispose methond when I am finsihed with an object rather then waiting
for the GC to do it for me.

Does that speed up anything or something like that as the memory is not
needed?

Not sure what you mean here at all.

I did you not see using the
For i as integer etc.

I completely disagree with this. When you program for someone else
they only care about 2 things. It does what it should and it does
what it should quickly.

You are free, forget maintainability, make programs that are so big after a
while that they don't perform anymore.

I actually was able to make this run twice as fast then it did
before. Instead of creating the hashtable then filling the TreeView
from the hastable I fill the tree after I find what I need. This cuts
out a big step and it went from 80 seconds to load to 30 seconds to
load.

Was you asking for advice or showing what a nice program you made, as what I
thought before I think the last. However if your load time is 30 seconds on
any computer than you should have a look how to revise your program, that is
very long. Did you use the begin and endedit for your treeview?

Cor