Code Efficiency

[kyern]

I have a report generator that is having some speed issues. What this does is loops over each category in the db per each category and performs and count on each one. The SQL queries are lightning fast, I even use getrows to get the data out quickly, but I think I should be able to use the loops much better than I do. The String Building is unoptimized I realize but that shouldn't give me as bad performance problems as I'm having. If anyone would be willing to take a look and suggest anything I'm all ears. Most of the problem is how much of this has to be generated dynamically, if I could do something to build the main and subcategories faster it would be fantastic.

Thanks

David

from_date = trim(Request.Form("fromdate"))
	to_date = trim(Request.Form("todate"))
	grp = trim(Request.Form("grp"))
	
	Set main_category_rows = New GetRows
	arr_maincat = main_category_rows.GetArray("SELECT maincategory_id, maincategory FROM tracking_maincategory")
	
	strDropDown = "<option value="""">---- Select an option ----</option>"
	For j = 0 To UBound(arr_maincat, 2)
		maincategory_id = arr_maincat(0, j)
		maincategory = arr_maincat(1, j)
		
		strHTML = strHTML & "<td valign=""top""><fieldset><legend><a href="""">" & maincategory & "</a></legend>" & vbCrLf
		strHTML = strHTML & "<table height=""212"" style=""font-size: 12px; font-family: verdana;"">"
		
		Set subcategory_rows = New GetRows
		arr_subcat = subcategory_rows.GetArray("SELECT category_id, subcategory FROM tracking_category WHERE activated = 1 AND maincategory_id = '" &  maincategory_id & "'")

		For k = 0 To UBound(arr_subcat, 2)
			category_id = arr_subcat(0, k)
			category = arr_subcat(1, k)
			
			strDropDown = strDropDown & "<option value=""" & category_id & """>" & maincategory & " - " & category
			
			strHTML = strHTML & "<tr><td valign=""top"">" &  category & "</td>"
			Set cat_count = New GetRows
			strSQL = "SELECT COUNT(category) FROM incident_tracking WHERE category = '" &  category_id & "'"
			
			If from_date <> "" and to_date <> "" Then
				strSQL = strSQL & " AND submitdate BETWEEN '" & from_date & "' AND '" & to_date & "'"
			Else
				strSQL = strSQL & " AND submitdate BETWEEN DATE_SUB(curdate(),INTERVAL WEEKDAY(curdate()) DAY) AND DATE_ADD(curdate(),INTERVAL 5-WEEKDAY(curdate()) DAY)"
			End If
			
			arr_cat_count = cat_count.GetArray(strSQL)
			count = arr_cat_count(0, 0)
			
			If from_date <> "" and to_date <> "" Then
				indQuery = "?from_date=" & from_date & "&to_date=" & to_date & "&grp=" & category_id
			Else
				indQuery = "?grp=" & category_id
			End If
			
			strHTML = strHTML & "<td valign=""top"" align=""right"" width=""40""><a href=""TrackingReport_ind.asp"  & indQuery & """ >" & count & "</a></td></tr>"
		Next
		
		strHTML = strHTML & "</table>"
		strHTML = strHTML & "</fieldset></td>"
	Next
	
	If from_date <> "" and to_date <> "" Then
		If grp <> "" Then
			excelQuery = "?from_date=" & from_date & "&to_date=" & to_date & "&grp=" & grp
		Else
			excelQuery = "?from_date=" & from_date & "&to_date=" & to_date
		End If
		extendedTitle = " For " & formatDate("%F %j%O", from_date) & " Through " & formatDate("%F %j%O", to_date)
	ElseIf grp <> "" Then
		excelQuery = "?grp=" & grp
		extendedTitle = " For The Current Week"
	Else
		extendedTitle = " For The Current Week"
	End If

 

[xwb]

Your dropdown list must be absolutely massive. At a guess it is this and the tables that are causing the performance hit. Would it be possible to limit the size of the tables to say 30?

 

[IT4EVR]

Since you are doing a lot here with strings, using the StringBuilder class may afford you some benefits.

 

[Tarwn]

SQL Queries if double for loops...whee.

Say you have 5 main categories, each with 5 sub categories. To load your page your doing
1 Query for main category *
5 Queries for subcategories *
5 Queries for incident counts =
25 queries

Ok, say you have 20 categories, each with 10 subcategories:
200 SQL Queries

The concatenation inside the loops makes this even worse as concatenation is inefficient in VBScript.


My suggesiton would be to reduce your 3 SQL statements down to a single SQL statement. Something like:

SELECT TMC.maincategory_id, TMC.maincategory, TC.category_id, TC.subcategory, COUNT(IT.category) as IncidentCount
FROM tracking_maincategory TMC LEFT JOIN tracking_category TC ON TMC.maincategory_id = TC.maincategory_id AND TC.activated = 1
LEFT JOIN incident_tracking IT ON TC.category_id = IT.category AND submitdate BETWEEN _____ AND _____
GROUP BY MC.maincategory_id, TMC.maincategory, TC.category_id, TC.subcategory
ORDER BY TMC.maincategory, TC.subcategory

Now you would just loop through the one array of data, keeping track of which category and subcategopry you were on so that when they change you can output the additional bits of code you need:

CONST MCAT_ID=0
CONST MCAT_NAME=1
CONST CAT_ID=2
CONST CAT_NAME=3
CONST INC_CNT = 4
Dim cur_mc, cur_sc
Dim end_mc, end_sc
end_mc = false
end_sc = false
Dim rowCtr

'do whatever you need to before the loop

For rowCtr = 0 to UBound(MyArray,2)
   'Check if we are starting a new main category
   If MyArray(MCAT_ID,rowCtr) <> cur_mc Then
      'put code here for start of new main category

      cur_mc = yArray(MCAT_ID,rowCtr)
      cur_sc = ""  'empty the subcat oto force start of new sub cat
   End If

   'Check if we are startinga new sub cat
   If MyArray(CAT_ID,rowCtr) <> cur_sc Then
      'put code here for start of new subcat

      cur_sc = MyArray(CAT_ID,rowCtr)
   End If

   'put code here to handle the incident count - MyArray(INC_CNT,rowCtr)

   'check to see if we need to end current sc/mc
   If rowCtr = UBound(MyArray,2) Then
      'out of rows, end both
      end_sc = True
      end_mc = True
   ElseIf MyArray(MCAT_ID,rowCtr + 1) <> cur_mc Then
      'main cat is changing, end both
      end_sc = True
      end_mc = True
   ElseIf MyArray(CAT_ID,rowCtr + 1) <> cur_sc Then
      'sub cat is changing
      end_sc = true
   End If

   'Finish the current sub cat
   If end_sc Then
      'code for end of sub cat
      end_sc = false
   End If

   'Finish the current main cat
   If end_mc Then
      'code for end of main cat
      end_mc = false
   End If
Next

'additional code after loop

Now you would only have a single SQL statement and a single loop to handle. With the addition of some sort of string builder, as IT4EVER suggested, you should see a remarkable increase in speed. Even without it you should see a significant increase since you won't be manipulating ADO objects as much.

BTW, there may be some syntax errors in there, I wrote it on the fly and guessed at some of the database stuff. I didn't include the SQL portion because I provided an example and you need to decide whether it would be better to generate those dates from VBScript or in SQL. I would also suggest that you may wantto move the entire SQL statement to a stoed procedure once you have it working the way you want it.

-T

 

[onpnt]

Good post as always Tarwn


____________ signature below ______________
General FAQ faq333-2924
5 steps to asking a question faq333-3811

 

[Tarwn]

Unfortunatly that wa smy morning coffee post and itleft me with no time to answer anyother questions (and actually made a little late to work )