Performance issue with this code

Question

the following code is for user control(it display banner), the page get stuck in IIS with status Executerequesthandler (when there is concurrent requests for this page), when I take this user control out from the page it runs smoothy, please note this control is embeded 5 times in the page. Here is the entire code for this user control, can someone spot out the problem?

Public Class daAds

    Private Remote_Host As String
    Private Script_Name As String
    Private PATH_INFO As String
    Private Page_Link As String
    Private Country As String   

    Public Property p_Country() As String
        Get
            Return Country
        End Get
        Set(ByVal value As String)
            Country = value
        End Set
    End Property

    Public Property p_Page_Link() As String
        Get
            Return Page_Link
        End Get
        Set(ByVal value As String)
            Page_Link = value
        End Set
    End Property

    Public Property p_Remote_Host() As String
        Get
            Return Remote_Host
        End Get
        Set(ByVal value As String)
            Remote_Host = value
        End Set
    End Property

    Public Property p_Script_Name() As String
        Get
            Return Script_Name
        End Get
        Set(ByVal value As String)
            Script_Name = value
        End Set
    End Property

    Private ConnectionToFetch As SqlConnection
    Private ReadOnly Property Connection() As SqlConnection
        Get
            ConnectionToFetch = New SqlConnection(ConnectionString)
            ConnectionToFetch.Open()
            Return ConnectionToFetch
        End Get
    End Property

    Private ReadOnly Property ConnectionString() As String
        Get
            Return ConfigurationManager.ConnectionStrings("ConnStr").ConnectionString
        End Get
    End Property

    Public Property p_PATH_INFO() As String
        Get
            Return PATH_INFO
        End Get
        Set(ByVal value As String)
            PATH_INFO = value
        End Set
    End Property

    Public Function showAd(ByVal Banner_inc As Integer, ByVal banner_layout As String, Optional ByVal ShowAdsInfo As Integer = 0) As String
        'Return ""

        Try
            'Dim connectionString As String = ConfigurationManager.ConnectionStrings("ConnStr").ConnectionString
            Dim imp_user_ip As String = Trim(Remote_Host)
            Dim imp_country As String = Trim(p_Country)
            imp_country = Replace(imp_country, Chr(10), "")
            imp_country = Replace(imp_country, Chr(13), "")

            Dim imp_page_name As String = Trim(Script_Name)
            Dim imp_page_name2 As String = Trim(PATH_INFO)
            Dim imp_page_link As String = p_Page_Link
            'Response.Write(imp_page_name)
            'ParamArrayAttribute()

            'Dim m As DataSet
            'm = SqlHelper.ExecuteDataset(connectionString, CommandType.StoredProcedure, "disp_banner_byPageName_views", parameters)

            Dim InsertCommand As New SqlCommand
            InsertCommand.Connection = Connection
            InsertCommand.CommandText = "disp_banner_byPageName_views"
            InsertCommand.CommandType = CommandType.StoredProcedure ' 
            'Dim IdParameter = New SqlParameter("@CategoryID", SqlDbType.Int)
            'Dim NameParameter = New SqlParameter("@CategoryName", SqlDbType.NVarChar)
            'IdParameter.Direction = ParameterDirection.Output
            'NameParameter.Value = txtCategoryName.Text
            'InsertCommand.Parameters.Add(IdParameter)
            'InsertCommand.Parameters.Add(NameParameter)

            Dim Param_Imp_user_ip = New SqlParameter("@imp_user_ip", SqlDbType.VarChar)
            Param_Imp_user_ip.Direction = ParameterDirection.Input
            Param_Imp_user_ip.Value = imp_user_ip
            InsertCommand.Parameters.Add(Param_Imp_user_ip)
            Param_Imp_user_ip = Nothing

            Dim Param_imp_country = New SqlParameter("@imp_country", SqlDbType.VarChar)
            Param_imp_country.Direction = ParameterDirection.Input
            Param_imp_country.Value = imp_country '"jo" '
            InsertCommand.Parameters.Add(Param_imp_country)
            Param_imp_country = Nothing

            Dim Param_banner_inc = New SqlParameter("@banner_inc", SqlDbType.Int)
            Param_banner_inc.Direction = ParameterDirection.Input
            Param_banner_inc.Value = Banner_inc
            InsertCommand.Parameters.Add(Param_banner_inc)
            Param_banner_inc = Nothing

            Dim Param_imp_page_name = New SqlParameter("@imp_page_name", SqlDbType.VarChar)
            Param_imp_page_name.Direction = ParameterDirection.Input
            Param_imp_page_name.Value = imp_page_name
            InsertCommand.Parameters.Add(Param_imp_page_name)
            Param_imp_page_name = Nothing

            Dim Param_imp_page_link = New SqlParameter("@imp_page_link", SqlDbType.VarChar)
            Param_imp_page_link.Direction = ParameterDirection.Input
            Param_imp_page_link.Value = imp_page_link
            InsertCommand.Parameters.Add(Param_imp_page_link)
            Param_imp_page_link = Nothing

            Dim Param_banner_layout = New SqlParameter("@banner_layout", SqlDbType.VarChar)
            Param_banner_layout.Direction = ParameterDirection.Input
            Param_banner_layout.Value = banner_layout
            InsertCommand.Parameters.Add(Param_banner_layout)
            Param_banner_layout = Nothing

            Dim Param_activeBanners = New SqlParameter("@activeBanners", SqlDbType.VarChar)
            Param_activeBanners.Direction = ParameterDirection.Input
            Param_activeBanners.Value = ""
            InsertCommand.Parameters.Add(Param_activeBanners)
            Param_activeBanners = Nothing

            Dim Param_banner_width = New SqlParameter("@banner_width", SqlDbType.Int)

            Param_banner_width.Direction = ParameterDirection.Output
            InsertCommand.Parameters.Add(Param_banner_width)

            Dim Param_banner_height = New SqlParameter("@banner_height", SqlDbType.Int)
            Param_banner_height.Direction = ParameterDirection.Output
            InsertCommand.Parameters.Add(Param_banner_height)

            Dim Param_campaign_id = New SqlParameter("@campaign_id", SqlDbType.Int)
            Param_campaign_id.Direction = ParameterDirection.Output
            InsertCommand.Parameters.Add(Param_campaign_id)

            Dim Param_imp_id = New SqlParameter("@imp_id", SqlDbType.Int)
            Param_imp_id.Direction = ParameterDirection.Output
            InsertCommand.Parameters.Add(Param_imp_id)

            Dim Param_banner_url = New SqlParameter("@banner_url", SqlDbType.VarChar, 500)
            Param_banner_url.Direction = ParameterDirection.Output
            InsertCommand.Parameters.Add(Param_banner_url)

            Dim Param_banner_img = New SqlParameter("@banner_img", SqlDbType.VarChar, 100)
            Param_banner_img.Direction = ParameterDirection.Output
            InsertCommand.Parameters.Add(Param_banner_img)

            Dim Param_banner_text = New SqlParameter("@banner_text", SqlDbType.VarChar, 1000)
            Param_banner_text.Direction = ParameterDirection.Output
            InsertCommand.Parameters.Add(Param_banner_text)   

            Dim Param_banner_script = New SqlParameter("@banner_script", SqlDbType.VarChar, 2000)
            Param_banner_script.Direction = ParameterDirection.Output
            InsertCommand.Parameters.Add(Param_banner_script)

            Dim Param_banner_ID = New SqlParameter("@banner_ID", SqlDbType.Int)
            Param_banner_ID.Direction = ParameterDirection.Output
            InsertCommand.Parameters.Add(Param_banner_ID)

            Dim param_adv_name_script = New SqlParameter("@adv_name", SqlDbType.VarChar, 2000)
            param_adv_name_script.Direction = ParameterDirection.Output
            InsertCommand.Parameters.Add(param_adv_name_script)

            InsertCommand.ExecuteNonQuery()

            Dim ActiveBanner As String = ""
            Dim banner_height As Integer
            Dim campaign_id As Integer
            Dim imp_id As Integer
            Dim banner_url As String
            Dim banner_img As String
            Dim banner_text As String
            Dim banner_script As String
            Dim banner_ID As Integer
            Dim banner_width As String

            'ActiveBanner = Param_activeBanners.Value()
            banner_width = Param_banner_width.Value()
            banner_height = Param_banner_height.Value()
            If (Not IsDBNull(Param_campaign_id.Value())) Then
                campaign_id = Convert.ToInt16(Param_campaign_id.Value())
            End If
            If (Not IsDBNull(Param_imp_id.Value())) Then
                imp_id = Convert.ToInt16(Param_imp_id.Value())
            End If

            banner_url = Param_banner_url.Value()
            banner_img = Param_banner_img.Value()
            banner_text = Param_banner_text.Value()
            banner_script = Param_banner_script.Value()
            banner_ID = Param_banner_ID.Value()

            ConnectionToFetch.Close()
            ConnectionToFetch = Nothing

            Param_banner_width = Nothing
            Param_banner_height = Nothing
            Param_campaign_id = Nothing
            Param_imp_id = Nothing
            Param_banner_url = Nothing
            Param_banner_img = Nothing
            Param_banner_text = Nothing
            Param_banner_script = Nothing
            Param_banner_ID = Nothing
            param_adv_name_script = Nothing

            If imp_page_link = "" Then
                imp_page_link = " "
            End If
            'Dim x As Integer = parameters(9).Value

            If String.IsNullOrEmpty(campaign_id) Then
                campaign_id = -1
            End If

            If IsNothing(campaign_id) Then
                campaign_id = -1
            End If

            If campaign_id < 1 Then 'If CInt("0" & param_campaign_id.value) < 1 Then
                Return "<!-- log name='campNull' value='" & campaign_id & "' -->"

            End If

            If ActiveBanner = "" Then
                ActiveBanner = banner_ID
            ElseIf InStr("," & ActiveBanner & ",", "," & banner_ID & ",") < 1 Then
                ActiveBanner = banner_ID & "," & ActiveBanner
            End If

            Dim strRet As String

            'If request.QueryString("ads") = 1 Then
            'Response.Write(" SessionID:" & Session.SessionID & " " & " disp_custom_banner " & campaign_id & "," & banner_ID & "," & adv_id & " Country=" & gCountry & " Banner=" & adv_name & " IP=" & request.ServerVariables("Remote_host"))
            ' End If

            Dim strbuilder As New StringBuilder

            If ShowAdsInfo = 1 Then
                strbuilder.Append("disp_custom_banner " & campaign_id & "," & banner_ID & "," & " Country=" & imp_country & ", Banner=" & param_adv_name_script.Value())
            End If

            strbuilder.Append("<!-- log banner=" & banner_ID & " activeBanners=" & ActiveBanner & " -->")
            strbuilder.Append("<script language='javascript' defer=defer>AdvimgBanner=" & IIf(imp_id = Nothing, 0, imp_id) & ";</script>" & vbCr)
            If Len(banner_script) > 5 Then
                ''''''''' added for counting issue
                Dim tmtmp As String = Replace(DateTime.Now.ToShortTimeString(), "PM", "")
                Dim tm As String = Replace(tmtmp, "AM", "")
                tm = Replace(tm, ":", "")
                '''''''''   
                Dim max, min, RandomNum
                max = 10000
                min = 1
                RandomNum = CStr(Int((max - min + 1) * Rnd() + min))
                RandomNum = RandomNum & "-" & banner_ID
                Dim ReFactor As String = Replace(banner_script, "[timestamp]", RandomNum & tm)
                strbuilder.Append(Replace(ReFactor, "&cacheburst=", "&cacheburst=" & RandomNum & tm))
                Return strbuilder.ToString

            End If
            If InStr(LCase(banner_img), ".swf") > 0 Then
                Dim url_str As String = HttpContext.Current.Server.UrlEncode("http://www.xxx.com/includes/bannerhits.asp?campaign_id=" & campaign_id & "&imp_id=" & imp_id & "&URL=" & HttpContext.Current.Server.UrlEncode(banner_url))
                Dim banner_str As String = "<A HREF=/includes/in_banner_hits.asp?campaign_id=" & campaign_id & "&imp_id=" & imp_id & "&URL=" & HttpContext.Current.Server.UrlEncode(banner_url) & " TARGET='_blank'>"
                Dim bannersrc As String = "/updates/banners/" & banner_img
                Dim concatEmbedID As String = "CAMP" & campaign_id
                Dim DivNameID As String = "flashbanner" & banner_layout
                Dim bannerhit As String = "http://www.xxx.com/includes/bannerhits.asp?campaign_id=" & campaign_id & "&imp_id=" & imp_id & "&URL=" & banner_url
                bannerhit = HttpContext.Current.Server.UrlEncode(bannerhit)

                strbuilder.Append("<div id='<%=DivNameID%>'>")
                strbuilder.Append("<a href='http://www.adobe.com/go/getflashplayer'>")
                strbuilder.Append("<img src='http://www.adobe.com/images/shared/download_buttons/get_flash_player.gif' alt='Get Adobe Flash player' border='0' /></a></div>")
                strbuilder.Append("<script type='text/javascript' src='/includes/scripts/swfobject.js' ></script>")

                strbuilder.Append("<script type='text/javascript'  >")
                strbuilder.Append("var so = new SWFObject(" + bannersrc + ", " + DivNameID + "," + banner_width + ", " + banner_height + ", ""6"", ""#ffffff"");")
                strbuilder.Append("so.addParam(""quality"", ""autohigh "");")
                strbuilder.Append("so.addParam(""bgcolor"", ""#ffffff"");")
                strbuilder.Append("so.addParam(""swliveconnect"", ""false"");")
                strbuilder.Append("so.addParam(""wmode"", ""transparent"");")
                strbuilder.Append("so.addVariable(""clickTAG""," + bannerhit + ");")
                strbuilder.Append("so.write(" + DivNameID + ");")
                strbuilder.Append("</SCRIPT>")
            Else
                strbuilder.Append("<A HREF=/includes/in_banner_hits.asp?campaign_id=" & campaign_id & "&imp_id=" & imp_id & "&URL=" & HttpContext.Current.Server.UrlEncode(banner_url) & " TARGET='_blank'>" & _
                         " <IMG SRC='/updates/banners/" & banner_img & "' WIDTH='" & banner_width & "' HEIGHT='" & banner_height & "' BORDER='0' ALT='" & banner_text & "' vspace='5'></A>")
                'response.write(banner_str)
            End If
            If Err.Number <> 0 Then
                strbuilder.Append("<!--log name='err' value='" & Err.Description & _
                             "' Source='" & Err.Source & "' Number='" & Err.Number & "'-->")
            End If  

            InsertCommand = Nothing

            Dim strReturn As String = strbuilder.ToString
            strbuilder = Nothing

            Return strReturn
        Catch ex As Exception
        End Try

    End Function

End Class

Your doing SQL queries in your control. These are potentially slow. You could see if they can be optimized. Furthermore you could cache the results outside of the user control so that the query doesn't have to be executed five times. — flo, Mar 25 '12 at 20:55
The same sql queries were doing well in the classic asp(the old version of this project), so there is nothing wrong with these queries. We cannot do cache since this is advertisement control each load should show a different banner. — Cassini, Mar 25 '12 at 20:59

score 2 · Accepted Answer · edited May 23 '17 at 12:27

2

In short: You should create,open,use,close,dispose Connections where you're using them.

The best way is to use the using-statement. By not closing the connection as soon as possible, the Connection-Pool needs to create new physical connections to the dbms which is very expensive in terms of perfomance.

Using conn As New SqlClient.SqlConnection(ConfigurationManager.ConnectionStrings("ConnStr").ConnectionString)
    Using insertCommand As New SqlClient.SqlCommand("disp_banner_byPageName_views", conn)
        insertCommand.CommandType = CommandType.StoredProcedure 
        ' ....
    End Using        
End Using

Performance problems are the least you get when not closing connections properly.

Edit: I've overlooked the ConnectionToFetch.Close in the middle of the code.

But anyway, you should use using or the finally of a try/catch to close a connection, otherwise it'll keep open in case of any exceptions. Because you've already a try/catch you could use it to close it in it's finally block.

I don't want to nag even more, but an empty catch is bad, because you'll never know when an exception was raised. You might want to log or at least throw it again there to catch it in Application_Error and/or in a custom error page or at the caller of this method.

Try
    ' code here 
Catch ex As Exception
    ' log exception and/or throw(what is always better than to intercept it) 
    Throw
Finally
    ConnectionToFetch.Close
End Try

edited May 23 '17 at 12:27

Community

1
1

answered Mar 25 '12 at 21:44

Tim Schmelter

450,073
74
686
939

@Cassini: This **is** a specific answer: You're opening continually new connectios whithout closing them which means the [Connection-Pool](http://msdn.microsoft.com/en-us/library/8xx3tyca.aspx) needs to create continually new physical connections instead of reusing existing. "A physical channel such as a socket or a named pipe must be established, the initial handshake with the server must occur, the connection string information must be parsed, the connection must be authenticated by the server, checks must be run for enlisting in the current transaction, and so on." – Tim Schmelter Mar 26 '12 at 07:06
Please look carefully to the code, I close the connection, you can search for "ConnectionToFetch.Close" to find out where I close it. – Cassini Mar 26 '12 at 15:52
@Cassini: I've overlooked it in fact. But anyway, you should use `using` or the `finally` of a try/catch to close a connection, otherwise it'll keep open in case of any exceptions. Because you've already a try/catch you could use it to close it in it's finally block. – Tim Schmelter Mar 26 '12 at 15:55
@Cassini: I don't want to nag even more, but an empty catch is bad, because you'll never know when an exception was raised ;) You might want to log or at least `throw` it again there(f.e. to catch it in [Application_Error](http://msdn.microsoft.com/en-us/library/24395wz3.aspx) and/or in a custom error page). – Tim Schmelter Mar 26 '12 at 16:02
Who gave me negative reputation! If he think this is a silly question then show me how is it silly and answer it! Anyone should not give any reputation until the question get closed, I start hate this websit. – Cassini Mar 26 '12 at 20:19
@Cassini: Why do you comment my answer as i would have downvoted? I wouldn't have answered then. I agree that people should always comment when they downvote so that someone can learn from it or edit the question/answer accordingly. But that said, i assume that people thought that you haven't prepared the ground enough when posting so much code. Actually answers on SO should also help other people in future, but with so much code this is very unlikely("too localized"). – Tim Schmelter Mar 26 '12 at 20:32
1

@Cassini: Have you tried my edited answer? Are you sure that no exception is raised which causes the connection to stay open? Have you debugged `showAd`? – Tim Schmelter Mar 26 '12 at 20:42
That did not solve the problem. – Cassini Mar 27 '12 at 10:30

Performance issue with this code

1 Answers1