HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

WebMethod slower on execution than Windows application of the same code

Submitted by: @import:stackexchange-codereview··
0
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

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:

Dim numCols As Integer, status As Integer, columnIdx As Integer


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:

numRows = 0


Loop

You're fetching the first result before iterating the result set:

status = cqResultSet.MoveNext

' Collect query results.
Do While status = AD_SUCCESS


And then your loop ends like this:

status = cqResultSet.MoveNext
Loop


I think you can eliminate the status identifier along with the first check, and do this instead:

Do While cqResultSet.MoveNext = AD_SUCCESS


The 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 Continue


Not 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 Integer
numRows = 0
status = cqResultSet.MoveNext

' Collect query results.
Do While status = AD_SUCCESS
status = cqResultSet.MoveNext
Loop
Do While cqResultSet.MoveNext = AD_SUCCESS

Context

StackExchange Code Review Q#1257, answer score: 7

Revisions (0)

No revisions yet.