3
\$\begingroup\$

I have all large file of all the order sold in one week. This file gives one line for every order. We have over 5000 orders a day. It read the file line by line and then adds the sale to a database of sales. to I really need to boost performance.

   static void sold4weeks1()
    {
        string sold, asin;
        string[] lines = System.IO.File.ReadAllLines(@"C:\out\qqqqq.txt");

        inventoryBLL u = new inventoryBLL();
        try
        {
            foreach (string line in lines)
            {

                char[] tabs = { '\t' };
                string[] words = line.Split(tabs);
                asin = words[12];
                sold = words[14];

                if (words[0].Substring(0, 3) == "S01") continue;
                try
                {
                    if (words[4] == "Shipped" || words[4] == "Unshipped")
                    {

                        u.setSoldin28(asin, Convert.ToInt16(sold));
                        Console.WriteLine("Update");
                    }
                }
                catch
                {
                }
            }
        }
        catch
        { }
    }
\$\endgroup\$
13
  • 1
    \$\begingroup\$ @joe what is the purpose of this?> if (words[0].Substring(0, 3) == "S01") continue; \$\endgroup\$
    – I__
    Commented Sep 23, 2011 at 20:42
  • \$\begingroup\$ @I__ I was just wondering the same thing. \$\endgroup\$
    – asawyer
    Commented Sep 23, 2011 at 20:43
  • \$\begingroup\$ @I__ S01 is a prefix for destroyed or requestioned order. \$\endgroup\$
    – Joe Tyman
    Commented Sep 23, 2011 at 20:43
  • 7
    \$\begingroup\$ Just a reminder...you should never use try...catch{} and do nothing. Eating exceptions is horrible. \$\endgroup\$
    – John Hartsock
    Commented Sep 23, 2011 at 20:44
  • 3
    \$\begingroup\$ @Coding: That will just introduce unnecessary overheads to this and ultimately make it a lot slower than it has to be. What's being done in the code isn't enough to justify doing that. \$\endgroup\$ Commented Sep 23, 2011 at 21:06

8 Answers 8

8
\$\begingroup\$

Is u.setSoldin28(asin, Convert.ToInt16(sold)); writing to the database?

Save up all your changes and make them all under a single connection when the parse is finished. That should save quite a bit of time.

You have alot of other issues with this code though. I strongly urge you to look into proper exception handling techniques.

\$\endgroup\$
1
  • 4
    \$\begingroup\$ This is really the only improvement to be made. This whole task is going to be IO bound, what the CPU does with it is hardly going to matter. \$\endgroup\$ Commented Sep 23, 2011 at 23:01
2
\$\begingroup\$

Don't use ReadAllLines, read it line by line, it will boost performance by a lot

\$\endgroup\$
1
  • 1
    \$\begingroup\$ @Dani: as given. However, we know that they have ~5000 lines in it (let's guess 2.4 MB) and.... there is going to be database insertion/update in the tail of the process. Mmm. My bet is that the speed loss is in the tail that's not being shown \$\endgroup\$
    – sehe
    Commented Sep 26, 2011 at 10:19
2
\$\begingroup\$

move the char array to outside the loop.

In my experience, using StreamReader is the fastest way to read from a file.

Parallel.For/ForEach that per line loop.

\$\endgroup\$
1
\$\begingroup\$

From your above code performance should not be a big issue at all except for the exception handling. Reading in the file should be relatively fast, especially since its once a day. The processing of that file however I have to assumem is the issue here. Download ANTS profile. Are you inserting data in a transaction, etc. There's more here than is visible. As mentioned, try/catch will have a detrimental effect as well (we noticed the same thing in an internal app)

\$\endgroup\$
0
\$\begingroup\$

Instead of writing one line of sale to database at a time try the following for boosting performance -

  • Bulk insert lines into a temp table (create a table with just an identity column and a string column and each line will be inserted into the string column with identity field auto-incremented).
  • Write a stored procedure to read the temp table created above, parse the line and separate out fields. Insert separated out fields into the main sales table where you want the data to go to.
  • Since the SP will run under SQL server itself, rather than having a connection open from programming runtime to SQL it will execute faster.
    • You can schedule a job to run during off hours so that other database accesses are not hampered.
\$\endgroup\$
0
\$\begingroup\$

I tried to get a clue if there is a performance leak in the posted code or not so i wrote a litte check Consoleapp for your Problem. (Excluding the Update of the Database off course).
It would give me a great pleasure if you would run this "analysis" with your code and post us the results.

using System;
using System.IO;
using System.Diagnostics;

namespace PerformanceCheck
{
class MainClass
{
    public static void Main (string[] args) {
        Console.WriteLine ("Press enter to write file");
        Console.ReadLine ();

        FileStream stream = null;
        TextWriter writer = null;
        try {
            FileInfo info = new FileInfo ("/tmp/file.txt");
            if(info.Exists)
                info.Delete();

            stream = info.OpenWrite ();

            writer = new StreamWriter (stream);
            for (int i = 0; i< 500; i++) {
                writer.WriteLine ("S01\tsdf\tsdf\tsdf\tShipped\tsdf\tsdf\tsdf\tsdf\tsdf\tsdf\tsdf\tsdf\tsdf\t" + i);
                writer.WriteLine ("S01\tsdf\tsdf\tsdf\tUnshipped\tsdf\tsdf\tsdf\tsdf\tsdf\tsdf\tsdf\tsdf\tsdf\t" + i);
                writer.WriteLine ("S02\tsdf\tsdf\tsdf\tShipped\tsdf\tsdf\tsdf\tsdf\tsdf\tsdf\tsdf\tsdf\tsdf\t" + i);
                writer.WriteLine ("S02\tsdf\tsdf\tsdf\tUnshipped\tsdf\tsdf\tsdf\tsdf\tsdf\tsdf\tsdf\tsdf\tsdf\t" + i);
            }

            Console.WriteLine ("Successfully written file!");   
        } catch (Exception ex) {
            Console.WriteLine ("Dude, Exception!");
            Console.WriteLine (ex.Message);
            Console.WriteLine ("Press enter to quit;");
            Console.ReadLine ();
            return;
        } finally {
            if (writer != null)
                writer.Flush ();
            if (stream != null)
                stream.Close ();
        }
        Console.WriteLine ("Press enter to insert Data into Database");
        Console.ReadLine ();
        sold4weeks1 ();
        Console.WriteLine ("Data updated, press enter to quit");
        Console.ReadLine ();
    }

    static void sold4weeks1 () {

        Stopwatch watch = new Stopwatch ();

        string sold, asin;

        watch.Start ();
        string[] lines = System.IO.File.ReadAllLines ("/tmp/file.txt");

        Console.WriteLine ("Reading lines took: " + watch.ElapsedMilliseconds + "ms");
        watch.Reset ();

        watch.Start ();
        Stopwatch lineReadWatch = new Stopwatch ();
        try {
            foreach (string line in lines) {
                lineReadWatch.Start ();

                char[] tabs = { '\t' };
                string[] words = line.Split (tabs);
                asin = words [12];
                sold = words [14];

                if (words [0].Substring (0, 3) == "S01")
                    continue;
                try {
                    if (words [4] == "Shipped" || words [4] == "Unshipped") {

                        int i = Convert.ToInt16 (sold);
                        Console.WriteLine ("Update: "+i);
                    }
                } catch {
                } finally {
                    lineReadWatch.Stop ();
                    lineReadWatch.Reset ();
                    Console.WriteLine ("Handling of line took: " + lineReadWatch.ElapsedMilliseconds + "ms");
                }
            }
        } catch {
        }
        Console.WriteLine ("Whole routine took: " + watch.ElapsedMilliseconds + "ms");
        watch.Stop ();
    }

}
}

Developed and successfully tested on Mono/OSX

As far as i can see, yes there are some points where performance can be gained, but i would be intrested, if this is the real bottleneck. As the others before i believe the huge performance gains can be made in the database update code.
But I'm ready to be profen wrong and so I would be verry interested in your performance mesurements!

Output Snippet:

Handling of line took: 0ms
Update: 499
Handling of line took: 0ms
Whole routine took: 38ms
Data updated, press enter to quit

\$\endgroup\$
0
\$\begingroup\$

First of all it is critical make performance profiles. I recommend setting up dottrace from http://www.jetbrains.com/profiler/. Using this get the baseline numbers. These baseline numbers would be great to prove performance improvement.

It is also critical to write scalable code. Instead of loading all 5000 lines in-memory try using powers of database as suggested by samiksc

\$\endgroup\$
0
\$\begingroup\$

Is it really necessary to boost performance? This is a typical batch job that can run in nighttime for hours without any problem. Just make it rock solid (firstly remove the empty catch block, big mistake). No need for complicated logic or clever tricks.

If it does take unacceptable time to process the data, the bottleneck must be in the inventoryBBL class (or the way you use it). Don't commit changes to database after every single modifications (batch processing). A .NET profiler and a database profiler are also good ideas. How much time do you code spend in .NET code and how much in SQL time compared to processing time?

\$\endgroup\$

Your Answer

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