Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have following in Windows Forms .NET 3.5

It works fine for csv with records less than 10,000 but is slower for records above 30,000. Input csv file can can any records between 1 - 1,00,000 records

Code currently used :

/// <summary>
        /// This will import file to the collection object
        /// </summary>
        private bool ImportFile()
        {
            try
            {

                String fName;
                String textLine = string.Empty;
                String[] splitLine;

                // clear the grid view

                accountsDataGridView.Rows.Clear();

                fName = openFileDialog1.FileName;

                if (System.IO.File.Exists(fName))
                {
                    System.IO.StreamReader objReader = new System.IO.StreamReader(fName);

                    do
                    {
                        textLine = objReader.ReadLine();
                        if (textLine != "")
                        {
                            splitLine = textLine.Split(',');
                            if (splitLine[0] != "" || splitLine[1] != "")
                            {
                                accountsDataGridView.Rows.Add(splitLine);
                            }
                        }
                    } while (objReader.Peek() != -1);
                }
                return true;
            }
            catch (Exception ex)
            {
                if (ex.Message.Contains("The process cannot access the file"))
                {
                    MessageBox.Show("The file you are importing is open.", "Import Account", MessageBoxButtons.OK, MessageBoxIcon.Warning);
                }
                else
                {
                    MessageBox.Show(ex.Message);
                }

                return false;
            }

        }

Sample Input file :
18906,Y
18908,Y
18909,Y
18910,Y
18912,N
18913,N

Need some advice on optimizing this code for fast reads & view in grid.

share|improve this question
    
Do you have a multi core machine? Then you can parse in parallel. Does the CSV files contain redundant data? In this case it does make sense to intern strings to reduce memory load. The main issue why parsing does become slower is that you need more memory to hold the data. –  Alois Kraus Jan 4 '13 at 12:48
8  
Never rely on Exception.Message to determine the type of an exception! Catch the right one instead. –  codesparkle Jan 4 '13 at 14:02
    
I haven't used it, but there's a CSV parser in the VB.NET runtime. It is Microsoft.VisualBasic.FileIO.TextFieldParser. It is probably not faster but will cater for quotes, etc. –  Mark Hurd Jan 4 '13 at 15:51
add comment

5 Answers

What's the use of presenting even 5,000 records to the user? Do you expect a user to scroll to row 4711, look at "18912,N", and say "Strange, why wasn't there a '12345,N' in row 1234?", or "Interesting, now let's see where the corresponding '54321,X" is to be found!"?

Your GUI should help the user to specify the comparatively small set(s) of records pertinent to his current problem/question; even if working with the data means "check every item in turn", it would be more human to present the records in small batches.

All this points to using your file as a database table bound to the controls of your GUI; then the problem of "reading a .csv fast" completely vanishes.

share|improve this answer
    
True but it is still a good idea to read the stuff at once at a decent speed. When reading a CSV file like this it you will get around 20MB/s which is far less than any hard disc can deliver. If you read it a second time you get it from the OS cache at ca. 500MB/s but you parse it still at 20MB/s. –  Alois Kraus Jan 4 '13 at 22:02
add comment

I am assuming it is OK to load everything into memory all at once, since you are doing that with the grid anyways. The fastest simple option I see is loading the data into a data object and binding it to the grid, rather than adding rows one at a time.

I tossed together a CSV file with 100,000 rows, all consisting of the following data:

"a", "a", "a", "a", 1234, 1234, 1234, 1234

(It was thrown together quick just for quick turn-around sake.)

I then provided 3 ways of loading. One loads the data much like you did above:

  private void LoadDirect (string fName)
  {
     String textLine = string.Empty;
     String[] splitLine;

     // clear the grid view

     CSVGrid.SuspendLayout ();

     CSVGrid.Rows.Clear ();

     if (System.IO.File.Exists (fName))
     {
        System.IO.StreamReader objReader = new System.IO.StreamReader (fName);

        do
        {
           textLine = objReader.ReadLine ();
           if (textLine != string.Empty)
           {
              splitLine = textLine.Split (',');
              if (splitLine [0] != string.Empty || splitLine [1] != string.Empty)
              {
                 CSVGrid.Rows.Add (splitLine);
              }
           }
        } while (objReader.Peek () != -1);
     }

     CSVGrid.ResumeLayout ();
  }

That consistently took around 21 seconds to populate the grid.

To eliminate the possibility that the largest slowdown factor was loading from the file one line at a time, I produced a similar method which loads the entire file up front, then used a StringReader in place of the StreamReader inside the loop:

private void LoadBuffered (string fName)
{
 String textLine = string.Empty;
 String[] splitLine;

 CSVGrid.SuspendLayout ();

 // clear the grid view
 CSVGrid.Rows.Clear ();

 if (System.IO.File.Exists (fName))
 {
    System.IO.StreamReader objReader = new System.IO.StreamReader (fName);

    var  contents = objReader.ReadToEnd ();
    var  strReader = new System.IO.StringReader (contents);

    do
    {
       textLine = strReader.ReadLine ();
       if (textLine != string.Empty)
       {
          splitLine = textLine.Split (',');
          if (splitLine [0] != string.Empty || splitLine [1] != string.Empty)
          {
             CSVGrid.Rows.Add (splitLine);
          }
       }
    } while (strReader.Peek () != -1);
 }
 CSVGrid.ResumeLayout ();
 }

Once again, this took around 21 seconds consistently.

Finally, I created a simplistic variant which loads all the data up front, places it into a System.Data.DataTable in the loop, then binds the table to the grid upon completion:

  private void LoadBound (string fName)
  {
     String textLine = string.Empty;
     String[] splitLine;

     // clear the grid view

     var data = new DataTable ();
     data.Columns.AddRange (new []
                            {
                               new DataColumn ("Column1"),
                               new DataColumn ("Column2"),
                               new DataColumn ("Column3"),
                               new DataColumn ("Column4"),
                               new DataColumn ("Column5",typeof (int)),
                               new DataColumn ("Column6",typeof (int)),
                               new DataColumn ("Column7",typeof (int)),
                               new DataColumn ("Column8",typeof (int)),
                            });

     data.Rows.Clear ();

     if (System.IO.File.Exists (fName))
     {
        System.IO.StreamReader objReader = new System.IO.StreamReader (fName);

        var  contents = objReader.ReadToEnd ();

        var  strReader = new System.IO.StringReader (contents);

        do
        {
           textLine = strReader.ReadLine ();
           if (textLine != string.Empty)
           {
              splitLine = textLine.Split (',');
              if (splitLine [0] != string.Empty || splitLine [1] != string.Empty)
              {
                 data.Rows.Add (splitLine);
              }
           }
        } while (strReader.Peek () != -1);
     }

     CSVGrid.DataSource = data;
  }

This variant consistently takes around 0.8 seconds. Perhaps unsurprisingly, the real killer was adding items to the grid row-by-row.

It has the added benefit of isolating your file reading, parsing, and grid population operations into separate sections of code which could be split into methods/objects.

Of course, rather than using a System.Data.DataTable, I would prefer to use a well-structured object. The DataTable was merely used to build a quick example.

share|improve this answer
1  
Nice analysis. I think you have nailed his perf issue. It should be forbidden to ask performance questions without showing a profiler screenshot. –  Alois Kraus Jan 5 '13 at 0:26
add comment

A few thoughts that could improve speed up what you're doing:

  1. Use parallel processing to process multiple lines simultaneously. If you've got a multi-core machine, this would go some way to offloading the task of manipulating strings and adding them to a control. The BlockingCollection<T> would be well-suited to this task.

  2. Buffer your read of the file using a FileStream. The IO itself is quite an overhead, so doing fewer-but-bigger reads could speed up the process considerably.

  3. Don't actually read all the rows into the grid. Set up pagination so that you only load a portion of it until the user requests the next set of data. It's very unlikely a human will actually read 30,000 rows.

Here's an example of how to use the FileStream class and to handle the processing of the lines as an asynchronous process:

// Internally the collection uses a ConcurrentQueue<T> by default.
var queue = new BlockingCollection<string>();

// Set up an asynchronous operation to process the queue.
Task.StartNew(() => 
{
    foreach(var line in queue.GetConsumingEnumerable())
    {
        if (textLine != "")
        {
            splitLine = textLine.Split(',');
            if (splitLine[0] != "" || splitLine[1] != "")
            {
                accountsDataGridView.Invoke( 
                    () => accountsDataGridView.Rows.Add(splitLine);
            }
        }
    }
});

/* You should tweak the buffersize to get the balance you want between memory use
   and speed of execution. */
var bufferSize = 8192; // 8k is the default size used by `FileStream`.

using(var fs = 
    new FileStream(
        openFileDialog1.FileName,
        FileMode.Open, // Open an existing file (don't create it)
        FileAccess.Read, // Open the file to read it
        FileShare.Read, // Allow sharing with other readers
        bufferSize))
{
    using(var reader = new StreamReader(fs))
    {
        while(reader.Peek() >= 0)
        {
            queue.Add(reader.ReadLine());
        }

        // Signal that there are no more items to be added.
        queue.CompleteAdding();
    }
}

As an aside, you probably want to perform the read of the CSV on a background thread, so the operation doesn't block the UI.

share|improve this answer
    
It is a very bad idea to call invoke for every parsed line. The overhead for this is far higher than to read and parse a single line. You have exchanged one bottleneck with another one. If have tried a similar thing to use one reader thread and to have n parsing threads. It turned out that the single threaded solution did outperform it still by 30%. The reason was (I think) that you do loose cache locality if one thread reads stuff and n other threads try to read other stuff. To prevent too many UI updates read here: geekswithblogs.net/akraus1/archive/2011/12/02/147923.aspx –  Alois Kraus Jan 4 '13 at 22:09
add comment

There isn't much to optimize in regards to speed, but following is much more readable. If it is too slow, it probably isn't the method reading the file, but your WinForm that needs to display >30k records.

    accountsDataGridView.Rows.Clear();
    using (FileStream file = new FileStream(openFileDialog1.FileName, FileMode.Open, FileAccess.Read, FileShare.Read, 4096))
    using (StreamReader reader = new StreamReader(file))
    {
        while (!reader.EndOfStream)
        {
            var fields = reader.ReadLine().Split(',');
            if (fields.Length == 2 && (fields[0] != "" || fields[1] != ""))
            {
                accountsDataGridView.Rows.Add(fields);
            }
        }
    }
share|improve this answer
add comment

We use the GenericParser library. It's pretty easy to use, too.

Your code would look something like this:

GenericParserAdapter parser = new GenericParserAdapter(fName);

parser.ColumnDelimiter = ',';
parser.FirstRowSetsExpectedColumnCount = true;
parser.FirstRowHasHeader = false;

accountsDataGridView.DataSource = parser.GetDataTable();
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.