patterncsharpMinor
WebMethod slower on execution than Windows application of the same code
Viewed 0 times
samethewebmethodapplicationthanslowerwindowscodeexecution
Problem
I have developed an application that interacts with IBM ClearQuest. The problem is that when I run everything locally, such as, run the webservice local and then ASP page local, everything is at the speed I expect. When I post the webservice (precompiled) to the server and run the web page through the server, the call to the webmethod takes at least 10x the amount of time it should. I don't know why this is happening. I made a console application that has the function in question and executes it on the server and locally, and they both return the same amounts of time (roughly). It's just when I move to executing via the webmethod that everything grinds to a snails pace.
Any ideas? This happens every time not just on the first call.
Maybe a code review might help here?
WebMethod (VB):
`Public Function RetrieveQueryResults(ByRef cqSession As ClearQuestOleServer.Session, _
ByVal sqlStmt As String) As List(Of SearchResultsSingleIssue)
Dim numCols As Integer, status As Integer, columnIdx As Integer
Dim numRows As Integer
Dim rowContents As String = ""
Dim colValue As New Object
Dim colLabel As New Object
Dim allitems As New List(Of SearchResultsSingleIssue)
Dim results As New SearchResultsSingleIssue
Dim cqResultSet As ClearQuestOleServer.OAdResultset
cqResultSet = cqSession.BuildSQLQuery(sqlStmt)
cqResultSet.Execute()
' Get the number of columns returned by the query.
numRows = 0
numCols = cqResultSet.GetNumberOfColumns
status = cqResultSet.MoveNext
' Collect query results.
Do While status = AD_SUCCESS
results = New SearchResultsSingleIssue
numRows = numRows + 1
For columnIdx = 1 To numCols
colLabel = cqResultSet.GetColumnLabel(columnIdx)
colValue = cqResultSet.GetColumnValue(columnIdx)
'Make sure that we dont pass along a null reference
If colValue = Nothing Then
colValue = ""
End If
Select Case colLabel
Case "ID"
results
Any ideas? This happens every time not just on the first call.
Maybe a code review might help here?
WebMethod (VB):
`Public Function RetrieveQueryResults(ByRef cqSession As ClearQuestOleServer.Session, _
ByVal sqlStmt As String) As List(Of SearchResultsSingleIssue)
Dim numCols As Integer, status As Integer, columnIdx As Integer
Dim numRows As Integer
Dim rowContents As String = ""
Dim colValue As New Object
Dim colLabel As New Object
Dim allitems As New List(Of SearchResultsSingleIssue)
Dim results As New SearchResultsSingleIssue
Dim cqResultSet As ClearQuestOleServer.OAdResultset
cqResultSet = cqSession.BuildSQLQuery(sqlStmt)
cqResultSet.Execute()
' Get the number of columns returned by the query.
numRows = 0
numCols = cqResultSet.GetNumberOfColumns
status = cqResultSet.MoveNext
' Collect query results.
Do While status = AD_SUCCESS
results = New SearchResultsSingleIssue
numRows = numRows + 1
For columnIdx = 1 To numCols
colLabel = cqResultSet.GetColumnLabel(columnIdx)
colValue = cqResultSet.GetColumnValue(columnIdx)
'Make sure that we dont pass along a null reference
If colValue = Nothing Then
colValue = ""
End If
Select Case colLabel
Case "ID"
results
Solution
This being the oldest c# zombie on the site, I thought the code could use a bit of reviewing. I don't think I can answer the question about the weird performance issue though.
VB
Declarations
You're not consistent with your declaration style:
While this is correct and avoids the common mistake of only declaring a type to the last variable declared, I don't like multiple declarations on the same line. It's more readable when they're separated. These three being declared on the same line just seems arbitrary.
The variables should be declared as close as possible to their usage, this block of declarations at the top of your method isn't helping readability either.
This line serves no purpose and can be eliminated:
Loop
You're fetching the first result before iterating the result set:
And then your loop ends like this:
I think you can eliminate the
The loop doesn't need to enter the
Not sure I like the
The point, is that you're naming all columns and all properties anyway, so the loop+switch isn't buying you anything except the need to now track what column index you're at.
Naming
I don't like the underscore in identifiers. Stick to PascalCase convention, be consistent. Also
If
C#
Most of the above also applies to the C# code. Both snippets do the same thing and the code roughly seems pretty equivalent, so I'm going to keep this review DRY and won't mention again to declare variables as close as possible to their usage ;)
What happened to the constant? Are magic numbers any less magical in C# than in VB?
Timing
You're timing your code the VB6 way, in both snippets. Take a look at the StopWatch class, which is much better at this kind of task (be it just for the
VB
Declarations
You're not consistent with your declaration style:
Dim numCols As Integer, status As Integer, columnIdx As IntegerWhile this is correct and avoids the common mistake of only declaring a type to the last variable declared, I don't like multiple declarations on the same line. It's more readable when they're separated. These three being declared on the same line just seems arbitrary.
The variables should be declared as close as possible to their usage, this block of declarations at the top of your method isn't helping readability either.
This line serves no purpose and can be eliminated:
numRows = 0Loop
You're fetching the first result before iterating the result set:
status = cqResultSet.MoveNext
' Collect query results.
Do While status = AD_SUCCESSAnd then your loop ends like this:
status = cqResultSet.MoveNext
LoopI think you can eliminate the
status identifier along with the first check, and do this instead:Do While cqResultSet.MoveNext = AD_SUCCESSThe loop doesn't need to enter the
Select Case block if colValue is Nothing - I'm more of a C# guy so this could be wrong, but I think the Continue keyword could be used here to skip that iteration right there:'Make sure that we dont pass along a null reference
If colValue = Nothing Then ContinueNot sure I like the
Select Case here, and I'm not sure the looping over returned columns is buying you anything either. I don't know this API, but if this were ADO.NET I'd recommend you get the value for each column name, and assign it to result if that column name exists, like this (syntax is probably wrong, and C#-like, but you get the idea):if (cqResultSet["ID"] != null) results.IssueId = cqResultSet["ID"].Value;The point, is that you're naming all columns and all properties anyway, so the loop+switch isn't buying you anything except the need to now track what column index you're at.
Naming
I don't like the underscore in identifiers. Stick to PascalCase convention, be consistent. Also
results would be a better name for allItems (Return allItems becomes Return result), and result would be a better name for results (allItems.Add(results) becomes results.Add(result)).If
AD_SUCCESS is a status code, this naming makes me think it's a constant. However status codes are a suite of closely related constants, usually mutually exclusive - this looks like a job for an Enum.C#
Most of the above also applies to the C# code. Both snippets do the same thing and the code roughly seems pretty equivalent, so I'm going to keep this review DRY and won't mention again to declare variables as close as possible to their usage ;)
while (status == 1)What happened to the constant? Are magic numbers any less magical in C# than in VB?
Timing
You're timing your code the VB6 way, in both snippets. Take a look at the StopWatch class, which is much better at this kind of task (be it just for the
ElapsedMilliseconds property!).Code Snippets
Dim numCols As Integer, status As Integer, columnIdx As IntegernumRows = 0status = cqResultSet.MoveNext
' Collect query results.
Do While status = AD_SUCCESSstatus = cqResultSet.MoveNext
LoopDo While cqResultSet.MoveNext = AD_SUCCESSContext
StackExchange Code Review Q#1257, answer score: 7
Revisions (0)
No revisions yet.