ASP.NET C# SQL Server DataReader

Question

I'm not super-skilled at ASP.NET, however I have a web application which HAD to be written in ASP.NET C#, and I'm having trouble with a page that loads data from the database and puts it into a table. Below is my code. Basically, what happens is it will intermittently say "Page can not be displayed" when trying to load. It ALWAYS takes about 10-20 seconds to load the page and I KNOW I'm doing something wrong. Can someone please point out my mistake?

using (SqlConnection dbConn = new SqlConnection(strConnection))
{
    SqlDataAdapter dbAdapter = new SqlDataAdapter();
    SqlCommand dbCommand = new SqlCommand();
    dbConn.Open();
    // I tried the SET ARTITHABORT portion below based on other posts I found on SO - no better performance though
    dbCommand.Connection = dbConn;
    dbCommand.CommandText = "SET ARITHABORT ON";        
    dbCommand.ExecuteNonQuery();        
    dbCommand.CommandText = @"SELECT ID, NAME, PART_NUMBER, BARCODE, QOH, MINIMUM_QOH, LAST_PRICE FROM INVENTORY" + Session["table_extension"].ToString();
    dbCommand.Connection = dbConn;
    SqlDataReader dbReader = dbCommand.ExecuteReader(CommandBehavior.CloseConnection);

    if (dbReader.HasRows)
    {
        strInventoryTable = @"<table id='inventoryTable' cellspacing='0' cellpadding='5' border='0' width='100%'>
                                <thead>
                                    <tr>
                                        <th>Name</th>
                                        <th>Part No.</th>
                                        <th>Barcode</th>
                                        <th>QOH</th>
                                        <th>Min. QOH</th>
                                        <th>Last Price</th>
                                        <th>Action</th>
                                    </tr>
                                </thead>
                                <tbody>";  
        string rowMarker = "even";
        while (dbReader.Read())
        {
            if (rowMarker == "even")
            {
                strInventoryTable += "<tr class='even clickable' onclick='location.href=\"edit_product.aspx?id=" + dbReader["ID"] + "\";'>";
                rowMarker = "odd";
            }
            else
            {
                strInventoryTable += "<tr class='odd clickable' onclick='location.href=\"edit_product.aspx?id=" + dbReader["ID"] + "\";'>";
                rowMarker = "even";
            }
            strInventoryTable += "<td>" + dbReader["NAME"] + "</td>";                        
            strInventoryTable += "<td class='tdCenter'>" + dbReader["PART_NUMBER"] + "</td>";
            strInventoryTable += "<td class='tdCenter'>" + dbReader["BARCODE"] + "</td>";
            strInventoryTable += "<td class='tdRight'>" + dbReader["QOH"] + "</td>";
            strInventoryTable += "<td class='tdRight'>" + dbReader["MINIMUM_QOH"] + "</td>";
            strInventoryTable += "<td class='tdRight'>" + globals.formatMoney(dbReader["LAST_PRICE"].ToString()) + "</td>";
            strInventoryTable += "<td class='tdCenter'><a href='edit_product.aspx?id=" + dbReader["ID"] + "'>Edit</a>&nbsp;&nbsp;";
            strInventoryTable += "<a href='delete_product.aspx?id=" + dbReader["ID"] + "'>Delete</a></td>";
            strInventoryTable += "</tr>";
        }
        strInventoryTable += "</tbody></table>";
    }
    else
    {
        strInventoryTable = "<p><strong><em>No inventory found in database</em></strong></p>";
    }
} 
inventoryTable.InnerHtml = strInventoryTable;

The query itself takes like 0.00003 seconds to complete. So I know it's not the query, and I'm only returning around 2,400 rows.

UPDATE

Trying to debug where the bottle neck is... here are the results

Query started:3/24/2016 9:06:06 AM

Query finished: 3/24/2016 9:06:06 AM

Data reader started: 3/24/2016 9:06:06 AM

Data reader ended: 3/24/2016 9:06:43 AM

So my issue is definitely in the while (dbReader.Read()) {} loop

You don't want to use a GridView instead of building the table "by hand"? — ConnorsFan, Mar 24 '16 at 13:00
go to this link and use GridView http://quickstarts.asp.net/quickstartv20/aspnet/doc/ctrlref/data/gridview.aspx — rashfmnb, Mar 24 '16 at 13:01
When you debug this, where is the bottleneck? Don't *guess*, *measure*. — David, Mar 24 '16 at 13:02
@ConnorsFan I'm fine with building it by hand, I just need to figure out what is causing the problem. Plus I'm using the jQuery dataTable plugin on this table after it is created to allow for searching/sorting. — Phil, Mar 24 '16 at 13:02
@David how do I go about figuring that out? How do I debug it? — Phil, Mar 24 '16 at 13:02
@Phil: https://msdn.microsoft.com/en-us/library/sc65sadd.aspx — David, Mar 24 '16 at 13:04
Start replacing that string concatenations with a StringBuilder. Append the text in your loops and write it out in a single call at the exit from the loop using the ToString() — Steve, Mar 24 '16 at 13:04
If the performance issue is string building, use `StringBuilder` insead of string concatenation. — Dan Guzman, Mar 24 '16 at 13:04
If you insist on building the HTML by hand, at least use a StringBuilder. Concatenating strings the way you do most likely has a huge performance impact. — Peter B, Mar 24 '16 at 13:04
It seems kind of silly to be building this output by hand in the code like this anyway. If this is MVC, it should be done in the view. If this is WebForms, bind the data to a control on the page. — David, Mar 24 '16 at 13:05
@ConnorsFan, just returning just one row works fine. It's only as the table continues to grow (now at approx 2,400 items) it takes longer and longer and bails out intermittently. — Phil, Mar 24 '16 at 13:11
You use `strInventoryTable += ` 11 times within each loop, so if you have 2,400 rows of data you have created and destroyed 26399 strings, and are left with one usable on. You definitely need to use a `StringBuilder` as suggested. This means you create 1 instance of string builder and simply modify it with each iteration. — GarethD, Mar 24 '16 at 13:12
I assume that using StringBuilder will help. In case it would not be enough, you could try this: http://stackoverflow.com/questions/10649843/how-to-increase-executiontimeout-for-a-long-running-query. — ConnorsFan, Mar 24 '16 at 13:14
Or, instead of a `StringBuilder`, just bind the data to a control. You have a table of data from a database and you want to display it in a table on the page. This is a *textbook* definition of what the WebForms controls are *for*. — David, Mar 24 '16 at 13:15
@Phil building the HTML by hand is *not* OK, even for demos. What are you going to do when you need to change a color? Add paging? Use CSS? There are *far* better, faster, more scaleable and maintainable ways to build tables, both with Web Forms and MVC. Which are you using? In WebForms for example you could use a [Repeater](http://www.w3schools.com/aspnet/aspnet_repeater.asp) while in MVC you could even use [a loop](http://www.asp.net/mvc/overview/older-versions-1/models-data/displaying-a-table-of-database-data-cs) in the view. Binding would also be a lot easier and performance a lot faster — Panagiotis Kanavos, Mar 24 '16 at 13:20

score 5 · Accepted Answer · answered Mar 24 '16 at 13:09

I really suggest to replace all the string concatenations with a StringBuilder.
All those string concatenations are continuously creating and allocating new strings in memory

Having an intermittent error is clearly a symptom of a stressed server.

    // Initialize with a big internal buffer
    // It seems that you have a lot of data but I can't measure from here, you can
    StringBuilder sb = new StringBuilder(1024*1024);     
    if (dbReader.HasRows)
    {
        sb.Append(@"<table id='inventoryTable' cellspacing='0' cellpadding='5' border='0' width='100%'>
                                <thead>
                                    <tr>
                                        <th>Name</th>
                                        <th>Part No.</th>
                                        <th>Barcode</th>
                                        <th>QOH</th>
                                        <th>Min. QOH</th>
                                        <th>Last Price</th>
                                        <th>Action</th>
                                    </tr>
                                </thead>
                                <tbody>");  
        string rowMarker = "even";
        while (dbReader.Read())
        {
            if (rowMarker == "even")
            {
                sb.Append("<tr class='even clickable' onclick='location.href=\"edit_product.aspx?id=" + dbReader["ID"] + "\";'>");
                rowMarker = "odd";
            }
            else
            {
                sb.Append("<tr class='odd clickable' onclick='location.href=\"edit_product.aspx?id=" + dbReader["ID"] + "\";'>");
                rowMarker = "even";
            }
            sb.Append("<td>" + dbReader["NAME"] + "</td>");
            sb.Apppend("<td class='tdCenter'>" + dbReader["PART_NUMBER"] + "</td>");
            ......
        }
        sb.Append("</tbody></table>");
    }
    else
    {
        sb.Append("<p><strong><em>No inventory found in database</em></strong></p>");
    }
    inventoryTable.InnerHtml = sb.ToString();

I'd suggest to let the web form or MVC view buld the table instead of trying to hand-code the table HTML in the backend — Panagiotis Kanavos, Mar 24 '16 at 13:14
Okay HOLY CRAP! I did this and the table loads INSTANTLY!!!! Thank you so much!!!! StringBuilder is my HERO! — Phil, Mar 24 '16 at 13:18
@Phil not yet. This is still ugly, slow and unmaintainable. Use the *proper* technologies to build tables. You could have built the same table *with* Bootstrap and paging in the number of lines you used — Panagiotis Kanavos, Mar 24 '16 at 13:21
I am feeling a bit guilty. The advices from @PanagiotisKanavos have a lot of merits. Think to this answer as a quick fix for your problem and when you have enough time to replace your actual code start looking at the tools suggested here. — Steve, Mar 24 '16 at 13:57

ASP.NET C# SQL Server DataReader

1 Answers1