0

I have something like this:

using (SqlDataAdapter a = new SqlDataAdapter(query, c))
            {
                // 3
                // Use DataAdapter to fill DataTable
                DataTable t = new DataTable();
                a.Fill(t);

                string txt = "";
                for (int i = 0; i < t.Rows.Count; i++)
                {
                    txt += "\n" + t.Rows[i][0] + "\t" + t.Rows[i][1] + "\t" + t.Rows[i][2] + "\t" + t.Rows[i][3] + "\t" + t.Rows[i][4] + "\t" + t.Rows[i][5] + "\t" + t.Rows[i][6] + "\t" + t.Rows[i][7] + "\t" + t.Rows[i][8] + "\t" + t.Rows[i][9] + "\t" + t.Rows[i][10] + "\t" + t.Rows[i][11] + "\t" + t.Rows[i][12] + "\t" + t.Rows[i][13] + "\t" + t.Rows[i][14] + "\t" + t.Rows[i][15] + "\t" + t.Rows[i][16] + "\t" + t.Rows[i][17] + "\t" + t.Rows[i][18] + "\t" + t.Rows[i][19];
                }


            }

I need this tabs between columns to generate txt file later. Problem is that t.Rows.Count = 600000 and this loop works 9 hours. Any ideas how to make this faster?

Regards kazik

3
  • Is your environment multi-core? This seems like an easily parallelized operation. Also, why not directly write to a file stream instead of generating a large string first? Commented Apr 26, 2012 at 20:29
  • 4
    You might want to try building the result using pure SQL. It may be faster than pushing all the data through to the C# side for processing. Also, failing that, instead of filling a datatable and then iterating, you could iterate directly over an IDataReader (streaming in the data as needed). Commented Apr 26, 2012 at 20:30
  • 1
    You may also want to write directly into file, and not hold the data in memory. Commented Apr 26, 2012 at 20:32

4 Answers 4

8

Use a StringBuilder instead of concatenating your strings.

            string txt = "";
            StringBuilder sb = new StringBuilder();
            for (int i = 0; i < t.Rows.Count; i++)
            {
                sb.Append(t.Rows[i][0]); 
                sb.Append("\t");
                sb.Append(t.Rows[i][1]);
                //and so on...
           }
           string txt = sb.ToString();

Adding a string to another string (by assigning it back again) takes effort relative to the length of the combined string - this gets very costly fast for large strings - it is a Schlemiel the Painter's algorithm. Using a StringBuilder avoids this since no new string have to be created every time you add a new string to its content.

I have to question though why you keep all of this data in memory just to write it out to a file - you should switch to a model that allows you to generate line by line of the text content, I would suggest using a data reader instead and method that yields text lines, e.g.:

public IEnumerable<string> GenerateTextOutput()
{
   //...
   //using SqlDataReader instead
   while(reader.Read())
   {
     //assemble string for next row
     string txt = "...";
     yield return txt;
   }
}

Then you can just write this output to disk:

File.WriteAllLines("foo.txt", GenerateTextOutput());
Sign up to request clarification or add additional context in comments.

1 Comment

Also, if you can estimate the size of the data, initialize the StringBuilder with that size.
4

Use StringBuilder. You can find examples everywhere on the web.

5 Comments

this article shows that concatenation is faster then StringBuilder: geekswithblogs.net/BlackRabbitCoder/archive/2010/05/10/…
It's not continuously concatenating each result together into one giant string as you are.
When the number of items is constant, this is true. If not, it's not. This is a common mistake btw: Resharper eroneously uses StringBuilder in generated ToString() methods although the number of items is constant. The C# compiler does this right in anonymous type ToString().
@usr: I've never heard of that before. Why does it matter if the number of items is constant?
Because you can statically size a holding array for the parts. You don't need to grow it. From that array you can also determine the final string size in a single cheap pass over it. You can then allocate the target string to the perfect size. (Look at string.Concat in Reflector).
2

Others mentioned StringBuilder, and they are correct. However, note that in .NET 4 StringBuilder became more efficient, avoiding reallocations and memory copying altogether. If you're using a previous version of .NET, you will get worse performance. It will still beat what you're doing.

I wonder what you're doing with txt after it's ready. You might want to avoid storing the string in memory altogether, and write it directly to a file (I don't believe you're showing your users 600,000 lines...)

Comments

0

StringBuilder has already been mentioned by others and should provide significant benefit.

Do you really need to keep the entire text in memory with 600K rows? You can speed this up a lot by directly writing to the file.

And for that many iterations, assigning Rows[i] to a variable might also provide some extra spped-up.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.